Skip to content

Commit

Permalink
Add a cache for maybe_lint_level_root_bounded.
Browse files Browse the repository at this point in the history
It's a nice speed win.
  • Loading branch information
nnethercote committed Jul 12, 2023
1 parent f234dc3 commit 667d75e
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 10 deletions.
10 changes: 10 additions & 0 deletions compiler/rustc_mir_build/src/build/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use rustc_hir as hir;
use rustc_hir::def::DefKind;
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_hir::{GeneratorKind, Node};
use rustc_index::bit_set::GrowableBitSet;
use rustc_index::{Idx, IndexSlice, IndexVec};
use rustc_infer::infer::{InferCtxt, TyCtxtInferExt};
use rustc_middle::hir::place::PlaceBase as HirPlaceBase;
Expand Down Expand Up @@ -215,6 +216,14 @@ struct Builder<'a, 'tcx> {
unit_temp: Option<Place<'tcx>>,

var_debug_info: Vec<VarDebugInfo<'tcx>>,

// A cache for `maybe_lint_level_roots_bounded`. That function is called
// repeatedly, and each time it effectively traces a path through a tree
// structure from a node towards the root, doing an attribute check on each
// node along the way. This cache records which nodes trace all the way to
// the root (most of them do) and saves us from retracing many sub-paths
// many times, and rechecking many nodes.
lint_level_roots_cache: GrowableBitSet<hir::ItemLocalId>,
}

type CaptureMap<'tcx> = SortedIndexMultiMap<usize, hir::HirId, Capture<'tcx>>;
Expand Down Expand Up @@ -725,6 +734,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
var_indices: Default::default(),
unit_temp: None,
var_debug_info: vec![],
lint_level_roots_cache: GrowableBitSet::new_empty(),
};

assert_eq!(builder.cfg.start_new_block(), START_BLOCK);
Expand Down
47 changes: 37 additions & 10 deletions compiler/rustc_mir_build/src/build/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -769,12 +769,16 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// 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.maybe_lint_level_root_bounded(current_id, self.hir_id),
self.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)
},
)
};

Expand All @@ -784,16 +788,24 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
}
}

/// Walks upwards from `id` to find a node which might change lint levels with attributes.
/// It stops at `bound` and just returns it if reached.
fn maybe_lint_level_root_bounded(&self, mut id: HirId, bound: HirId) -> HirId {
/// 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 == bound {
return bound;
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;
}

Expand All @@ -802,7 +814,22 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
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.
Expand Down

0 comments on commit 667d75e

Please sign in to comment.