Lazy decoding of DefPathTable from crate metadata (non-incremental case)

Author: Aaron1011

Diff for: src/librustc_hir/definitions.rs

@@ -23,7 +23,7 @@ use tracing::debug;
 /// 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, Decodable, Encodable)]
+#[derive(Clone, Default)]
 pub struct DefPathTable {
     index_to_key: IndexVec<DefIndex, DefKey>,
     def_path_hashes: IndexVec<DefIndex, DefPathHash>,
@@ -42,10 +42,6 @@ impl DefPathTable {
         index
     }
 
-    pub fn next_id(&self) -> DefIndex {
-        DefIndex::from(self.index_to_key.len())
-    }
-
     #[inline(always)]
     pub fn def_key(&self, index: DefIndex) -> DefKey {
         self.index_to_key[index]
@@ -58,15 +54,25 @@ impl DefPathTable {
         hash
     }
 
-    pub fn add_def_path_hashes_to(&self, cnum: CrateNum, out: &mut FxHashMap<DefPathHash, DefId>) {
-        out.extend(self.def_path_hashes.iter().enumerate().map(|(index, &hash)| {
-            let def_id = DefId { krate: cnum, index: DefIndex::from(index) };
-            (hash, def_id)
-        }));
+    pub fn num_def_ids(&self) -> usize {
+        self.index_to_key.len()
     }
 
-    pub fn size(&self) -> usize {
-        self.index_to_key.len()
+    pub fn enumerated_keys_and_path_hashes(
+        &self,
+    ) -> impl Iterator<Item = (DefIndex, &DefKey, &DefPathHash)> + '_ {
+        self.index_to_key
+            .iter_enumerated()
+            .map(move |(index, key)| (index, key, &self.def_path_hashes[index]))
+    }
+
+    pub fn all_def_path_hashes_and_def_ids(
+        &self,
+        krate: CrateNum,
+    ) -> impl Iterator<Item = (DefPathHash, DefId)> + '_ {
+        self.def_path_hashes
+            .iter_enumerated()
+            .map(move |(index, hash)| (*hash, DefId { krate, index }))
     }
 }
 

Diff for: src/librustc_metadata/rmeta/decoder.rs

@@ -16,7 +16,6 @@ use rustc_expand::proc_macro::{AttrProcMacro, BangProcMacro, ProcMacroDerive};
 use rustc_hir as hir;
 use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res};
 use rustc_hir::def_id::{CrateNum, DefId, DefIndex, CRATE_DEF_INDEX, LOCAL_CRATE};
-use rustc_hir::definitions::DefPathTable;
 use rustc_hir::definitions::{DefKey, DefPath, DefPathData, DefPathHash};
 use rustc_hir::lang_items;
 use rustc_index::vec::{Idx, IndexVec};
@@ -29,7 +28,6 @@ use rustc_middle::mir::interpret::{AllocDecodingSession, AllocDecodingState};
 use rustc_middle::mir::{self, Body, Promoted};
 use rustc_middle::ty::codec::TyDecoder;
 use rustc_middle::ty::{self, Ty, TyCtxt};
-use rustc_middle::util::common::record_time;
 use rustc_serialize::{opaque, Decodable, Decoder};
 use rustc_session::Session;
 use rustc_span::hygiene::ExpnDataDecodeMode;
@@ -69,12 +67,6 @@ crate struct CrateMetadata {
     /// universal (`for<'tcx>`), that is paired up with whichever `TyCtxt`
     /// is being used to decode those values.
     root: CrateRoot<'static>,
-    /// For each definition in this crate, we encode a key. When the
-    /// crate is loaded, we read all the keys and put them in this
-    /// hashmap, which gives the reverse mapping. This allows us to
-    /// quickly retrace a `DefPath`, which is needed for incremental
-    /// compilation support.
-    def_path_table: DefPathTable,
     /// Trait impl data.
     /// FIXME: Used only from queries and can use query cache,
     /// so pre-decoding can probably be avoided.
@@ -91,6 +83,10 @@ crate struct CrateMetadata {
     /// Do not access the value directly, as it might not have been initialized yet.
     /// The field must always be initialized to `DepNodeIndex::INVALID`.
     dep_node_index: AtomicCell<DepNodeIndex>,
+    /// Caches decoded `DefKey`s.
+    def_key_cache: Lock<FxHashMap<DefIndex, DefKey>>,
+    /// Caches decoded `DefPathHash`es.
+    def_path_hash_cache: Lock<FxHashMap<DefIndex, DefPathHash>>,
 
     // --- Other significant crate properties ---
     /// ID of this crate, from the current compilation session's point of view.
@@ -807,7 +803,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
                     data.has_auto_impl,
                     data.is_marker,
                     data.specialization_kind,
-                    self.def_path_table.def_path_hash(item_id),
+                    self.def_path_hash(item_id),
                 )
             }
             EntryKind::TraitAlias => ty::TraitDef::new(
@@ -817,7 +813,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
                 false,
                 false,
                 ty::trait_def::TraitSpecializationKind::None,
-                self.def_path_table.def_path_hash(item_id),
+                self.def_path_hash(item_id),
             ),
             _ => bug!("def-index does not refer to trait or trait alias"),
         }
