-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Add a cache for maybe_lint_level_root_bounded
#113609
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -90,8 +90,8 @@ use rustc_index::{IndexSlice, IndexVec}; | |
use rustc_middle::middle::region; | ||
use rustc_middle::mir::*; | ||
use rustc_middle::thir::{Expr, LintLevel}; | ||
|
||
use rustc_middle::ty::Ty; | ||
use rustc_session::lint::Level; | ||
use rustc_span::{Span, DUMMY_SP}; | ||
|
||
#[derive(Debug)] | ||
|
@@ -760,20 +760,25 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { | |
) { | ||
let (current_root, parent_root) = | ||
if self.tcx.sess.opts.unstable_opts.maximal_hir_to_mir_coverage { | ||
// Some consumers of rustc need to map MIR locations back to HIR nodes. Currently the | ||
// the only part of rustc that tracks MIR -> HIR is the `SourceScopeLocalData::lint_root` | ||
// field that tracks lint levels for MIR locations. Normally the number of source scopes | ||
// is limited to the set of nodes with lint annotations. The -Zmaximal-hir-to-mir-coverage | ||
// flag changes this behavior to maximize the number of source scopes, increasing the | ||
// granularity of the MIR->HIR mapping. | ||
// Some consumers of rustc need to map MIR locations back to HIR nodes. Currently | ||
// the the only part of rustc that tracks MIR -> HIR is the | ||
// `SourceScopeLocalData::lint_root` field that tracks lint levels for MIR | ||
// locations. Normally the number of source scopes is limited to the set of nodes | ||
// with lint annotations. The -Zmaximal-hir-to-mir-coverage flag changes this | ||
// behavior to maximize the number of source scopes, increasing the granularity of | ||
// the MIR->HIR mapping. | ||
(current_id, parent_id) | ||
} else { | ||
// Use `maybe_lint_level_root_bounded` with `self.hir_id` as a bound | ||
// to avoid adding Hir dependencies on our parents. | ||
// We estimate the true lint roots here to avoid creating a lot of source scopes. | ||
// Use `maybe_lint_level_root_bounded` to avoid adding Hir dependencies on our | ||
// parents. We estimate the true lint roots here to avoid creating a lot of source | ||
// scopes. | ||
( | ||
self.tcx.maybe_lint_level_root_bounded(current_id, self.hir_id), | ||
self.tcx.maybe_lint_level_root_bounded(parent_id, self.hir_id), | ||
self.maybe_lint_level_root_bounded(current_id), | ||
if parent_id == self.hir_id { | ||
parent_id // this is very common | ||
} else { | ||
self.maybe_lint_level_root_bounded(parent_id) | ||
}, | ||
) | ||
}; | ||
|
||
|
@@ -783,6 +788,50 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { | |
} | ||
} | ||
|
||
/// Walks upwards from `orig_id` to find a node which might change lint levels with attributes. | ||
/// It stops at `self.hir_id` and just returns it if reached. | ||
fn maybe_lint_level_root_bounded(&mut self, orig_id: HirId) -> HirId { | ||
// This assertion lets us just store `ItemLocalId` in the cache, rather | ||
// than the full `HirId`. | ||
assert_eq!(orig_id.owner, self.hir_id.owner); | ||
|
||
let mut id = orig_id; | ||
let hir = self.tcx.hir(); | ||
loop { | ||
if id == self.hir_id { | ||
// This is a moderately common case, mostly hit for previously unseen nodes. | ||
break; | ||
} | ||
|
||
if hir.attrs(id).iter().any(|attr| Level::from_attr(attr).is_some()) { | ||
// This is a rare case. It's for a node path that doesn't reach the root due to an | ||
// intervening lint level attribute. This result doesn't get cached. | ||
return id; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we eventually cache this too? If we have a whole HIR subtree that hits the same lint root, different than There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I originally did cache these values, using an |
||
} | ||
|
||
let next = hir.parent_id(id); | ||
if next == id { | ||
bug!("lint traversal reached the root of the crate"); | ||
} | ||
id = next; | ||
|
||
// This lookup is just an optimization; it can be removed without affecting | ||
// functionality. It might seem strange to see this at the end of this loop, but the | ||
// `orig_id` passed in to this function is almost always previously unseen, for which a | ||
// lookup will be a miss. So we only do lookups for nodes up the parent chain, where | ||
// cache lookups have a very high hit rate. | ||
if self.lint_level_roots_cache.contains(id.local_id) { | ||
break; | ||
} | ||
} | ||
|
||
// `orig_id` traced to `self_id`; record this fact. If `orig_id` is a leaf node it will | ||
// rarely (never?) subsequently be searched for, but it's hard to know if that is the case. | ||
// The performance wins from the cache all come from caching non-leaf nodes. | ||
self.lint_level_roots_cache.insert(orig_id.local_id); | ||
self.hir_id | ||
} | ||
|
||
/// Creates a new source scope, nested in the current one. | ||
pub(crate) fn new_source_scope( | ||
&mut self, | ||
|
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.
Should this be made a fast path inside
maybe_lint_level_root_bounded
?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.
id == self.hir_id
is already the first check withinmaybe_lint_level_root_bounded
. I added this pre-check here because (a) it's useful documentation, and (b) it gave a 0.4% instruction count win fordeep-vector
, due to avoiding the function call overhead.