From 027fa9e23b6cc66a1ea9a88479dadf4056a10430 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 23 Dec 2023 17:33:11 +0000 Subject: [PATCH 1/3] Only store StableCrateId once in DefPathTable. --- compiler/rustc_hir/src/definitions.rs | 37 ++++++++++++++++----------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_hir/src/definitions.rs b/compiler/rustc_hir/src/definitions.rs index 2ab9a6ef32ce5..46c39110992bd 100644 --- a/compiler/rustc_hir/src/definitions.rs +++ b/compiler/rustc_hir/src/definitions.rs @@ -20,22 +20,36 @@ use std::hash::Hash; /// Internally the `DefPathTable` holds a tree of `DefKey`s, where each `DefKey` /// stores the `DefIndex` of its parent. /// There is one `DefPathTable` for each crate. -#[derive(Clone, Default, Debug)] +#[derive(Debug)] pub struct DefPathTable { + stable_crate_id: StableCrateId, index_to_key: IndexVec, - def_path_hashes: IndexVec, + // We do only store the local hash, as all the definitions are from the current crate. + def_path_hashes: IndexVec, def_path_hash_to_index: DefPathHashMap, } impl DefPathTable { + fn new(stable_crate_id: StableCrateId) -> DefPathTable { + DefPathTable { + stable_crate_id, + index_to_key: Default::default(), + def_path_hashes: Default::default(), + def_path_hash_to_index: Default::default(), + } + } + fn allocate(&mut self, key: DefKey, def_path_hash: DefPathHash) -> DefIndex { + // Assert that all DefPathHashes correctly contain the local crate's StableCrateId. + debug_assert_eq!(self.stable_crate_id, def_path_hash.stable_crate_id()); + let index = { let index = DefIndex::from(self.index_to_key.len()); debug!("DefPathTable::insert() - {:?} <-> {:?}", key, index); self.index_to_key.push(key); index }; - self.def_path_hashes.push(def_path_hash); + self.def_path_hashes.push(def_path_hash.local_hash()); debug_assert!(self.def_path_hashes.len() == self.index_to_key.len()); // Check for hash collisions of DefPathHashes. These should be @@ -58,13 +72,6 @@ impl DefPathTable { ); } - // Assert that all DefPathHashes correctly contain the local crate's - // StableCrateId - #[cfg(debug_assertions)] - if let Some(root) = self.def_path_hashes.get(CRATE_DEF_INDEX) { - assert!(def_path_hash.stable_crate_id() == root.stable_crate_id()); - } - index } @@ -73,19 +80,19 @@ impl DefPathTable { self.index_to_key[index] } + #[instrument(level = "trace", skip(self), ret)] #[inline(always)] pub fn def_path_hash(&self, index: DefIndex) -> DefPathHash { let hash = self.def_path_hashes[index]; - debug!("def_path_hash({:?}) = {:?}", index, hash); - hash + DefPathHash::new(self.stable_crate_id, hash) } pub fn enumerated_keys_and_path_hashes( &self, - ) -> impl Iterator + ExactSizeIterator + '_ { + ) -> impl Iterator + ExactSizeIterator + '_ { self.index_to_key .iter_enumerated() - .map(move |(index, key)| (index, key, &self.def_path_hashes[index])) + .map(move |(index, key)| (index, key, self.def_path_hash(index))) } } @@ -329,7 +336,7 @@ impl Definitions { let def_path_hash = key.compute_stable_hash(parent_hash); // Create the root definition. - let mut table = DefPathTable::default(); + let mut table = DefPathTable::new(stable_crate_id); let root = LocalDefId { local_def_index: table.allocate(key, def_path_hash) }; assert_eq!(root.local_def_index, CRATE_DEF_INDEX); From 75ad81ce934123d71c9662fbbd5ca67ef507cbfb Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sun, 24 Dec 2023 01:06:58 +0000 Subject: [PATCH 2/3] Do not store stable_crate_id again in Definitions. --- compiler/rustc_hir/src/definitions.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_hir/src/definitions.rs b/compiler/rustc_hir/src/definitions.rs index 46c39110992bd..d6bbf183f3beb 100644 --- a/compiler/rustc_hir/src/definitions.rs +++ b/compiler/rustc_hir/src/definitions.rs @@ -103,9 +103,6 @@ impl DefPathTable { pub struct Definitions { table: DefPathTable, next_disambiguator: UnordMap<(LocalDefId, DefPathData), u32>, - - /// The [StableCrateId] of the local crate. - stable_crate_id: StableCrateId, } /// A unique identifier that we can use to lookup a definition @@ -340,7 +337,7 @@ impl Definitions { let root = LocalDefId { local_def_index: table.allocate(key, def_path_hash) }; assert_eq!(root.local_def_index, CRATE_DEF_INDEX); - Definitions { table, next_disambiguator: Default::default(), stable_crate_id } + Definitions { table, next_disambiguator: Default::default() } } /// Adds a definition with a parent definition. @@ -382,7 +379,7 @@ impl Definitions { hash: DefPathHash, err: &mut dyn FnMut() -> !, ) -> LocalDefId { - debug_assert!(hash.stable_crate_id() == self.stable_crate_id); + debug_assert!(hash.stable_crate_id() == self.table.stable_crate_id); self.table .def_path_hash_to_index .get(&hash) From 821920b2a3bb63a76233e8c1862409d51d85a5e9 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sun, 24 Dec 2023 12:41:35 +0000 Subject: [PATCH 3/3] Do not store stable crate id in on-disk hash map. --- compiler/rustc_data_structures/src/hashes.rs | 2 +- compiler/rustc_hir/src/def_path_hash_map.rs | 17 +++++++++-------- compiler/rustc_hir/src/definitions.rs | 7 ++++--- .../src/rmeta/def_path_hash_map.rs | 4 +++- 4 files changed, 17 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_data_structures/src/hashes.rs b/compiler/rustc_data_structures/src/hashes.rs index ad068cdbc9841..291ee5bbe26e3 100644 --- a/compiler/rustc_data_structures/src/hashes.rs +++ b/compiler/rustc_data_structures/src/hashes.rs @@ -25,7 +25,7 @@ impl Hash64 { pub const ZERO: Hash64 = Hash64 { inner: 0 }; #[inline] - pub(crate) fn new(n: u64) -> Self { + pub fn new(n: u64) -> Self { Self { inner: n } } diff --git a/compiler/rustc_hir/src/def_path_hash_map.rs b/compiler/rustc_hir/src/def_path_hash_map.rs index 8bfb47af26f1a..9a6dee1e511df 100644 --- a/compiler/rustc_hir/src/def_path_hash_map.rs +++ b/compiler/rustc_hir/src/def_path_hash_map.rs @@ -1,21 +1,22 @@ -use rustc_data_structures::fingerprint::Fingerprint; -use rustc_span::def_id::{DefIndex, DefPathHash}; +use rustc_data_structures::stable_hasher::Hash64; +use rustc_span::def_id::DefIndex; #[derive(Clone, Default)] pub struct Config; impl odht::Config for Config { - type Key = DefPathHash; + // This hash-map is single-crate, so we only need to key by the local hash. + type Key = Hash64; type Value = DefIndex; - type EncodedKey = [u8; 16]; + type EncodedKey = [u8; 8]; type EncodedValue = [u8; 4]; type H = odht::UnHashFn; #[inline] - fn encode_key(k: &DefPathHash) -> [u8; 16] { - k.0.to_le_bytes() + fn encode_key(k: &Hash64) -> [u8; 8] { + k.as_u64().to_le_bytes() } #[inline] @@ -24,8 +25,8 @@ impl odht::Config for Config { } #[inline] - fn decode_key(k: &[u8; 16]) -> DefPathHash { - DefPathHash(Fingerprint::from_le_bytes(*k)) + fn decode_key(k: &[u8; 8]) -> Hash64 { + Hash64::new(u64::from_le_bytes(*k)) } #[inline] diff --git a/compiler/rustc_hir/src/definitions.rs b/compiler/rustc_hir/src/definitions.rs index d6bbf183f3beb..9fb1fc19bf4e4 100644 --- a/compiler/rustc_hir/src/definitions.rs +++ b/compiler/rustc_hir/src/definitions.rs @@ -42,6 +42,7 @@ impl DefPathTable { fn allocate(&mut self, key: DefKey, def_path_hash: DefPathHash) -> DefIndex { // Assert that all DefPathHashes correctly contain the local crate's StableCrateId. debug_assert_eq!(self.stable_crate_id, def_path_hash.stable_crate_id()); + let local_hash = def_path_hash.local_hash(); let index = { let index = DefIndex::from(self.index_to_key.len()); @@ -49,12 +50,12 @@ impl DefPathTable { self.index_to_key.push(key); index }; - self.def_path_hashes.push(def_path_hash.local_hash()); + self.def_path_hashes.push(local_hash); debug_assert!(self.def_path_hashes.len() == self.index_to_key.len()); // Check for hash collisions of DefPathHashes. These should be // exceedingly rare. - if let Some(existing) = self.def_path_hash_to_index.insert(&def_path_hash, &index) { + if let Some(existing) = self.def_path_hash_to_index.insert(&local_hash, &index) { let def_path1 = DefPath::make(LOCAL_CRATE, existing, |idx| self.def_key(idx)); let def_path2 = DefPath::make(LOCAL_CRATE, index, |idx| self.def_key(idx)); @@ -382,7 +383,7 @@ impl Definitions { debug_assert!(hash.stable_crate_id() == self.table.stable_crate_id); self.table .def_path_hash_to_index - .get(&hash) + .get(&hash.local_hash()) .map(|local_def_index| LocalDefId { local_def_index }) .unwrap_or_else(|| err()) } diff --git a/compiler/rustc_metadata/src/rmeta/def_path_hash_map.rs b/compiler/rustc_metadata/src/rmeta/def_path_hash_map.rs index 4f280bb9d80e2..9950bc1c31f70 100644 --- a/compiler/rustc_metadata/src/rmeta/def_path_hash_map.rs +++ b/compiler/rustc_metadata/src/rmeta/def_path_hash_map.rs @@ -19,7 +19,9 @@ impl DefPathHashMapRef<'_> { #[inline] pub fn def_path_hash_to_def_index(&self, def_path_hash: &DefPathHash) -> DefIndex { match *self { - DefPathHashMapRef::OwnedFromMetadata(ref map) => map.get(def_path_hash).unwrap(), + DefPathHashMapRef::OwnedFromMetadata(ref map) => { + map.get(&def_path_hash.local_hash()).unwrap() + } DefPathHashMapRef::BorrowedFromTcx(_) => { panic!("DefPathHashMap::BorrowedFromTcx variant only exists for serialization") }