From 62139ffad4a77d45b9651b04b440c89c5b9c1b5c Mon Sep 17 00:00:00 2001 From: Tyson Nottingham Date: Fri, 1 Jan 2021 14:06:17 -0800 Subject: [PATCH] Remove DepKind::CrateMetadata and pre-allocation of DepNodes Remove much of the special-case handling around crate metadata dependency tracking by replacing `DepKind::CrateMetadata` and the pre-allocation of corresponding `DepNodes` with on-demand invocation of the `crate_hash` query. --- compiler/rustc_incremental/src/lib.rs | 1 - .../rustc_incremental/src/persist/load.rs | 9 ----- compiler/rustc_incremental/src/persist/mod.rs | 1 - compiler/rustc_interface/src/passes.rs | 6 ---- compiler/rustc_metadata/src/rmeta/decoder.rs | 34 +------------------ .../src/rmeta/decoder/cstore_impl.rs | 24 ++++++++++--- .../rustc_middle/src/dep_graph/dep_node.rs | 14 -------- compiler/rustc_middle/src/dep_graph/mod.rs | 15 ++------ compiler/rustc_middle/src/ty/context.rs | 27 ++------------- 9 files changed, 24 insertions(+), 107 deletions(-) diff --git a/compiler/rustc_incremental/src/lib.rs b/compiler/rustc_incremental/src/lib.rs index a80c4be3e93..95456c07b10 100644 --- a/compiler/rustc_incremental/src/lib.rs +++ b/compiler/rustc_incremental/src/lib.rs @@ -17,7 +17,6 @@ mod persist; pub use assert_dep_graph::assert_dep_graph; pub use persist::copy_cgu_workproduct_to_incr_comp_cache_dir; pub use persist::delete_workproduct_files; -pub use persist::dep_graph_tcx_init; pub use persist::finalize_session_directory; pub use persist::garbage_collect_session_directories; pub use persist::in_incr_comp_dir; diff --git a/compiler/rustc_incremental/src/persist/load.rs b/compiler/rustc_incremental/src/persist/load.rs index 35428dc8d84..0add0c5aa26 100644 --- a/compiler/rustc_incremental/src/persist/load.rs +++ b/compiler/rustc_incremental/src/persist/load.rs @@ -4,7 +4,6 @@ use rustc_data_structures::fx::FxHashMap; use rustc_hir::definitions::Definitions; use rustc_middle::dep_graph::{PreviousDepGraph, SerializedDepGraph, WorkProduct, WorkProductId}; use rustc_middle::ty::query::OnDiskCache; -use rustc_middle::ty::TyCtxt; use rustc_serialize::opaque::Decoder; use rustc_serialize::Decodable as RustcDecodable; use rustc_session::Session; @@ -15,14 +14,6 @@ use super::file_format; use super::fs::*; use super::work_product; -pub fn dep_graph_tcx_init(tcx: TyCtxt<'_>) { - if !tcx.dep_graph.is_fully_enabled() { - return; - } - - tcx.allocate_metadata_dep_nodes(); -} - type WorkProductMap = FxHashMap; pub enum LoadResult { diff --git a/compiler/rustc_incremental/src/persist/mod.rs b/compiler/rustc_incremental/src/persist/mod.rs index 7bc3b47e15a..8821b34b502 100644 --- a/compiler/rustc_incremental/src/persist/mod.rs +++ b/compiler/rustc_incremental/src/persist/mod.rs @@ -15,7 +15,6 @@ pub use fs::garbage_collect_session_directories; pub use fs::in_incr_comp_dir; pub use fs::in_incr_comp_dir_sess; pub use fs::prepare_session_directory; -pub use load::dep_graph_tcx_init; pub use load::load_query_result_cache; pub use load::LoadResult; pub use load::{load_dep_graph, DepGraphFuture}; diff --git a/compiler/rustc_interface/src/passes.rs b/compiler/rustc_interface/src/passes.rs index b67704119bc..ead2512d3b2 100644 --- a/compiler/rustc_interface/src/passes.rs +++ b/compiler/rustc_interface/src/passes.rs @@ -797,12 +797,6 @@ pub fn create_global_ctxt<'tcx>( }) }); - // Do some initialization of the DepGraph that can only be done with the tcx available. - let icx = ty::tls::ImplicitCtxt::new(&gcx); - ty::tls::enter_context(&icx, |_| { - icx.tcx.sess.time("dep_graph_tcx_init", || rustc_incremental::dep_graph_tcx_init(icx.tcx)); - }); - QueryContext(gcx) } diff --git a/compiler/rustc_metadata/src/rmeta/decoder.rs b/compiler/rustc_metadata/src/rmeta/decoder.rs index 6e381fd2965..e864f53f73d 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder.rs @@ -10,7 +10,7 @@ use rustc_data_structures::captures::Captures; use rustc_data_structures::fingerprint::{Fingerprint, FingerprintDecoder}; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::svh::Svh; -use rustc_data_structures::sync::{AtomicCell, Lock, LockGuard, Lrc, OnceCell}; +use rustc_data_structures::sync::{Lock, LockGuard, Lrc, OnceCell}; use rustc_data_structures::unhash::UnhashMap; use rustc_errors::ErrorReported; use rustc_expand::base::{SyntaxExtension, SyntaxExtensionKind}; @@ -21,7 +21,6 @@ use rustc_hir::def_id::{CrateNum, DefId, DefIndex, CRATE_DEF_INDEX, LOCAL_CRATE} use rustc_hir::definitions::{DefKey, DefPath, DefPathData, DefPathHash}; use rustc_hir::lang_items; use rustc_index::vec::{Idx, IndexVec}; -use rustc_middle::dep_graph::{self, DepNode, DepNodeExt, DepNodeIndex}; use rustc_middle::hir::exports::Export; use rustc_middle::middle::cstore::{CrateSource, ExternCrate}; use rustc_middle::middle::cstore::{ForeignModule, LinkagePreference, NativeLib}; @@ -84,11 +83,6 @@ crate struct CrateMetadata { def_path_hash_map: OnceCell>, /// Used for decoding interpret::AllocIds in a cached & thread-safe manner. alloc_decoding_state: AllocDecodingState, - /// 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`. - dep_node_index: AtomicCell, /// Caches decoded `DefKey`s. def_key_cache: Lock>, /// Caches decoded `DefPathHash`es. @@ -1592,31 +1586,6 @@ impl<'a, 'tcx> CrateMetadataRef<'a> { self.def_path_hash_unlocked(index, &mut def_path_hashes) } - /// Get the `DepNodeIndex` corresponding this crate. The result of this - /// method is cached in the `dep_node_index` field. - fn get_crate_dep_node_index(&self, tcx: TyCtxt<'tcx>) -> DepNodeIndex { - let mut dep_node_index = self.dep_node_index.load(); - - 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. - // 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 = - DepNode::from_def_path_hash(def_path_hash, dep_graph::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 - } - /// Imports the source_map from an external crate into the source_map of the crate /// currently being compiled (the "local crate"). /// @@ -1833,7 +1802,6 @@ impl CrateMetadata { source_map_import_info: OnceCell::new(), def_path_hash_map: Default::default(), alloc_decoding_state, - dep_node_index: AtomicCell::new(DepNodeIndex::INVALID), cnum, cnum_map, dependencies, diff --git a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs index b7f22885217..979e70b9e31 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs @@ -44,18 +44,33 @@ macro_rules! provide { let ($def_id, $other) = def_id_arg.into_args(); assert!(!$def_id.is_local()); - let $cdata = CStore::from_tcx($tcx).get_crate_data($def_id.krate); - 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); + $tcx.ensure().crate_hash($def_id.krate); } + let $cdata = CStore::from_tcx($tcx).get_crate_data($def_id.krate); + $compute })* + // The other external query providers call `crate_hash` in order to register a + // dependency on the crate metadata. The `crate_hash` implementation differs in + // that it doesn't need to do this (and can't, as it would cause a query cycle). + fn crate_hash<'tcx>( + tcx: TyCtxt<'tcx>, + def_id_arg: ty::query::query_keys::crate_hash<'tcx>, + ) -> ty::query::query_values::crate_hash<'tcx> { + let _prof_timer = tcx.prof.generic_activity("metadata_decode_entry_crate_hash"); + + let (def_id, _) = def_id_arg.into_args(); + assert!(!def_id.is_local()); + + CStore::from_tcx(tcx).get_crate_data(def_id.krate).root.hash + } + *providers = Providers { $($name,)* + crate_hash, ..*providers }; } @@ -191,7 +206,6 @@ provide! { <'tcx> tcx, def_id, other, cdata, }) } crate_disambiguator => { cdata.root.disambiguator } - crate_hash => { cdata.root.hash } crate_host_hash => { cdata.host_hash } original_crate_name => { cdata.root.name } diff --git a/compiler/rustc_middle/src/dep_graph/dep_node.rs b/compiler/rustc_middle/src/dep_graph/dep_node.rs index b775846bba4..195c00f3535 100644 --- a/compiler/rustc_middle/src/dep_graph/dep_node.rs +++ b/compiler/rustc_middle/src/dep_graph/dep_node.rs @@ -214,17 +214,6 @@ pub mod dep_kind { try_load_from_on_disk_cache: |_, _| {}, }; - // Represents metadata from an extern crate. - pub const CrateMetadata: DepKindStruct = DepKindStruct { - has_params: true, - is_anon: false, - is_eval_always: true, - - can_reconstruct_query_key: || true, - force_from_dep_node: |_, dep_node| bug!("force_from_dep_node: encountered {:?}", dep_node), - try_load_from_on_disk_cache: |_, _| {}, - }; - pub const TraitSelect: DepKindStruct = DepKindStruct { has_params: false, is_anon: true, @@ -379,9 +368,6 @@ rustc_dep_node_append!([define_dep_nodes!][ <'tcx> // We use this for most things when incr. comp. is turned off. [] Null, - // Represents metadata from an extern crate. - [eval_always] CrateMetadata(CrateNum), - [anon] TraitSelect, [] CompileCodegenUnit(Symbol), diff --git a/compiler/rustc_middle/src/dep_graph/mod.rs b/compiler/rustc_middle/src/dep_graph/mod.rs index 22e9cc1cd3e..50c9a696fb9 100644 --- a/compiler/rustc_middle/src/dep_graph/mod.rs +++ b/compiler/rustc_middle/src/dep_graph/mod.rs @@ -115,20 +115,9 @@ impl<'tcx> DepContext for TyCtxt<'tcx> { // be removed. https://github.com/rust-lang/rust/issues/62649 is one such // bug that must be fixed before removing this. match dep_node.kind { - DepKind::hir_owner | DepKind::hir_owner_nodes | DepKind::CrateMetadata => { + DepKind::hir_owner | DepKind::hir_owner_nodes => { if let Some(def_id) = dep_node.extract_def_id(*self) { - if def_id_corresponds_to_hir_dep_node(*self, def_id.expect_local()) { - if dep_node.kind == DepKind::CrateMetadata { - // The `DefPath` has corresponding node, - // and that node should have been marked - // either red or green in `data.colors`. - bug!( - "DepNode {:?} should have been \ - pre-marked as red or green but wasn't.", - dep_node - ); - } - } else { + if !def_id_corresponds_to_hir_dep_node(*self, def_id.expect_local()) { // This `DefPath` does not have a // corresponding `DepNode` (e.g. a // struct field), and the ` DefPath` diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index 3540f0f06b6..bc15991089e 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -1,7 +1,7 @@ //! Type context book-keeping. use crate::arena::Arena; -use crate::dep_graph::{self, DepGraph, DepKind, DepNode, DepNodeExt}; +use crate::dep_graph::DepGraph; use crate::hir::exports::ExportMap; use crate::ich::{NodeIdHashingMode, StableHashingContext}; use crate::infer::canonical::{Canonical, CanonicalVarInfo, CanonicalVarInfos}; @@ -37,8 +37,7 @@ use rustc_data_structures::sync::{self, Lock, Lrc, WorkerLocal}; use rustc_errors::ErrorReported; use rustc_hir as hir; use rustc_hir::def::{DefKind, Res}; -use rustc_hir::def_id::{CrateNum, DefId, DefIdMap, LocalDefId}; -use rustc_hir::def_id::{CRATE_DEF_INDEX, LOCAL_CRATE}; +use rustc_hir::def_id::{CrateNum, DefId, DefIdMap, LocalDefId, LOCAL_CRATE}; use rustc_hir::definitions::Definitions; use rustc_hir::intravisit::Visitor; use rustc_hir::lang_items::LangItem; @@ -1315,28 +1314,6 @@ impl<'tcx> TyCtxt<'tcx> { StableHashingContext::ignore_spans(self.sess, krate, self.definitions, &*self.cstore) } - // This method makes sure that we have a DepNode and a Fingerprint for - // every upstream crate. It needs to be called once right after the tcx is - // created. - // With full-fledged red/green, the method will probably become unnecessary - // as this will be done on-demand. - pub fn allocate_metadata_dep_nodes(self) { - // We cannot use the query versions of crates() and crate_hash(), since - // those would need the DepNodes that we are allocating here. - for cnum in self.cstore.crates_untracked() { - let def_path_hash = self.def_path_hash(DefId { krate: cnum, index: CRATE_DEF_INDEX }); - let dep_node = DepNode::from_def_path_hash(def_path_hash, DepKind::CrateMetadata); - let crate_hash = self.cstore.crate_hash_untracked(cnum); - self.dep_graph.with_task( - dep_node, - self, - crate_hash, - |_, x| x, // No transformation needed - dep_graph::hash_result, - ); - } - } - pub fn serialize_query_result_cache(self, encoder: &mut FileEncoder) -> FileEncodeResult { self.queries.on_disk_cache.as_ref().map(|c| c.serialize(self, encoder)).unwrap_or(Ok(())) }