-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Rework HIR API to make invocations of the hir_crate query harder. #88880
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Some changes occurred in src/tools/clippy. cc @rust-lang/clippy |
r? @wesleywiser (rust-highfive has picked a reviewer for you, use r? to override) |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@bors try @rust-timer queue |
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
⌛ Trying commit 2067357c615db4c3df72eca28d54b3e8572255d2 with merge 61a27a20fa9db4b749e8bf81b59fc0bcb5d60587... |
☀️ Try build successful - checks-actions |
Queued 61a27a20fa9db4b749e8bf81b59fc0bcb5d60587 with parent 0212c70, future comparison URL. |
Finished benchmarking commit (61a27a20fa9db4b749e8bf81b59fc0bcb5d60587): comparison url. Summary: This benchmark run did not return any relevant changes. If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf. @bors rollup=never |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The title of the PR says that hir_crate
query calls are avoided, but the PR code looks to me like it's "just" (in quotes, because it is a good) clean up and API simplification. Am I missing something or should the title get updated?
@@ -23,7 +23,13 @@ LL | pub fn foo() {} | |||
|
|||
error: requires `sized` lang_item | |||
|
|||
error: aborting due to 3 previous errors | |||
error: requires `sized` lang_item |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not very important (deduplicated for users anyway), but any idea why these are tripled?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm also curious about this.
My first aim was to remove calls to |
Right, I got that, but it looks to me like the same amount of calls are still happening, but they are now hidden behind new functions. |
☔ The latest upstream changes (presumably #89328) made this pull request unmergeable. Please resolve the merge conflicts. |
⌛ Testing commit 77c3002 with merge cca895bdbd29df606af923faf3967edda3ee16ba... |
The job Click to see the possible cause of the failure (guessed by this bot)
|
💔 Test failed - checks-actions |
@bors retry |
☀️ Test successful - checks-actions |
Finished benchmarking commit (c02371c): comparison url. Summary: This benchmark run did not return any relevant changes. If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. @rustbot label: -perf-regression |
Tagging as a perf regression. Our heuristics currently didn't flag this earlier, but it seems like many-assoc-items is indeed regressed by this PR. |
Rework HIR API to make invocations of the hir_crate query harder. `hir_crate` forces the recomputation of queries that depend on it. This PR aims at avoiding useless invocations of `hir_crate` by making dependent code go through `tcx.hir()`.
Index and hash HIR as part of lowering Part of rust-lang#88186 ~Based on rust-lang#88880 (see merge commit).~ Once HIR is lowered, it is later indexed by the `index_hir` query and hashed for `crate_hash`. This PR moves those post-processing steps to lowering itself. As a side objective, the HIR crate data structure is refactored as an `IndexVec<LocalDefId, Option<OwnerInfo<'hir>>>` where `OwnerInfo` stores all the relevant information for an HIR owner. r? `@michaelwoerister` cc `@petrochenkov`
hir_crate
forces the recomputation of queries that depend on it.This PR aims at avoiding useless invocations of
hir_crate
by making dependent code go throughtcx.hir()
.