From 3a6d5c2bebfbbbbcf3c196bd83e67681e8ecaa1c Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 15 May 2021 17:23:16 +0200 Subject: [PATCH 1/2] Avoid creating anonymous nodes with zero or one dependency. --- .../rustc_incremental/src/persist/save.rs | 1 + .../rustc_query_system/src/dep_graph/graph.rs | 76 ++++++++++++------- 2 files changed, 50 insertions(+), 27 deletions(-) diff --git a/compiler/rustc_incremental/src/persist/save.rs b/compiler/rustc_incremental/src/persist/save.rs index 9603b102cbc5d..a8455854ebb5f 100644 --- a/compiler/rustc_incremental/src/persist/save.rs +++ b/compiler/rustc_incremental/src/persist/save.rs @@ -229,6 +229,7 @@ pub fn build_dep_graph( } Some(DepGraph::new( + &sess.prof, prev_graph, prev_work_products, encoder, diff --git a/compiler/rustc_query_system/src/dep_graph/graph.rs b/compiler/rustc_query_system/src/dep_graph/graph.rs index cfae65abe8383..9481da70f4241 100644 --- a/compiler/rustc_query_system/src/dep_graph/graph.rs +++ b/compiler/rustc_query_system/src/dep_graph/graph.rs @@ -44,6 +44,7 @@ rustc_index::newtype_index! { impl DepNodeIndex { pub const INVALID: DepNodeIndex = DepNodeIndex::MAX; + pub const DUMMY_ANON: DepNodeIndex = DepNodeIndex::from_u32(0); } impl std::convert::From for QueryInvocationId { @@ -108,6 +109,7 @@ where impl DepGraph { pub fn new( + profiler: &SelfProfilerRef, prev_graph: SerializedDepGraph, prev_work_products: FxHashMap, encoder: FileEncoder, @@ -116,16 +118,23 @@ impl DepGraph { ) -> DepGraph { let prev_graph_node_count = prev_graph.node_count(); + let current = + CurrentDepGraph::new(prev_graph_node_count, encoder, record_graph, record_stats); + + // Instantiate an *always green* node for dependency-less anonymous queries. + let _green_node_index = current.intern_new_node( + profiler, + DepNode { kind: DepKind::NULL, hash: current.anon_id_seed.into() }, + smallvec![], + Fingerprint::ZERO, + ); + debug_assert_eq!(_green_node_index, DepNodeIndex::DUMMY_ANON); + DepGraph { data: Some(Lrc::new(DepGraphData { previous_work_products: prev_work_products, dep_node_debug: Default::default(), - current: CurrentDepGraph::new( - prev_graph_node_count, - encoder, - record_graph, - record_stats, - ), + current, emitting_diagnostics: Default::default(), emitting_diagnostics_cond_var: Condvar::new(), previous: prev_graph, @@ -287,29 +296,42 @@ impl DepGraph { let task_deps = Lock::new(TaskDeps::default()); let result = K::with_deps(Some(&task_deps), op); let task_deps = task_deps.into_inner(); + let task_deps = task_deps.reads; - // The dep node indices are hashed here instead of hashing the dep nodes of the - // dependencies. These indices may refer to different nodes per session, but this isn't - // a problem here because we that ensure the final dep node hash is per session only by - // combining it with the per session random number `anon_id_seed`. This hash only need - // to map the dependencies to a single value on a per session basis. - let mut hasher = StableHasher::new(); - task_deps.reads.hash(&mut hasher); - - let target_dep_node = DepNode { - kind: dep_kind, - // Fingerprint::combine() is faster than sending Fingerprint - // through the StableHasher (at least as long as StableHasher - // is so slow). - hash: data.current.anon_id_seed.combine(hasher.finish()).into(), - }; + let dep_node_index = match task_deps.len() { + 0 => { + // Dependency-less anonymous nodes can safely be replaced by a dummy node. + DepNodeIndex::DUMMY_ANON + } + 1 => { + // When there is only one dependency, don't bother creating a node. + task_deps[0] + } + _ => { + // The dep node indices are hashed here instead of hashing the dep nodes of the + // dependencies. These indices may refer to different nodes per session, but this isn't + // a problem here because we that ensure the final dep node hash is per session only by + // combining it with the per session random number `anon_id_seed`. This hash only need + // to map the dependencies to a single value on a per session basis. + let mut hasher = StableHasher::new(); + task_deps.hash(&mut hasher); + + let target_dep_node = DepNode { + kind: dep_kind, + // Fingerprint::combine() is faster than sending Fingerprint + // through the StableHasher (at least as long as StableHasher + // is so slow). + hash: data.current.anon_id_seed.combine(hasher.finish()).into(), + }; - let dep_node_index = data.current.intern_new_node( - cx.profiler(), - target_dep_node, - task_deps.reads, - Fingerprint::ZERO, - ); + data.current.intern_new_node( + cx.profiler(), + target_dep_node, + task_deps, + Fingerprint::ZERO, + ) + } + }; (result, dep_node_index) } else { From b51f24f021a985e689f35214302ed5844907962d Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Tue, 1 Jun 2021 21:46:30 +0200 Subject: [PATCH 2/2] Make the reasoning more explicit. --- compiler/rustc_query_system/src/dep_graph/graph.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_query_system/src/dep_graph/graph.rs b/compiler/rustc_query_system/src/dep_graph/graph.rs index 9481da70f4241..71e67dfee538b 100644 --- a/compiler/rustc_query_system/src/dep_graph/graph.rs +++ b/compiler/rustc_query_system/src/dep_graph/graph.rs @@ -44,7 +44,7 @@ rustc_index::newtype_index! { impl DepNodeIndex { pub const INVALID: DepNodeIndex = DepNodeIndex::MAX; - pub const DUMMY_ANON: DepNodeIndex = DepNodeIndex::from_u32(0); + pub const SINGLETON_DEPENDENCYLESS_ANON_NODE: DepNodeIndex = DepNodeIndex::from_u32(0); } impl std::convert::From for QueryInvocationId { @@ -121,14 +121,14 @@ impl DepGraph { let current = CurrentDepGraph::new(prev_graph_node_count, encoder, record_graph, record_stats); - // Instantiate an *always green* node for dependency-less anonymous queries. + // Instantiate a dependy-less node only once for anonymous queries. let _green_node_index = current.intern_new_node( profiler, DepNode { kind: DepKind::NULL, hash: current.anon_id_seed.into() }, smallvec![], Fingerprint::ZERO, ); - debug_assert_eq!(_green_node_index, DepNodeIndex::DUMMY_ANON); + debug_assert_eq!(_green_node_index, DepNodeIndex::SINGLETON_DEPENDENCYLESS_ANON_NODE); DepGraph { data: Some(Lrc::new(DepGraphData { @@ -300,8 +300,12 @@ impl DepGraph { let dep_node_index = match task_deps.len() { 0 => { - // Dependency-less anonymous nodes can safely be replaced by a dummy node. - DepNodeIndex::DUMMY_ANON + // Because the dep-node id of anon nodes is computed from the sets of its + // dependencies we already know what the ID of this dependency-less node is + // going to be (i.e. equal to the precomputed + // `SINGLETON_DEPENDENCYLESS_ANON_NODE`). As a consequence we can skip creating + // a `StableHasher` and sending the node through interning. + DepNodeIndex::SINGLETON_DEPENDENCYLESS_ANON_NODE } 1 => { // When there is only one dependency, don't bother creating a node.