Skip to content

Commit

Permalink
Auto merge of rust-lang#119259 - cjgillot:single-crate-id, r=Mark-Sim…
Browse files Browse the repository at this point in the history
…ulacrum

Only store StableCrateId once in DefPathTable.

rust-lang#119238 made me think of this.

cc `@Mark-Simulacrum`
  • Loading branch information
bors committed Dec 29, 2023
2 parents 1a7e97f + 821920b commit dc450f9
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 32 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_data_structures/src/hashes.rs
Expand Up @@ -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 }
}

Expand Down
17 changes: 9 additions & 8 deletions 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]
Expand All @@ -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]
Expand Down
49 changes: 27 additions & 22 deletions compiler/rustc_hir/src/definitions.rs
Expand Up @@ -20,27 +20,42 @@ 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<DefIndex, DefKey>,
def_path_hashes: IndexVec<DefIndex, DefPathHash>,
// We do only store the local hash, as all the definitions are from the current crate.
def_path_hashes: IndexVec<DefIndex, Hash64>,
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 local_hash = def_path_hash.local_hash();

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(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));

Expand All @@ -58,13 +73,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
}

Expand All @@ -73,19 +81,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<Item = (DefIndex, &DefKey, &DefPathHash)> + ExactSizeIterator + '_ {
) -> impl Iterator<Item = (DefIndex, &DefKey, DefPathHash)> + 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)))
}
}

Expand All @@ -96,9 +104,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
Expand Down Expand Up @@ -329,11 +334,11 @@ 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);

Definitions { table, next_disambiguator: Default::default(), stable_crate_id }
Definitions { table, next_disambiguator: Default::default() }
}

/// Adds a definition with a parent definition.
Expand Down Expand Up @@ -375,10 +380,10 @@ 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)
.get(&hash.local_hash())
.map(|local_def_index| LocalDefId { local_def_index })
.unwrap_or_else(|| err())
}
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_metadata/src/rmeta/def_path_hash_map.rs
Expand Up @@ -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")
}
Expand Down

0 comments on commit dc450f9

Please sign in to comment.