-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Avoid invoking the hir_crate query to traverse the HIR #88435
Conversation
(rust-highfive has picked a reviewer for you, use r? to override) |
@bors try @rust-timer queue |
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
⌛ Trying commit bc771728c56343d8fb09e4953354643a3473cea4 with merge ee52a1f5457264871e36cc1047d4bcbabd4f0a6a... |
☀️ Try build successful - checks-actions |
Queued ee52a1f5457264871e36cc1047d4bcbabd4f0a6a with parent 42a2a53, future comparison URL. |
Finished benchmarking try commit (ee52a1f5457264871e36cc1047d4bcbabd4f0a6a): comparison url. Summary: ERROR categorizing benchmark run! Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. @bors rollup=never |
r? @Aaron1011 |
@@ -519,6 +519,22 @@ impl<'hir> Map<'hir> { | |||
} | |||
} | |||
|
|||
/// Walks the contents of a crate. See also `Crate::visit_all_items`. | |||
pub fn walk_crate(self, visitor: &mut impl Visitor<'hir>) { |
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.
Could you rename this to something like walk_crate_without_crate_attrs
, to make it clear that it doesn't walk everything in the crate?
bc77172
to
d119a13
Compare
@bors r+ |
📌 Commit d119a13 has been approved by |
☀️ Test successful - checks-actions |
A performance run was done on this PR, but for some reason categorization failed. The aforementioned performance run predicted that this would be a small win, at most. However, during weekly performance triage, it was tagged as having mixed results on rustc instruction counts. Tagging as perf-regression mostly as hint that maybe someone should go and figure out why there was a discrepancy between what was predicted during PR's run versus what actually happened when it landed on nightly. |
Trying to reproduce regressions locally in cachegrind led to a few improvements -- and the new significance algorithm puts them close to the significance threshold. So that seems consistent with the original results, and the new ones are probably closer to noise than actual meaningful changes. Marking as triaged; doesn't seem important to investigate further. |
Walking the HIR tree is done using the
hir_crate
query. However, this is unnecessary, sincehir_owner(CRATE_DEF_ID)
provides the same information. Since depending onhir_crate
forces dependents to always be executed, this leads to unnecessary work.By splitting HIR and attributes visits, we can avoid an edge to
hir_crate
when trying to visit the HIR tree.