From 9dbd7be70f8b09aad232d5c4eda684cff622102d Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Wed, 9 Oct 2019 16:41:24 +0200 Subject: [PATCH 1/4] Cache the DepNodeIndex of upstream crates in order to avoid multiple locks and table lookups on each access of crate metadata. --- src/librustc/dep_graph/graph.rs | 2 +- src/librustc_metadata/creader.rs | 6 ++++-- src/librustc_metadata/cstore.rs | 10 +++++++++- src/librustc_metadata/cstore_impl.rs | 16 ++++++---------- src/librustc_metadata/decoder.rs | 25 +++++++++++++++++++++++++ 5 files changed, 45 insertions(+), 14 deletions(-) diff --git a/src/librustc/dep_graph/graph.rs b/src/librustc/dep_graph/graph.rs index e0cb00cf697f4..7e506f6e1b7f9 100644 --- a/src/librustc/dep_graph/graph.rs +++ b/src/librustc/dep_graph/graph.rs @@ -30,7 +30,7 @@ rustc_index::newtype_index! { } impl DepNodeIndex { - const INVALID: DepNodeIndex = DepNodeIndex::MAX; + pub const INVALID: DepNodeIndex = DepNodeIndex::MAX; } #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] diff --git a/src/librustc_metadata/creader.rs b/src/librustc_metadata/creader.rs index 042252bc13e61..cae4858c844c5 100644 --- a/src/librustc_metadata/creader.rs +++ b/src/librustc_metadata/creader.rs @@ -3,10 +3,11 @@ use crate::cstore::{self, CStore, CrateSource, MetadataBlob}; use crate::locator::{self, CratePaths}; use crate::schema::{CrateRoot, CrateDep}; -use rustc_data_structures::sync::{Lrc, RwLock, Lock}; +use rustc_data_structures::sync::{Lrc, RwLock, Lock, AtomicCell}; use rustc::hir::def_id::CrateNum; use rustc_data_structures::svh::Svh; +use rustc::dep_graph::DepNodeIndex; use rustc::middle::cstore::DepKind; use rustc::mir::interpret::AllocDecodingState; use rustc::session::{Session, CrateDisambiguator}; @@ -271,7 +272,8 @@ impl<'a> CrateLoader<'a> { }, private_dep, span, - raw_proc_macros + raw_proc_macros, + dep_node_index: AtomicCell::new(DepNodeIndex::INVALID), }; let cmeta = Lrc::new(cmeta); diff --git a/src/librustc_metadata/cstore.rs b/src/librustc_metadata/cstore.rs index 833c846573f63..98a08e501f14c 100644 --- a/src/librustc_metadata/cstore.rs +++ b/src/librustc_metadata/cstore.rs @@ -2,6 +2,7 @@ // crates and libraries use crate::schema; +use rustc::dep_graph::DepNodeIndex; use rustc::hir::def_id::{CrateNum, DefIndex}; use rustc::hir::map::definitions::DefPathTable; use rustc::middle::cstore::{DepKind, ExternCrate, MetadataLoader}; @@ -9,7 +10,7 @@ use rustc::mir::interpret::AllocDecodingState; use rustc_index::vec::IndexVec; use rustc::util::nodemap::{FxHashMap, NodeMap}; -use rustc_data_structures::sync::{Lrc, RwLock, Lock}; +use rustc_data_structures::sync::{Lrc, RwLock, Lock, AtomicCell}; use syntax::ast; use syntax::ext::base::SyntaxExtension; use syntax_pos; @@ -83,6 +84,13 @@ pub struct CrateMetadata { pub span: Span, pub raw_proc_macros: Option<&'static [ProcMacro]>, + + /// The `DepNodeIndex` of the `DepNode` representing this upstream crate. + /// It is initialized on the first access in `get_crate_dep_node_index()`. + /// Do not access the value directly, as it might not have been initialized + /// yet. + /// The field must always be initialized to `DepNodeIndex::INVALID`. + pub(super) dep_node_index: AtomicCell, } pub struct CStore { diff --git a/src/librustc_metadata/cstore_impl.rs b/src/librustc_metadata/cstore_impl.rs index cce0900bef3dd..b9f1d5a0a920d 100644 --- a/src/librustc_metadata/cstore_impl.rs +++ b/src/librustc_metadata/cstore_impl.rs @@ -51,19 +51,15 @@ macro_rules! provide { let ($def_id, $other) = def_id_arg.into_args(); assert!(!$def_id.is_local()); - let def_path_hash = $tcx.def_path_hash(DefId { - krate: $def_id.krate, - index: CRATE_DEF_INDEX - }); - let dep_node = def_path_hash - .to_dep_node(rustc::dep_graph::DepKind::CrateMetadata); - // The DepNodeIndex of the DepNode::CrateMetadata should be - // cached somewhere, so that we can use read_index(). - $tcx.dep_graph.read(dep_node); - let $cdata = $tcx.crate_data_as_rc_any($def_id.krate); let $cdata = $cdata.downcast_ref::() .expect("CrateStore created data is not a CrateMetadata"); + + if $tcx.dep_graph.is_fully_enabled() { + let crate_dep_node_index = $cdata.get_crate_dep_node_index($tcx); + $tcx.dep_graph.read_index(crate_dep_node_index); + } + $compute })* diff --git a/src/librustc_metadata/decoder.rs b/src/librustc_metadata/decoder.rs index eed355cbc1358..ce66534e5659f 100644 --- a/src/librustc_metadata/decoder.rs +++ b/src/librustc_metadata/decoder.rs @@ -13,6 +13,7 @@ use rustc::hir::def::{self, Res, DefKind, CtorOf, CtorKind}; use rustc::hir::def_id::{CrateNum, DefId, DefIndex, LocalDefId, CRATE_DEF_INDEX, LOCAL_CRATE}; use rustc_data_structures::fingerprint::Fingerprint; use rustc_data_structures::fx::FxHashMap; +use rustc::dep_graph::{DepNodeIndex, DepKind}; use rustc::middle::lang_items; use rustc::mir::{self, interpret}; use rustc::mir::interpret::AllocDecodingSession; @@ -1365,6 +1366,30 @@ impl<'a, 'tcx> CrateMetadata { // This shouldn't borrow twice, but there is no way to downgrade RefMut to Ref. self.source_map_import_info.borrow() } + + /// Get the `DepNodeIndex` corresponding this crate. The result of this + /// method is cached in the `dep_node_index` field. + pub(super) fn get_crate_dep_node_index(&self, tcx: TyCtxt<'tcx>) -> DepNodeIndex { + let mut dep_node_index = self.dep_node_index.load(); + + if dep_node_index == DepNodeIndex::INVALID { + // We have not cached the DepNodeIndex for this upstream crate yet, + // so use the dep-graph to find it out and cache it. + // Note that multiple threads can enter this block concurrently. + // That is fine because the DepNodeIndex remains constant + // throughout the whole compilation session, and multiple stores + // would always write the same value. + + let def_path_hash = self.def_path_hash(CRATE_DEF_INDEX); + let dep_node = def_path_hash.to_dep_node(DepKind::CrateMetadata); + + dep_node_index = tcx.dep_graph.dep_node_index_of(&dep_node); + assert!(dep_node_index != DepNodeIndex::INVALID); + self.dep_node_index.store(dep_node_index); + } + + dep_node_index + } } // Cannot be implemented on 'ProcMacro', as libproc_macro From 003d5a63670b61eabf16fe3057bd1d5e2ec65e11 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Wed, 9 Oct 2019 16:43:47 +0200 Subject: [PATCH 2/4] self-profiling: Add events for tracking crate metadata loading related activities. --- src/librustc_metadata/creader.rs | 3 +++ src/librustc_metadata/cstore_impl.rs | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/src/librustc_metadata/creader.rs b/src/librustc_metadata/creader.rs index cae4858c844c5..50a2187c93740 100644 --- a/src/librustc_metadata/creader.rs +++ b/src/librustc_metadata/creader.rs @@ -197,6 +197,9 @@ impl<'a> CrateLoader<'a> { dep_kind: DepKind, name: Symbol ) -> (CrateNum, Lrc) { + let _prof_timer = + self.sess.prof.generic_activity("metadata_register_crate"); + let crate_root = lib.metadata.get_root(); self.verify_no_symbol_conflicts(span, &crate_root); diff --git a/src/librustc_metadata/cstore_impl.rs b/src/librustc_metadata/cstore_impl.rs index b9f1d5a0a920d..cc68cc7f56e3c 100644 --- a/src/librustc_metadata/cstore_impl.rs +++ b/src/librustc_metadata/cstore_impl.rs @@ -47,6 +47,9 @@ macro_rules! provide { $tcx: TyCtxt<$lt>, def_id_arg: T, ) -> as QueryConfig<$lt>>::Value { + let _prof_timer = + $tcx.prof.generic_activity("metadata_decode_entry"); + #[allow(unused_variables)] let ($def_id, $other) = def_id_arg.into_args(); assert!(!$def_id.is_local()); @@ -444,6 +447,8 @@ impl cstore::CStore { } pub fn load_macro_untracked(&self, id: DefId, sess: &Session) -> LoadedMacro { + let _prof_timer = sess.prof.generic_activity("metadata_load_macro"); + let data = self.get_crate_data(id.krate); if data.is_proc_macro_crate() { return LoadedMacro::ProcMacro(data.load_proc_macro(id.index, sess)); From 0ee6a96eb76c4526012127cf1b65941233473579 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Wed, 9 Oct 2019 16:45:22 +0200 Subject: [PATCH 3/4] Remove some outdated comments about dependency tracking from cstore_impl. --- src/librustc_metadata/cstore_impl.rs | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/librustc_metadata/cstore_impl.rs b/src/librustc_metadata/cstore_impl.rs index cc68cc7f56e3c..b701933a540cf 100644 --- a/src/librustc_metadata/cstore_impl.rs +++ b/src/librustc_metadata/cstore_impl.rs @@ -526,20 +526,10 @@ impl CrateStore for cstore::CStore { /// parent `DefId` as well as some idea of what kind of data the /// `DefId` refers to. fn def_key(&self, def: DefId) -> DefKey { - // Note: loading the def-key (or def-path) for a def-id is not - // a *read* of its metadata. This is because the def-id is - // really just an interned shorthand for a def-path, which is the - // canonical name for an item. - // - // self.dep_graph.read(DepNode::MetaData(def)); self.get_crate_data(def.krate).def_key(def.index) } fn def_path(&self, def: DefId) -> DefPath { - // See `Note` above in `def_key()` for why this read is - // commented out: - // - // self.dep_graph.read(DepNode::MetaData(def)); self.get_crate_data(def.krate).def_path(def.index) } From a9853fce25723cbcb96380c3881fadb234530410 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Thu, 10 Oct 2019 09:42:06 +0200 Subject: [PATCH 4/4] Add 'unlikely' annotation to branch in crate DepNodeIndex caching. --- src/librustc_metadata/decoder.rs | 2 +- src/librustc_metadata/lib.rs | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/librustc_metadata/decoder.rs b/src/librustc_metadata/decoder.rs index ce66534e5659f..6f5280843489e 100644 --- a/src/librustc_metadata/decoder.rs +++ b/src/librustc_metadata/decoder.rs @@ -1372,7 +1372,7 @@ impl<'a, 'tcx> CrateMetadata { pub(super) fn get_crate_dep_node_index(&self, tcx: TyCtxt<'tcx>) -> DepNodeIndex { let mut dep_node_index = self.dep_node_index.load(); - if dep_node_index == DepNodeIndex::INVALID { + if unlikely!(dep_node_index == DepNodeIndex::INVALID) { // We have not cached the DepNodeIndex for this upstream crate yet, // so use the dep-graph to find it out and cache it. // Note that multiple threads can enter this block concurrently. diff --git a/src/librustc_metadata/lib.rs b/src/librustc_metadata/lib.rs index 9273b064ba9ce..6aa684b1c3d01 100644 --- a/src/librustc_metadata/lib.rs +++ b/src/librustc_metadata/lib.rs @@ -1,6 +1,7 @@ #![doc(html_root_url = "https://doc.rust-lang.org/nightly/")] #![feature(box_patterns)] +#![feature(core_intrinsics)] #![feature(crate_visibility_modifier)] #![feature(drain_filter)] #![feature(in_band_lifetimes)] @@ -11,6 +12,7 @@ #![feature(rustc_private)] #![feature(slice_patterns)] #![feature(specialization)] +#![feature(stmt_expr_attributes)] #![recursion_limit="256"]