Skip to content

Commit bd2706d

Browse files
committed
Auto merge of rust-lang#119265 - Mark-Simulacrum:remove-cache, r=<try>
Remove metadata decoding DefPathHash cache My expectation is that this cache is largely useless. Decoding a DefPathHash from metadata is essentially a pair of memory loads - there's no heavyweight processing involved. Caching it behind a HashMap just adds extra cost and incurs hashing overheads for the indices. Based on rust-lang#119238.
2 parents 5eccfc3 + f098f34 commit bd2706d

File tree

5 files changed

+17
-43
lines changed

5 files changed

+17
-43
lines changed

compiler/rustc_metadata/src/rmeta/decoder.rs

+9-15
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use crate::rmeta::*;
66

77
use rustc_ast as ast;
88
use rustc_data_structures::captures::Captures;
9+
use rustc_data_structures::fingerprint::Fingerprint;
910
use rustc_data_structures::owned_slice::OwnedSlice;
1011
use rustc_data_structures::sync::{AppendOnlyVec, AtomicBool, Lock, Lrc, OnceLock};
1112
use rustc_data_structures::unhash::UnhashMap;
@@ -87,8 +88,6 @@ pub(crate) struct CrateMetadata {
8788
alloc_decoding_state: AllocDecodingState,
8889
/// Caches decoded `DefKey`s.
8990
def_key_cache: Lock<FxHashMap<DefIndex, DefKey>>,
90-
/// Caches decoded `DefPathHash`es.
91-
def_path_hash_cache: Lock<FxHashMap<DefIndex, DefPathHash>>,
9291

9392
// --- Other significant crate properties ---
9493
/// ID of this crate, from the current compilation session's point of view.
@@ -1484,20 +1483,16 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
14841483
DefPath::make(self.cnum, id, |parent| self.def_key(parent))
14851484
}
14861485

1487-
fn def_path_hash_unlocked(
1488-
self,
1489-
index: DefIndex,
1490-
def_path_hashes: &mut FxHashMap<DefIndex, DefPathHash>,
1491-
) -> DefPathHash {
1492-
*def_path_hashes
1493-
.entry(index)
1494-
.or_insert_with(|| self.root.tables.def_path_hashes.get(self, index))
1495-
}
1496-
14971486
#[inline]
14981487
fn def_path_hash(self, index: DefIndex) -> DefPathHash {
1499-
let mut def_path_hashes = self.def_path_hash_cache.lock();
1500-
self.def_path_hash_unlocked(index, &mut def_path_hashes)
1488+
// This is a hack to workaround the fact that we can't easily encode/decode a Hash64
1489+
// into the FixedSizeEncoding, as Hash64 lacks a Default impl. A future refactor to
1490+
// relax the Default restriction will likely fix this.
1491+
let fingerprint = Fingerprint::new(
1492+
self.root.stable_crate_id.as_u64(),
1493+
self.root.tables.def_path_hashes.get(self, index),
1494+
);
1495+
DefPathHash::new(self.root.stable_crate_id, fingerprint.split().1)
15011496
}
15021497

15031498
#[inline]
@@ -1824,7 +1819,6 @@ impl CrateMetadata {
18241819
extern_crate: Lock::new(None),
18251820
hygiene_context: Default::default(),
18261821
def_key_cache: Default::default(),
1827-
def_path_hash_cache: Default::default(),
18281822
};
18291823

18301824
// Need `CrateMetadataRef` to decode `DefId`s in simplified types.

compiler/rustc_metadata/src/rmeta/encoder.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -467,13 +467,13 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
467467
let def_key = self.lazy(table.def_key(def_index));
468468
let def_path_hash = table.def_path_hash(def_index);
469469
self.tables.def_keys.set_some(def_index, def_key);
470-
self.tables.def_path_hashes.set(def_index, def_path_hash);
470+
self.tables.def_path_hashes.set(def_index, def_path_hash.local_hash().as_u64());
471471
}
472472
} else {
473473
for (def_index, def_key, def_path_hash) in table.enumerated_keys_and_path_hashes() {
474474
let def_key = self.lazy(def_key);
475475
self.tables.def_keys.set_some(def_index, def_key);
476-
self.tables.def_path_hashes.set(def_index, *def_path_hash);
476+
self.tables.def_path_hashes.set(def_index, def_path_hash.local_hash().as_u64());
477477
}
478478
}
479479
}

compiler/rustc_metadata/src/rmeta/mod.rs

+6-1
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,12 @@ define_tables! {
386386
is_type_alias_impl_trait: Table<DefIndex, bool>,
387387
type_alias_is_lazy: Table<DefIndex, bool>,
388388
attr_flags: Table<DefIndex, AttrFlags>,
389-
def_path_hashes: Table<DefIndex, DefPathHash>,
389+
// The u64 is the crate-local part of the DefPathHash. All hashes in this crate have the same
390+
// StableCrateId, so we omit encoding those into the table.
391+
//
392+
// Note also that this table is fully populated (no gaps) as every DefIndex should have a
393+
// corresponding DefPathHash.
394+
def_path_hashes: Table<DefIndex, u64>,
390395
explicit_item_bounds: Table<DefIndex, LazyArray<(ty::Clause<'static>, Span)>>,
391396
inferred_outlives_of: Table<DefIndex, LazyArray<(ty::Clause<'static>, Span)>>,
392397
inherent_impls: Table<DefIndex, LazyArray<DefIndex>>,

compiler/rustc_metadata/src/rmeta/table.rs

-23
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
use crate::rmeta::*;
22

3-
use rustc_data_structures::fingerprint::Fingerprint;
43
use rustc_hir::def::CtorOf;
54
use rustc_index::Idx;
65

@@ -44,12 +43,6 @@ impl<T> IsDefault for LazyArray<T> {
4443
}
4544
}
4645

47-
impl IsDefault for DefPathHash {
48-
fn is_default(&self) -> bool {
49-
self.0 == Fingerprint::ZERO
50-
}
51-
}
52-
5346
impl IsDefault for UnusedGenericParams {
5447
fn is_default(&self) -> bool {
5548
// UnusedGenericParams encodes the *un*usedness as a bitset.
@@ -234,22 +227,6 @@ fixed_size_enum! {
234227
}
235228
}
236229

237-
// We directly encode `DefPathHash` because a `LazyValue` would incur a 25% cost.
238-
impl FixedSizeEncoding for DefPathHash {
239-
type ByteArray = [u8; 16];
240-
241-
#[inline]
242-
fn from_bytes(b: &[u8; 16]) -> Self {
243-
DefPathHash(Fingerprint::from_le_bytes(*b))
244-
}
245-
246-
#[inline]
247-
fn write_to_bytes(self, b: &mut [u8; 16]) {
248-
debug_assert!(!self.is_default());
249-
*b = self.0.to_le_bytes();
250-
}
251-
}
252-
253230
// We directly encode RawDefId because using a `LazyValue` would incur a 50% overhead in the worst case.
254231
impl FixedSizeEncoding for Option<RawDefId> {
255232
type ByteArray = [u8; 8];

compiler/rustc_span/src/def_id.rs

-2
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,6 @@ impl DefPathHash {
114114
}
115115

116116
/// Returns the crate-local part of the [DefPathHash].
117-
///
118-
/// Used for tests.
119117
#[inline]
120118
pub fn local_hash(&self) -> Hash64 {
121119
self.0.split().1

0 commit comments

Comments
 (0)