Skip to content

Commit

Permalink
Auto merge of #83114 - cjgillot:hop, r=eddyb
Browse files Browse the repository at this point in the history
Move HIR parenting information out of hir_owner

Split out of #82681.

The parent of a HIR node and its content are currently bundled together, but are rarely used together.
This PR separates both information in two distinct queries for HIR owners.
This reduces incremental invalidation for HIR items that appear within a function body when this body (and the local ids) changes.
  • Loading branch information
bors committed May 1, 2021
2 parents 5f304a5 + d794cb0 commit 6e2a344
Show file tree
Hide file tree
Showing 12 changed files with 289 additions and 273 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_ast_lowering/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
let mut generic_args = vec![];
for (idx, arg) in args.into_iter().enumerate() {
if legacy_args_idx.contains(&idx) {
let parent_def_id = self.current_hir_id_owner.last().unwrap().0;
let parent_def_id = self.current_hir_id_owner.0;
let node_id = self.resolver.next_node_id();

// Add a definition for the in-band const def.
Expand Down
26 changes: 10 additions & 16 deletions compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ struct LoweringContext<'a, 'hir: 'a> {

type_def_lifetime_params: DefIdMap<usize>,

current_hir_id_owner: Vec<(LocalDefId, u32)>,
current_hir_id_owner: (LocalDefId, u32),
item_local_id_counters: NodeMap<u32>,
node_id_to_hir_id: IndexVec<NodeId, Option<hir::HirId>>,

Expand Down Expand Up @@ -321,7 +321,7 @@ pub fn lower_crate<'a, 'hir>(
anonymous_lifetime_mode: AnonymousLifetimeMode::PassThrough,
type_def_lifetime_params: Default::default(),
current_module: CRATE_DEF_ID,
current_hir_id_owner: vec![(CRATE_DEF_ID, 0)],
current_hir_id_owner: (CRATE_DEF_ID, 0),
item_local_id_counters: Default::default(),
node_id_to_hir_id: IndexVec::new(),
generator_kind: None,
Expand Down Expand Up @@ -594,9 +594,10 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
.insert(owner, HIR_ID_COUNTER_LOCKED)
.unwrap_or_else(|| panic!("no `item_local_id_counters` entry for {:?}", owner));
let def_id = self.resolver.local_def_id(owner);
self.current_hir_id_owner.push((def_id, counter));
let old_owner = std::mem::replace(&mut self.current_hir_id_owner, (def_id, counter));
let ret = f(self);
let (new_def_id, new_counter) = self.current_hir_id_owner.pop().unwrap();
let (new_def_id, new_counter) =
std::mem::replace(&mut self.current_hir_id_owner, old_owner);

debug_assert!(def_id == new_def_id);
debug_assert!(new_counter >= counter);
Expand All @@ -614,8 +615,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
/// properly. Calling the method twice with the same `NodeId` is fine though.
fn lower_node_id(&mut self, ast_node_id: NodeId) -> hir::HirId {
self.lower_node_id_generic(ast_node_id, |this| {
let &mut (owner, ref mut local_id_counter) =
this.current_hir_id_owner.last_mut().unwrap();
let &mut (owner, ref mut local_id_counter) = &mut this.current_hir_id_owner;
let local_id = *local_id_counter;
*local_id_counter += 1;
hir::HirId { owner, local_id: hir::ItemLocalId::from_u32(local_id) }
Expand Down Expand Up @@ -868,10 +868,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
// wouldn't have been added yet.
let generics = this.lower_generics_mut(
generics,
ImplTraitContext::Universal(
&mut params,
this.current_hir_id_owner.last().unwrap().0,
),
ImplTraitContext::Universal(&mut params, this.current_hir_id_owner.0),
);
let res = f(this, &mut params);
(params, (generics, res))
Expand Down Expand Up @@ -1077,7 +1074,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
}
AssocTyConstraintKind::Bound { ref bounds } => {
let mut capturable_lifetimes;
let mut parent_def_id = self.current_hir_id_owner.last().unwrap().0;
let mut parent_def_id = self.current_hir_id_owner.0;
// Piggy-back on the `impl Trait` context to figure out the correct behavior.
let (desugar_to_impl_trait, itctx) = match itctx {
// We are in the return position:
Expand Down Expand Up @@ -1198,7 +1195,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {

// Construct a AnonConst where the expr is the "ty"'s path.

let parent_def_id = self.current_hir_id_owner.last().unwrap().0;
let parent_def_id = self.current_hir_id_owner.0;
let node_id = self.resolver.next_node_id();

// Add a definition for the in-band const def.
Expand Down Expand Up @@ -1814,10 +1811,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
if let Some((_, ibty)) = &mut in_band_ty_params {
this.lower_ty_direct(
&param.ty,
ImplTraitContext::Universal(
ibty,
this.current_hir_id_owner.last().unwrap().0,
),
ImplTraitContext::Universal(ibty, this.current_hir_id_owner.0),
)
} else {
this.lower_ty_direct(&param.ty, ImplTraitContext::disallowed())
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/arena.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ macro_rules! arena_types {
[] predicates: rustc_middle::ty::PredicateInner<$tcx>,

// HIR query types
[few] indexed_hir: rustc_middle::hir::map::IndexedHir<$tcx>,
[few] indexed_hir: rustc_middle::hir::IndexedHir<$tcx>,
[few] hir_definitions: rustc_hir::definitions::Definitions,
[] hir_owner: rustc_middle::hir::Owner<$tcx>,
[] hir_owner_nodes: rustc_middle::hir::OwnerNodes<$tcx>,
Expand Down
139 changes: 46 additions & 93 deletions compiler/rustc_middle/src/hir/map/collector.rs
Original file line number Diff line number Diff line change
@@ -1,22 +1,20 @@
use crate::arena::Arena;
use crate::hir::map::{Entry, HirOwnerData, Map};
use crate::hir::{Owner, OwnerNodes, ParentedNode};
use crate::hir::map::{HirOwnerData, Map};
use crate::hir::{IndexedHir, Owner, OwnerNodes, ParentedNode};
use crate::ich::StableHashingContext;
use crate::middle::cstore::CrateStore;
use rustc_data_structures::fingerprint::Fingerprint;
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use rustc_data_structures::svh::Svh;
use rustc_hir as hir;
use rustc_hir::def_id::LocalDefId;
use rustc_hir::def_id::CRATE_DEF_INDEX;
use rustc_hir::def_id::{LocalDefId, LOCAL_CRATE};
use rustc_hir::definitions::{self, DefPathHash};
use rustc_hir::definitions;
use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
use rustc_hir::*;
use rustc_index::vec::{Idx, IndexVec};
use rustc_session::{CrateDisambiguator, Session};
use rustc_session::Session;
use rustc_span::source_map::SourceMap;
use rustc_span::{Span, Symbol, DUMMY_SP};
use rustc_span::{Span, DUMMY_SP};

use std::iter::repeat;

Expand All @@ -31,6 +29,7 @@ pub(super) struct NodeCollector<'a, 'hir> {
source_map: &'a SourceMap,

map: IndexVec<LocalDefId, HirOwnerData<'hir>>,
parenting: FxHashMap<LocalDefId, HirId>,

/// The parent of this node
parent_node: hir::HirId,
Expand All @@ -40,10 +39,6 @@ pub(super) struct NodeCollector<'a, 'hir> {
definitions: &'a definitions::Definitions,

hcx: StableHashingContext<'a>,

// We are collecting HIR hashes here so we can compute the
// crate hash from them later on.
hir_body_nodes: Vec<(DefPathHash, Fingerprint)>,
}

fn insert_vec_map<K: Idx, V: Clone>(map: &mut IndexVec<K, Option<V>>, k: K, v: V) {
Expand All @@ -58,34 +53,20 @@ fn insert_vec_map<K: Idx, V: Clone>(map: &mut IndexVec<K, Option<V>>, k: K, v: V

fn hash_body(
hcx: &mut StableHashingContext<'_>,
def_path_hash: DefPathHash,
item_like: impl for<'a> HashStable<StableHashingContext<'a>>,
hir_body_nodes: &mut Vec<(DefPathHash, Fingerprint)>,
) -> Fingerprint {
let hash = {
let mut stable_hasher = StableHasher::new();
hcx.while_hashing_hir_bodies(true, |hcx| {
item_like.hash_stable(hcx, &mut stable_hasher);
});
stable_hasher.finish()
};
hir_body_nodes.push((def_path_hash, hash));
hash
let mut stable_hasher = StableHasher::new();
hcx.while_hashing_hir_bodies(true, |hcx| {
item_like.hash_stable(hcx, &mut stable_hasher);
});
stable_hasher.finish()
}

fn upstream_crates(cstore: &dyn CrateStore) -> Vec<(Symbol, Fingerprint, Svh)> {
let mut upstream_crates: Vec<_> = cstore
.crates_untracked()
.iter()
.map(|&cnum| {
let name = cstore.crate_name_untracked(cnum);
let disambiguator = cstore.crate_disambiguator_untracked(cnum).to_fingerprint();
let hash = cstore.crate_hash_untracked(cnum);
(name, disambiguator, hash)
})
.collect();
upstream_crates.sort_unstable_by_key(|&(name, dis, _)| (name.as_str(), dis));
upstream_crates
/// Represents an entry and its parent `HirId`.
#[derive(Copy, Clone, Debug)]
pub struct Entry<'hir> {
parent: HirId,
node: Node<'hir>,
}

impl<'a, 'hir> NodeCollector<'a, 'hir> {
Expand All @@ -96,11 +77,6 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
definitions: &'a definitions::Definitions,
mut hcx: StableHashingContext<'a>,
) -> NodeCollector<'a, 'hir> {
let root_mod_def_path_hash =
definitions.def_path_hash(LocalDefId { local_def_index: CRATE_DEF_INDEX });

let mut hir_body_nodes = Vec::new();

let hash = {
let Crate {
ref item,
Expand All @@ -120,7 +96,7 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
attrs: _,
} = *krate;

hash_body(&mut hcx, root_mod_def_path_hash, item, &mut hir_body_nodes)
hash_body(&mut hcx, item)
};

let mut collector = NodeCollector {
Expand All @@ -131,10 +107,10 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
current_dep_node_owner: LocalDefId { local_def_index: CRATE_DEF_INDEX },
definitions,
hcx,
hir_body_nodes,
map: (0..definitions.def_index_count())
.map(|_| HirOwnerData { signature: None, with_bodies: None })
.collect(),
parenting: FxHashMap::default(),
};
collector.insert_entry(
hir::CRATE_HIR_ID,
Expand All @@ -145,55 +121,13 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
collector
}

pub(super) fn finalize_and_compute_crate_hash(
mut self,
crate_disambiguator: CrateDisambiguator,
cstore: &dyn CrateStore,
commandline_args_hash: u64,
) -> (IndexVec<LocalDefId, HirOwnerData<'hir>>, Svh) {
pub(super) fn finalize_and_compute_crate_hash(mut self) -> IndexedHir<'hir> {
// Insert bodies into the map
for (id, body) in self.krate.bodies.iter() {
let bodies = &mut self.map[id.hir_id.owner].with_bodies.as_mut().unwrap().bodies;
assert!(bodies.insert(id.hir_id.local_id, body).is_none());
}

self.hir_body_nodes.sort_unstable_by_key(|bn| bn.0);

let node_hashes = self.hir_body_nodes.iter().fold(
Fingerprint::ZERO,
|combined_fingerprint, &(def_path_hash, fingerprint)| {
combined_fingerprint.combine(def_path_hash.0.combine(fingerprint))
},
);

let upstream_crates = upstream_crates(cstore);

// We hash the final, remapped names of all local source files so we
// don't have to include the path prefix remapping commandline args.
// If we included the full mapping in the SVH, we could only have
// reproducible builds by compiling from the same directory. So we just
// hash the result of the mapping instead of the mapping itself.
let mut source_file_names: Vec<_> = self
.source_map
.files()
.iter()
.filter(|source_file| source_file.cnum == LOCAL_CRATE)
.map(|source_file| source_file.name_hash)
.collect();

source_file_names.sort_unstable();

let crate_hash_input = (
((node_hashes, upstream_crates), source_file_names),
(commandline_args_hash, crate_disambiguator.to_fingerprint()),
);

let mut stable_hasher = StableHasher::new();
crate_hash_input.hash_stable(&mut self.hcx, &mut stable_hasher);
let crate_hash: Fingerprint = stable_hasher.finish();

let svh = Svh::new(crate_hash.to_smaller_hash());
(self.map, svh)
IndexedHir { map: self.map, parenting: self.parenting }
}

fn insert_entry(&mut self, id: HirId, entry: Entry<'hir>, hash: Fingerprint) {
Expand All @@ -218,8 +152,7 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
nodes.hash = hash;

debug_assert!(data.signature.is_none());
data.signature =
Some(self.arena.alloc(Owner { parent: entry.parent, node: entry.node }));
data.signature = Some(self.arena.alloc(Owner { node: entry.node }));

let dk_parent = self.definitions.def_key(id.owner).parent;
if let Some(dk_parent) = dk_parent {
Expand All @@ -231,6 +164,8 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
id.owner, dk_parent, entry.parent,
)
}

debug_assert_eq!(self.parenting.get(&id.owner), Some(&entry.parent));
}
} else {
assert_eq!(entry.parent.owner, id.owner);
Expand Down Expand Up @@ -294,15 +229,28 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
f: F,
) {
let prev_owner = self.current_dep_node_owner;

let def_path_hash = self.definitions.def_path_hash(dep_node_owner);

let hash = hash_body(&mut self.hcx, def_path_hash, item_like, &mut self.hir_body_nodes);
let hash = hash_body(&mut self.hcx, item_like);

self.current_dep_node_owner = dep_node_owner;
f(self, hash);
self.current_dep_node_owner = prev_owner;
}

fn insert_nested(&mut self, item: LocalDefId) {
#[cfg(debug_assertions)]
{
let dk_parent = self.definitions.def_key(item).parent.unwrap();
let dk_parent = LocalDefId { local_def_index: dk_parent };
let dk_parent = self.definitions.local_def_id_to_hir_id(dk_parent);
debug_assert_eq!(
dk_parent.owner, self.parent_node.owner,
"Different parents for {:?}",
item
)
}

assert_eq!(self.parenting.insert(item, self.parent_node), None);
}
}

impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
Expand All @@ -318,18 +266,22 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {

fn visit_nested_item(&mut self, item: ItemId) {
debug!("visit_nested_item: {:?}", item);
self.insert_nested(item.def_id);
self.visit_item(self.krate.item(item));
}

fn visit_nested_trait_item(&mut self, item_id: TraitItemId) {
self.insert_nested(item_id.def_id);
self.visit_trait_item(self.krate.trait_item(item_id));
}

fn visit_nested_impl_item(&mut self, item_id: ImplItemId) {
self.insert_nested(item_id.def_id);
self.visit_impl_item(self.krate.impl_item(item_id));
}

fn visit_nested_foreign_item(&mut self, foreign_id: ForeignItemId) {
self.insert_nested(foreign_id.def_id);
self.visit_foreign_item(self.krate.foreign_item(foreign_id));
}

Expand Down Expand Up @@ -517,6 +469,7 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
self.definitions.local_def_id_to_hir_id(LocalDefId { local_def_index })
});
self.with_parent(parent, |this| {
this.insert_nested(macro_def.def_id);
this.with_dep_node_owner(macro_def.def_id, macro_def, |this, hash| {
this.insert_with_hash(
macro_def.span,
Expand Down
Loading

0 comments on commit 6e2a344

Please sign in to comment.