Auto merge of #119238 - Mark-Simulacrum:def-hash-efficiency, r=<try>

Specialize DefPathHash table to skip encoding crate IDs

The current implementation is ad-hoc and likely should be replaced with a non-table based approach (i.e., fully pulling out DefPathHash from the rmeta table infrastructure, of which we use ~none now), but this was an easy way to get an initial PR out.

The main pending question is whether the assumption made here that there is exactly one shared prefix accurate? If not, is it right that the number should be typically small? (If so a deduplication scheme of which this is a special case almost certainly makes sense).

We encode a lot of these (1000s) so the savings of 8 bytes/hash add up quickly. Opening this PR to get opinions more on the general idea and to run perf on whether the underlying impl will perform OK.

Author: bors

compiler/rustc_metadata/src/rmeta/decoder.rs

@@ -6,6 +6,7 @@ use crate::rmeta::*;
 
 use rustc_ast as ast;
 use rustc_data_structures::captures::Captures;
+use rustc_data_structures::fingerprint::Fingerprint;
 use rustc_data_structures::owned_slice::OwnedSlice;
 use rustc_data_structures::sync::{AppendOnlyVec, AtomicBool, Lock, Lrc, OnceLock};
 use rustc_data_structures::unhash::UnhashMap;
@@ -1489,9 +1490,16 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
         index: DefIndex,
         def_path_hashes: &mut FxHashMap<DefIndex, DefPathHash>,
     ) -> DefPathHash {
-        *def_path_hashes
-            .entry(index)
-            .or_insert_with(|| self.root.tables.def_path_hashes.get(self, index))
+        *def_path_hashes.entry(index).or_insert_with(|| {
+            // This is a hack to workaround the fact that we can't easily encode/decode a Hash64
+            // into the FixedSizeEncoding, as Hash64 lacks a Default impl. A future refactor to
+            // relax the Default restriction will likely fix this.
+            let fingerprint = Fingerprint::new(
+                self.root.stable_crate_id.as_u64(),
+                self.root.tables.def_path_hashes.get(self, index),
+            );
+            DefPathHash::new(self.root.stable_crate_id, fingerprint.split().1)
+        })
     }
 
     #[inline]

compiler/rustc_metadata/src/rmeta/encoder.rs

@@ -467,13 +467,17 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
                 let def_key = self.lazy(table.def_key(def_index));
                 let def_path_hash = table.def_path_hash(def_index);
                 self.tables.def_keys.set_some(def_index, def_key);
-                self.tables.def_path_hashes.set(def_index, def_path_hash);
+                assert_eq!(
+                    def_path_hash.stable_crate_id(),
+                    self.tcx.def_path_hash(LOCAL_CRATE.as_def_id()).stable_crate_id()
+                );
+                self.tables.def_path_hashes.set(def_index, def_path_hash.local_hash().as_u64());
             }
         } else {
             for (def_index, def_key, def_path_hash) in table.enumerated_keys_and_path_hashes() {
                 let def_key = self.lazy(def_key);
                 self.tables.def_keys.set_some(def_index, def_key);
-                self.tables.def_path_hashes.set(def_index, *def_path_hash);
+                self.tables.def_path_hashes.set(def_index, def_path_hash.local_hash().as_u64());
             }
         }
     }

compiler/rustc_metadata/src/rmeta/mod.rs

@@ -386,7 +386,14 @@ define_tables! {
     is_type_alias_impl_trait: Table<DefIndex, bool>,
     type_alias_is_lazy: Table<DefIndex, bool>,
     attr_flags: Table<DefIndex, AttrFlags>,
-    def_path_hashes: Table<DefIndex, DefPathHash>,
+    // The u64 is the crate-local part of the DefPathHash. All hashes in this crate have the same
+    // StableCrateId, so we omit encoding those into the table.
+    //
+    // N.B. this means that we can't distinguish between non-present items and a present but zero
+    // local hash item. In practice the compiler shouldn't care about non-present items in a foreign
+    // crate, so this should be OK. If we do start to care we should most likely adjust our hashing
+    // to reserve a bit (e.g., NonZeroU64).
+    def_path_hashes: Table<DefIndex, u64>,
     explicit_item_bounds: Table<DefIndex, LazyArray<(ty::Clause<'static>, Span)>>,
     inferred_outlives_of: Table<DefIndex, LazyArray<(ty::Clause<'static>, Span)>>,
     inherent_impls: Table<DefIndex, LazyArray<DefIndex>>,

compiler/rustc_metadata/src/rmeta/table.rs

@@ -1,6 +1,5 @@
 use crate::rmeta::*;
 
-use rustc_data_structures::fingerprint::Fingerprint;
 use rustc_hir::def::CtorOf;
 use rustc_index::Idx;
 
@@ -44,12 +43,6 @@ impl<T> IsDefault for LazyArray<T> {
     }
 }
 
-impl IsDefault for DefPathHash {
-    fn is_default(&self) -> bool {
-        self.0 == Fingerprint::ZERO
-    }
-}
-
 impl IsDefault for UnusedGenericParams {
     fn is_default(&self) -> bool {
         // UnusedGenericParams encodes the *un*usedness as a bitset.
@@ -234,22 +227,6 @@ fixed_size_enum! {
     }
 }
 
-// We directly encode `DefPathHash` because a `LazyValue` would incur a 25% cost.
-impl FixedSizeEncoding for DefPathHash {
-    type ByteArray = [u8; 16];
-
-    #[inline]
-    fn from_bytes(b: &[u8; 16]) -> Self {
-        DefPathHash(Fingerprint::from_le_bytes(*b))
-    }
-
-    #[inline]
-    fn write_to_bytes(self, b: &mut [u8; 16]) {
-        debug_assert!(!self.is_default());
-        *b = self.0.to_le_bytes();
-    }
-}
-
 // We directly encode RawDefId because using a `LazyValue` would incur a 50% overhead in the worst case.
 impl FixedSizeEncoding for Option<RawDefId> {
     type ByteArray = [u8; 8];

compiler/rustc_span/src/def_id.rs

@@ -114,8 +114,6 @@ impl DefPathHash {
     }
 
     /// Returns the crate-local part of the [DefPathHash].
-    ///
-    /// Used for tests.
     #[inline]
     pub fn local_hash(&self) -> Hash64 {
         self.0.split().1