@@ -1509,12 +1505,14 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
 
     #[inline]
     fn def_key(&self, index: DefIndex) -> DefKey {
-        let mut key = self.def_path_table.def_key(index);
-        if self.is_proc_macro(index) {
-            let name = self.raw_proc_macro(index).name();
-            key.disambiguated_data.data = DefPathData::MacroNs(Symbol::intern(name));
-        }
-        key
+        *self.def_key_cache.lock().entry(index).or_insert_with(|| {
+            let mut key = self.root.tables.def_keys.get(self, index).unwrap().decode(self);
+            if self.is_proc_macro(index) {
+                let name = self.raw_proc_macro(index).name();
+                key.disambiguated_data.data = DefPathData::MacroNs(Symbol::intern(name));
+            }
+            key
+        })
     }
 
     // Returns the path leading to the thing with this `id`.
@@ -1723,9 +1721,6 @@ impl CrateMetadata {
         private_dep: bool,
         host_hash: Option<Svh>,
     ) -> CrateMetadata {
-        let def_path_table = record_time(&sess.perf_stats.decode_def_path_tables_time, || {
-            root.def_path_table.decode((&blob, sess))
-        });
         let trait_impls = root
             .impls
             .decode((&blob, sess))
@@ -1737,7 +1732,6 @@ impl CrateMetadata {
         CrateMetadata {
             blob,
             root,
-            def_path_table,
             trait_impls,
             raw_proc_macros,
             source_map_import_info: OnceCell::new(),
@@ -1752,6 +1746,8 @@ impl CrateMetadata {
             host_hash,
             extern_crate: Lock::new(None),
             hygiene_context: Default::default(),
+            def_key_cache: Default::default(),
+            def_path_hash_cache: Default::default(),
         }
     }
 
@@ -1828,6 +1824,10 @@ impl CrateMetadata {
         self.root.hash
     }
 
+    fn num_def_ids(&self) -> usize {
+        self.root.tables.def_keys.size()
+    }
+
     fn local_def_id(&self, index: DefIndex) -> DefId {
         DefId { krate: self.cnum, index }
     }
@@ -1843,10 +1843,34 @@ impl CrateMetadata {
 
         None
     }
+}
+
+impl<'a, 'tcx> CrateMetadataRef<'a> {
+    fn def_path_hash_unlocked(
+        &self,
+        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).unwrap().decode(self)
+        })
+    }
 
     #[inline]
     fn def_path_hash(&self, index: DefIndex) -> DefPathHash {
-        self.def_path_table.def_path_hash(index)
+        let mut def_path_hashes = self.def_path_hash_cache.lock();
+        self.def_path_hash_unlocked(index, &mut def_path_hashes)
+    }
+
+    fn all_def_path_hashes_and_def_ids(&self) -> Vec<(DefPathHash, DefId)> {
+        let mut result = Vec::new();
+        let mut def_path_hashes = self.def_path_hash_cache.lock();
+        for index in 0..self.num_def_ids() {
+            let index = DefIndex::from_usize(index);
+            let def_path_hash = self.def_path_hash_unlocked(index, &mut def_path_hashes);
+            result.push((def_path_hash, self.local_def_id(index)));
+        }
+        result
     }
 
     /// Get the `DepNodeIndex` corresponding this crate. The result of this

Diff for: src/librustc_metadata/rmeta/decoder/cstore_impl.rs

@@ -9,7 +9,6 @@ use rustc_ast::expand::allocator::AllocatorKind;
 use rustc_data_structures::svh::Svh;
 use rustc_hir as hir;
 use rustc_hir::def_id::{CrateNum, DefId, DefIdMap, CRATE_DEF_INDEX, LOCAL_CRATE};
-use rustc_hir::definitions::DefPathTable;
 use rustc_hir::definitions::{DefKey, DefPath, DefPathHash};
 use rustc_middle::hir::exports::Export;
 use rustc_middle::middle::cstore::{CrateSource, CrateStore, EncodedMetadata};
@@ -486,8 +485,12 @@ impl CrateStore for CStore {
         self.get_crate_data(def.krate).def_path_hash(def.index)
     }
 
