From 29cc76f0fc45e8ec0da07c513296a9980db4b9b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Wed, 6 Mar 2024 04:17:17 +0100 Subject: [PATCH 1/3] Add a profiler reference to `GraphEncoder` --- compiler/rustc_middle/src/ty/context.rs | 2 +- .../rustc_query_system/src/dep_graph/graph.rs | 36 ++++++------------- .../src/dep_graph/serialized.rs | 13 +++---- 3 files changed, 18 insertions(+), 33 deletions(-) diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index 4eec93532a246..059dcabe937e3 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -650,7 +650,7 @@ impl<'tcx> GlobalCtxt<'tcx> { } pub fn finish(&self) -> FileEncodeResult { - self.dep_graph.finish_encoding(&self.sess.prof) + self.dep_graph.finish_encoding() } } diff --git a/compiler/rustc_query_system/src/dep_graph/graph.rs b/compiler/rustc_query_system/src/dep_graph/graph.rs index 7dc1a1f791752..b0df0067b5bb4 100644 --- a/compiler/rustc_query_system/src/dep_graph/graph.rs +++ b/compiler/rustc_query_system/src/dep_graph/graph.rs @@ -134,7 +134,6 @@ impl DepGraph { // Instantiate a dependy-less node only once for anonymous queries. let _green_node_index = current.intern_new_node( - profiler, DepNode { kind: D::DEP_KIND_NULL, hash: current.anon_id_seed.into() }, EdgesVec::new(), Fingerprint::ZERO, @@ -443,12 +442,7 @@ impl DepGraphData { hash: self.current.anon_id_seed.combine(hasher.finish()).into(), }; - self.current.intern_new_node( - cx.profiler(), - target_dep_node, - task_deps, - Fingerprint::ZERO, - ) + self.current.intern_new_node(target_dep_node, task_deps, Fingerprint::ZERO) } }; @@ -871,11 +865,8 @@ impl DepGraphData { // We allocating an entry for the node in the current dependency graph and // adding all the appropriate edges imported from the previous graph - let dep_node_index = self.current.promote_node_and_deps_to_current( - qcx.dep_context().profiler(), - &self.previous, - prev_dep_node_index, - ); + let dep_node_index = + self.current.promote_node_and_deps_to_current(&self.previous, prev_dep_node_index); // ... emitting any stored diagnostic ... @@ -981,12 +972,8 @@ impl DepGraph { } } - pub fn finish_encoding(&self, profiler: &SelfProfilerRef) -> FileEncodeResult { - if let Some(data) = &self.data { - data.current.encoder.steal().finish(profiler) - } else { - Ok(0) - } + pub fn finish_encoding(&self) -> FileEncodeResult { + if let Some(data) = &self.data { data.current.encoder.steal().finish() } else { Ok(0) } } pub(crate) fn next_virtual_depnode_index(&self) -> DepNodeIndex { @@ -1150,6 +1137,7 @@ impl CurrentDepGraph { prev_graph_node_count, record_graph, record_stats, + profiler, )), new_node_to_index: Sharded::new(|| { FxHashMap::with_capacity_and_hasher( @@ -1183,7 +1171,6 @@ impl CurrentDepGraph { #[inline(always)] fn intern_new_node( &self, - profiler: &SelfProfilerRef, key: DepNode, edges: EdgesVec, current_fingerprint: Fingerprint, @@ -1191,8 +1178,7 @@ impl CurrentDepGraph { let dep_node_index = match self.new_node_to_index.lock_shard_by_value(&key).entry(key) { Entry::Occupied(entry) => *entry.get(), Entry::Vacant(entry) => { - let dep_node_index = - self.encoder.borrow().send(profiler, key, current_fingerprint, edges); + let dep_node_index = self.encoder.borrow().send(key, current_fingerprint, edges); entry.insert(dep_node_index); dep_node_index } @@ -1223,8 +1209,7 @@ impl CurrentDepGraph { let dep_node_index = match prev_index_to_index[prev_index] { Some(dep_node_index) => dep_node_index, None => { - let dep_node_index = - self.encoder.borrow().send(profiler, key, fingerprint, edges); + let dep_node_index = self.encoder.borrow().send(key, fingerprint, edges); prev_index_to_index[prev_index] = Some(dep_node_index); dep_node_index } @@ -1261,7 +1246,7 @@ impl CurrentDepGraph { let fingerprint = fingerprint.unwrap_or(Fingerprint::ZERO); // This is a new node: it didn't exist in the previous compilation session. - let dep_node_index = self.intern_new_node(profiler, key, edges, fingerprint); + let dep_node_index = self.intern_new_node(key, edges, fingerprint); (dep_node_index, None) } @@ -1269,7 +1254,6 @@ impl CurrentDepGraph { fn promote_node_and_deps_to_current( &self, - profiler: &SelfProfilerRef, prev_graph: &SerializedDepGraph, prev_index: SerializedDepNodeIndex, ) -> DepNodeIndex { @@ -1286,7 +1270,7 @@ impl CurrentDepGraph { .map(|i| prev_index_to_index[i].unwrap()) .collect(); let fingerprint = prev_graph.fingerprint_by_index(prev_index); - let dep_node_index = self.encoder.borrow().send(profiler, key, fingerprint, edges); + let dep_node_index = self.encoder.borrow().send(key, fingerprint, edges); prev_index_to_index[prev_index] = Some(dep_node_index); #[cfg(debug_assertions)] self.record_edge(dep_node_index, key, fingerprint); diff --git a/compiler/rustc_query_system/src/dep_graph/serialized.rs b/compiler/rustc_query_system/src/dep_graph/serialized.rs index 3272a0ed16735..d8db94defbdf4 100644 --- a/compiler/rustc_query_system/src/dep_graph/serialized.rs +++ b/compiler/rustc_query_system/src/dep_graph/serialized.rs @@ -504,6 +504,7 @@ impl EncoderState { } pub struct GraphEncoder { + profiler: SelfProfilerRef, status: Lock>, record_graph: Option>, } @@ -514,10 +515,11 @@ impl GraphEncoder { prev_node_count: usize, record_graph: bool, record_stats: bool, + profiler: &SelfProfilerRef, ) -> Self { let record_graph = record_graph.then(|| Lock::new(DepGraphQuery::new(prev_node_count))); let status = Lock::new(EncoderState::new(encoder, record_stats)); - GraphEncoder { status, record_graph } + GraphEncoder { status, record_graph, profiler: profiler.clone() } } pub(crate) fn with_query(&self, f: impl Fn(&DepGraphQuery)) { @@ -580,18 +582,17 @@ impl GraphEncoder { pub(crate) fn send( &self, - profiler: &SelfProfilerRef, node: DepNode, fingerprint: Fingerprint, edges: EdgesVec, ) -> DepNodeIndex { - let _prof_timer = profiler.generic_activity("incr_comp_encode_dep_graph"); + let _prof_timer = self.profiler.generic_activity("incr_comp_encode_dep_graph"); let node = NodeInfo { node, fingerprint, edges }; self.status.lock().encode_node(&node, &self.record_graph) } - pub fn finish(self, profiler: &SelfProfilerRef) -> FileEncodeResult { - let _prof_timer = profiler.generic_activity("incr_comp_encode_dep_graph"); - self.status.into_inner().finish(profiler) + pub fn finish(self) -> FileEncodeResult { + let _prof_timer = self.profiler.generic_activity("incr_comp_encode_dep_graph"); + self.status.into_inner().finish(&self.profiler) } } From a2499bdfbe42f1538a0176026196920be14ef105 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Wed, 6 Mar 2024 04:31:56 +0100 Subject: [PATCH 2/3] Remove profiling from `intern_node` --- .../rustc_query_system/src/dep_graph/graph.rs | 37 +++---------------- 1 file changed, 5 insertions(+), 32 deletions(-) diff --git a/compiler/rustc_query_system/src/dep_graph/graph.rs b/compiler/rustc_query_system/src/dep_graph/graph.rs index b0df0067b5bb4..03e435506ab4c 100644 --- a/compiler/rustc_query_system/src/dep_graph/graph.rs +++ b/compiler/rustc_query_system/src/dep_graph/graph.rs @@ -1,6 +1,6 @@ use rustc_data_structures::fingerprint::Fingerprint; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; -use rustc_data_structures::profiling::{EventId, QueryInvocationId, SelfProfilerRef}; +use rustc_data_structures::profiling::{QueryInvocationId, SelfProfilerRef}; use rustc_data_structures::sharded::{self, Sharded}; use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; use rustc_data_structures::steal::Steal; @@ -142,7 +142,6 @@ impl DepGraph { // Instantiate a dependy-less red node only once for anonymous queries. let (red_node_index, red_node_prev_index_and_color) = current.intern_node( - profiler, &prev_graph, DepNode { kind: D::DEP_KIND_RED, hash: Fingerprint::ZERO.into() }, EdgesVec::new(), @@ -371,13 +370,8 @@ impl DepGraphData { hash_result.map(|f| dcx.with_stable_hashing_context(|mut hcx| f(&mut hcx, &result))); // Intern the new `DepNode`. - let (dep_node_index, prev_and_color) = self.current.intern_node( - dcx.profiler(), - &self.previous, - key, - edges, - current_fingerprint, - ); + let (dep_node_index, prev_and_color) = + self.current.intern_node(&self.previous, key, edges, current_fingerprint); hashing_timer.finish_with_query_invocation_id(dep_node_index.into()); @@ -579,13 +573,8 @@ impl DepGraph { }); // Intern the new `DepNode` with the dependencies up-to-now. - let (dep_node_index, prev_and_color) = data.current.intern_node( - cx.profiler(), - &data.previous, - node, - edges, - current_fingerprint, - ); + let (dep_node_index, prev_and_color) = + data.current.intern_node(&data.previous, node, edges, current_fingerprint); hashing_timer.finish_with_query_invocation_id(dep_node_index.into()); @@ -1087,12 +1076,6 @@ pub(super) struct CurrentDepGraph { /// debugging and only active with `debug_assertions`. total_read_count: AtomicU64, total_duplicate_read_count: AtomicU64, - - /// The cached event id for profiling node interning. This saves us - /// from having to look up the event id every time we intern a node - /// which may incur too much overhead. - /// This will be None if self-profiling is disabled. - node_intern_event_id: Option, } impl CurrentDepGraph { @@ -1127,10 +1110,6 @@ impl CurrentDepGraph { let new_node_count_estimate = 102 * prev_graph_node_count / 100 + 200; - let node_intern_event_id = profiler - .get_or_alloc_cached_string("incr_comp_intern_dep_graph_node") - .map(EventId::from_label); - CurrentDepGraph { encoder: Steal::new(GraphEncoder::new( encoder, @@ -1153,7 +1132,6 @@ impl CurrentDepGraph { fingerprints: Lock::new(IndexVec::from_elem_n(None, new_node_count_estimate)), total_read_count: AtomicU64::new(0), total_duplicate_read_count: AtomicU64::new(0), - node_intern_event_id, } } @@ -1192,16 +1170,11 @@ impl CurrentDepGraph { fn intern_node( &self, - profiler: &SelfProfilerRef, prev_graph: &SerializedDepGraph, key: DepNode, edges: EdgesVec, fingerprint: Option, ) -> (DepNodeIndex, Option<(SerializedDepNodeIndex, DepNodeColor)>) { - // Get timer for profiling `DepNode` interning - let _node_intern_timer = - self.node_intern_event_id.map(|eid| profiler.generic_activity_with_event_id(eid)); - if let Some(prev_index) = prev_graph.node_to_index_opt(&key) { let get_dep_node_index = |fingerprint| { let mut prev_index_to_index = self.prev_index_to_index.lock(); From 9707e103ea129e575c2beb8e034a9ce6230a62f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Wed, 6 Mar 2024 04:40:39 +0100 Subject: [PATCH 3/3] Avoid the double lock around `EncoderState` --- .../rustc_query_system/src/dep_graph/graph.rs | 19 +++++++++---------- .../src/dep_graph/serialized.rs | 16 +++++++++------- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_query_system/src/dep_graph/graph.rs b/compiler/rustc_query_system/src/dep_graph/graph.rs index 03e435506ab4c..9f067273f3580 100644 --- a/compiler/rustc_query_system/src/dep_graph/graph.rs +++ b/compiler/rustc_query_system/src/dep_graph/graph.rs @@ -3,7 +3,6 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::profiling::{QueryInvocationId, SelfProfilerRef}; use rustc_data_structures::sharded::{self, Sharded}; use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; -use rustc_data_structures::steal::Steal; use rustc_data_structures::sync::{AtomicU32, AtomicU64, Lock, Lrc}; use rustc_data_structures::unord::UnordMap; use rustc_index::IndexVec; @@ -194,7 +193,7 @@ impl DepGraph { pub fn with_query(&self, f: impl Fn(&DepGraphQuery)) { if let Some(data) = &self.data { - data.current.encoder.borrow().with_query(f) + data.current.encoder.with_query(f) } } @@ -954,7 +953,7 @@ impl DepGraph { pub fn print_incremental_info(&self) { if let Some(data) = &self.data { - data.current.encoder.borrow().print_incremental_info( + data.current.encoder.print_incremental_info( data.current.total_read_count.load(Ordering::Relaxed), data.current.total_duplicate_read_count.load(Ordering::Relaxed), ) @@ -962,7 +961,7 @@ impl DepGraph { } pub fn finish_encoding(&self) -> FileEncodeResult { - if let Some(data) = &self.data { data.current.encoder.steal().finish() } else { Ok(0) } + if let Some(data) = &self.data { data.current.encoder.finish() } else { Ok(0) } } pub(crate) fn next_virtual_depnode_index(&self) -> DepNodeIndex { @@ -1045,7 +1044,7 @@ rustc_index::newtype_index! { /// manipulating both, we acquire `new_node_to_index` or `prev_index_to_index` /// first, and `data` second. pub(super) struct CurrentDepGraph { - encoder: Steal>, + encoder: GraphEncoder, new_node_to_index: Sharded>, prev_index_to_index: Lock>>, @@ -1111,13 +1110,13 @@ impl CurrentDepGraph { let new_node_count_estimate = 102 * prev_graph_node_count / 100 + 200; CurrentDepGraph { - encoder: Steal::new(GraphEncoder::new( + encoder: GraphEncoder::new( encoder, prev_graph_node_count, record_graph, record_stats, profiler, - )), + ), new_node_to_index: Sharded::new(|| { FxHashMap::with_capacity_and_hasher( new_node_count_estimate / sharded::shards(), @@ -1156,7 +1155,7 @@ impl CurrentDepGraph { let dep_node_index = match self.new_node_to_index.lock_shard_by_value(&key).entry(key) { Entry::Occupied(entry) => *entry.get(), Entry::Vacant(entry) => { - let dep_node_index = self.encoder.borrow().send(key, current_fingerprint, edges); + let dep_node_index = self.encoder.send(key, current_fingerprint, edges); entry.insert(dep_node_index); dep_node_index } @@ -1182,7 +1181,7 @@ impl CurrentDepGraph { let dep_node_index = match prev_index_to_index[prev_index] { Some(dep_node_index) => dep_node_index, None => { - let dep_node_index = self.encoder.borrow().send(key, fingerprint, edges); + let dep_node_index = self.encoder.send(key, fingerprint, edges); prev_index_to_index[prev_index] = Some(dep_node_index); dep_node_index } @@ -1243,7 +1242,7 @@ impl CurrentDepGraph { .map(|i| prev_index_to_index[i].unwrap()) .collect(); let fingerprint = prev_graph.fingerprint_by_index(prev_index); - let dep_node_index = self.encoder.borrow().send(key, fingerprint, edges); + let dep_node_index = self.encoder.send(key, fingerprint, edges); prev_index_to_index[prev_index] = Some(dep_node_index); #[cfg(debug_assertions)] self.record_edge(dep_node_index, key, fingerprint); diff --git a/compiler/rustc_query_system/src/dep_graph/serialized.rs b/compiler/rustc_query_system/src/dep_graph/serialized.rs index d8db94defbdf4..cc823917ce8ce 100644 --- a/compiler/rustc_query_system/src/dep_graph/serialized.rs +++ b/compiler/rustc_query_system/src/dep_graph/serialized.rs @@ -505,7 +505,7 @@ impl EncoderState { pub struct GraphEncoder { profiler: SelfProfilerRef, - status: Lock>, + status: Lock>>, record_graph: Option>, } @@ -518,7 +518,7 @@ impl GraphEncoder { profiler: &SelfProfilerRef, ) -> Self { let record_graph = record_graph.then(|| Lock::new(DepGraphQuery::new(prev_node_count))); - let status = Lock::new(EncoderState::new(encoder, record_stats)); + let status = Lock::new(Some(EncoderState::new(encoder, record_stats))); GraphEncoder { status, record_graph, profiler: profiler.clone() } } @@ -533,7 +533,8 @@ impl GraphEncoder { total_read_count: u64, total_duplicate_read_count: u64, ) { - let status = self.status.lock(); + let mut status = self.status.lock(); + let status = status.as_mut().unwrap(); if let Some(record_stats) = &status.stats { let mut stats: Vec<_> = record_stats.values().collect(); stats.sort_by_key(|s| -(s.node_counter as i64)); @@ -588,11 +589,12 @@ impl GraphEncoder { ) -> DepNodeIndex { let _prof_timer = self.profiler.generic_activity("incr_comp_encode_dep_graph"); let node = NodeInfo { node, fingerprint, edges }; - self.status.lock().encode_node(&node, &self.record_graph) + self.status.lock().as_mut().unwrap().encode_node(&node, &self.record_graph) } - pub fn finish(self) -> FileEncodeResult { - let _prof_timer = self.profiler.generic_activity("incr_comp_encode_dep_graph"); - self.status.into_inner().finish(&self.profiler) + pub fn finish(&self) -> FileEncodeResult { + let _prof_timer = self.profiler.generic_activity("incr_comp_encode_dep_graph_finish"); + + self.status.lock().take().unwrap().finish(&self.profiler) } }