-    fn def_path_table(&self, cnum: CrateNum) -> &DefPathTable {
-        &self.get_crate_data(cnum).cdata.def_path_table
+    fn all_def_path_hashes_and_def_ids(&self, cnum: CrateNum) -> Vec<(DefPathHash, DefId)> {
+        self.get_crate_data(cnum).all_def_path_hashes_and_def_ids()
+    }
+
+    fn num_def_ids(&self, cnum: CrateNum) -> usize {
+        self.get_crate_data(cnum).num_def_ids()
     }
 
     fn crates_untracked(&self) -> Vec<CrateNum> {

Diff for: src/librustc_metadata/rmeta/encoder.rs

@@ -9,7 +9,6 @@ use rustc_data_structures::sync::{join, Lrc};
 use rustc_hir as hir;
 use rustc_hir::def::CtorKind;
 use rustc_hir::def_id::{CrateNum, DefId, DefIndex, LocalDefId, CRATE_DEF_INDEX, LOCAL_CRATE};
-use rustc_hir::definitions::DefPathTable;
 use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
 use rustc_hir::itemlikevisit::{ItemLikeVisitor, ParItemLikeVisitor};
 use rustc_hir::lang_items;
@@ -418,9 +417,14 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
         }
     }
 
-    fn encode_def_path_table(&mut self) -> Lazy<DefPathTable> {
-        let definitions = self.tcx.hir().definitions();
-        self.lazy(definitions.def_path_table())
+    fn encode_def_path_table(&mut self) {
+        let table = self.tcx.hir().definitions().def_path_table();
+        for (def_index, def_key, def_path_hash) in table.enumerated_keys_and_path_hashes() {
+            let def_key = self.lazy(def_key);
+            let def_path_hash = self.lazy(def_path_hash);
+            self.tables.def_keys.set(def_index, def_key);
+            self.tables.def_path_hashes.set(def_index, def_path_hash);
+        }
     }
 
     fn encode_source_map(&mut self) -> Lazy<[rustc_span::SourceFile]> {
@@ -525,7 +529,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
 
         // Encode DefPathTable
         i = self.position();
-        let def_path_table = self.encode_def_path_table();
+        self.encode_def_path_table();
         let def_path_table_bytes = self.position() - i;
 
         // Encode the def IDs of impls, for coherence checking.
@@ -642,7 +646,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
             native_libraries,
             foreign_modules,
             source_map,
-            def_path_table,
             impls,
             exported_symbols,
             interpret_alloc_index,

Diff for: src/librustc_metadata/rmeta/mod.rs

@@ -7,7 +7,8 @@ use rustc_data_structures::svh::Svh;
 use rustc_data_structures::sync::MetadataRef;
 use rustc_hir as hir;
 use rustc_hir::def::CtorKind;
-use rustc_hir::def_id::{DefId, DefIndex};
+use rustc_hir::def_id::{DefId, DefIndex, DefPathHash};
+use rustc_hir::definitions::DefKey;
 use rustc_hir::lang_items;
 use rustc_index::{bit_set::FiniteBitSet, vec::IndexVec};
 use rustc_middle::hir::exports::Export;
@@ -195,7 +196,6 @@ crate struct CrateRoot<'tcx> {
     diagnostic_items: Lazy<[(Symbol, DefIndex)]>,
     native_libraries: Lazy<[NativeLib]>,
     foreign_modules: Lazy<[ForeignModule]>,
-    def_path_table: Lazy<rustc_hir::definitions::DefPathTable>,
     impls: Lazy<[TraitImpls]>,
     interpret_alloc_index: Lazy<[u32]>,
 
@@ -285,6 +285,12 @@ define_tables! {
     mir: Table<DefIndex, Lazy!(mir::Body<'tcx>)>,
     promoted_mir: Table<DefIndex, Lazy!(IndexVec<mir::Promoted, mir::Body<'tcx>>)>,
     unused_generic_params: Table<DefIndex, Lazy<FiniteBitSet<u32>>>,
+    // `def_keys` and `def_path_hashes` represent a lazy version of a
+    // `DefPathTable`. This allows us to avoid deserializing an entire
+    // `DefPathTable` up front, since we may only ever use a few
+    // definitions from any given crate.
+    def_keys: Table<DefIndex, Lazy<DefKey>>,
+    def_path_hashes: Table<DefIndex, Lazy<DefPathHash>>
 }
 
 #[derive(Copy, Clone, MetadataEncodable, MetadataDecodable)]

Diff for: src/librustc_metadata/rmeta/table.rs

@@ -201,4 +201,9 @@ where
         let bytes = &metadata.raw_bytes()[start..start + self.meta];
         <Option<T>>::maybe_read_from_bytes_at(bytes, i.index())?
     }
+
+    /// Size of the table in entries, including possible gaps.
+    pub(super) fn size(&self) -> usize {
+        self.meta / <Option<T>>::BYTE_LEN
+    }
 }

Diff for: src/librustc_middle/middle/cstore.rs

@@ -9,7 +9,7 @@ use rustc_ast::expand::allocator::AllocatorKind;
 use rustc_data_structures::svh::Svh;
 use rustc_data_structures::sync::{self, MetadataRef};
 use rustc_hir::def_id::{CrateNum, DefId, LOCAL_CRATE};
-use rustc_hir::definitions::{DefKey, DefPath, DefPathHash, DefPathTable};
+use rustc_hir::definitions::{DefKey, DefPath, DefPathHash};
 use rustc_macros::HashStable;
 use rustc_session::search_paths::PathKind;
 use rustc_session::utils::NativeLibKind;
@@ -187,7 +187,8 @@ pub trait CrateStore {
     fn def_key(&self, def: DefId) -> DefKey;
     fn def_path(&self, def: DefId) -> DefPath;
     fn def_path_hash(&self, def: DefId) -> DefPathHash;
-    fn def_path_table(&self, cnum: CrateNum) -> &DefPathTable;
+    fn all_def_path_hashes_and_def_ids(&self, cnum: CrateNum) -> Vec<(DefPathHash, DefId)>;
+    fn num_def_ids(&self, cnum: CrateNum) -> usize;
 
     // "queries" used in resolve that aren't tracked for incremental compilation
     fn crate_name_untracked(&self, cnum: CrateNum) -> Symbol;

Diff for: src/librustc_middle/ty/context.rs

@@ -1102,20 +1102,13 @@ impl<'tcx> TyCtxt<'tcx> {
         providers[LOCAL_CRATE] = local_providers;
 
         let def_path_hash_to_def_id = if s.opts.build_dep_graph() {
-            let def_path_tables = crates
-                .iter()
-                .map(|&cnum| (cnum, cstore.def_path_table(cnum)))
-                .chain(iter::once((LOCAL_CRATE, definitions.def_path_table())));
+            let capacity = definitions.def_path_table().num_def_ids()
+                + crates.iter().map(|cnum| cstore.num_def_ids(*cnum)).sum::<usize>();
+            let mut map = FxHashMap::with_capacity_and_hasher(capacity, Default::default());
 
-            // Precompute the capacity of the hashmap so we don't have to
-            // re-allocate when populating it.
-            let capacity = def_path_tables.clone().map(|(_, t)| t.size()).sum::<usize>();
-
-            let mut map: FxHashMap<_, _> =
-                FxHashMap::with_capacity_and_hasher(capacity, ::std::default::Default::default());
-
-            for (cnum, def_path_table) in def_path_tables {
-                def_path_table.add_def_path_hashes_to(cnum, &mut map);
+            map.extend(definitions.def_path_table().all_def_path_hashes_and_def_ids(LOCAL_CRATE));
+            for cnum in &crates {
+                map.extend(cstore.all_def_path_hashes_and_def_ids(*cnum).into_iter());
             }
 
             Some(map)

Diff for: src/librustc_session/session.rs

@@ -218,8 +218,6 @@ pub struct Session {
 pub struct PerfStats {
     /// The accumulated time spent on computing symbol hashes.
     pub symbol_hash_time: Lock<Duration>,
-    /// The accumulated time spent decoding def path tables from metadata.
-    pub decode_def_path_tables_time: Lock<Duration>,
     /// Total number of values canonicalized queries constructed.
     pub queries_canonicalized: AtomicUsize,
     /// Number of times this query is invoked.
@@ -862,10 +860,6 @@ impl Session {
             "Total time spent computing symbol hashes:      {}",
             duration_to_secs_str(*self.perf_stats.symbol_hash_time.lock())
         );
-        println!(
-            "Total time spent decoding DefPath tables:      {}",
-            duration_to_secs_str(*self.perf_stats.decode_def_path_tables_time.lock())
-        );
         println!(
             "Total queries canonicalized:                   {}",
             self.perf_stats.queries_canonicalized.load(Ordering::Relaxed)
@@ -1339,7 +1333,6 @@ pub fn build_session(
         prof,
         perf_stats: PerfStats {
             symbol_hash_time: Lock::new(Duration::from_secs(0)),
-            decode_def_path_tables_time: Lock::new(Duration::from_secs(0)),
             queries_canonicalized: AtomicUsize::new(0),
             normalize_generic_arg_after_erasing_regions: AtomicUsize::new(0),
             normalize_projection_ty: AtomicUsize::new(0),