Auto merge of #122064 - Zoxc:dep-graph-encode-tweaks, r=cjgillot

Dep node encoding cleanups

This does some cleanups around dep node encoding.

Performance change with `-Zthreads=2`:

<table><tr><td rowspan="2">Benchmark</td><td colspan="1"><b>Before</b></th><td colspan="2"><b>After</b></th><td colspan="1"><b>Before</b></th><td colspan="2"><b>After</b></th></tr><tr><td align="right">Time</td><td align="right">Time</td><td align="right">%</th><td align="right">Memory</td><td align="right">Memory</td><td align="right">%</th></tr><tr><td>🟣 <b>clap</b>:check:unchanged</td><td align="right">0.4337s</td><td align="right">0.4306s</td><td align="right"> -0.72%</td><td align="right">88.90 MiB</td><td align="right">89.04 MiB</td><td align="right"> 0.15%</td></tr><tr><td>🟣 <b>hyper</b>:check:unchanged</td><td align="right">0.1541s</td><td align="right">0.1528s</td><td align="right"> -0.86%</td><td align="right">51.99 MiB</td><td align="right">52.03 MiB</td><td align="right"> 0.07%</td></tr><tr><td>🟣 <b>regex</b>:check:unchanged</td><td align="right">0.3286s</td><td align="right">0.3248s</td><td align="right">💚  -1.15%</td><td align="right">71.89 MiB</td><td align="right">71.74 MiB</td><td align="right"> -0.21%</td></tr><tr><td>🟣 <b>syn</b>:check:unchanged</td><td align="right">0.6118s</td><td align="right">0.6057s</td><td align="right">💚  -1.01%</td><td align="right">106.59 MiB</td><td align="right">106.66 MiB</td><td align="right"> 0.06%</td></tr><tr><td>🟣 <b>syntex_syntax</b>:check:unchanged</td><td align="right">1.4570s</td><td align="right">1.4463s</td><td align="right"> -0.74%</td><td align="right">197.29 MiB</td><td align="right">197.33 MiB</td><td align="right"> 0.02%</td></tr><tr><td>Total</td><td align="right">2.9852s</td><td align="right">2.9601s</td><td align="right"> -0.84%</td><td align="right">516.66 MiB</td><td align="right">516.80 MiB</td><td align="right"> 0.03%</td></tr><tr><td>Summary</td><td align="right">1.0000s</td><td align="right">0.9911s</td><td align="right"> -0.89%</td><td align="right">1 byte</td><td align="right">1.00 bytes</td><td align="right"> 0.02%</td></tr></table>

r? `@cjgillot`

compiler/rustc_middle/src/ty/context.rs

@@ -709,7 +709,7 @@ pub fn enter<'a: 'tcx, F, R>(&'a self, f: F) -> R
     }
 
     pub fn finish(&self) -> FileEncodeResult {
-        self.dep_graph.finish_encoding(&self.sess.prof)
+        self.dep_graph.finish_encoding()
     }
 }
 

compiler/rustc_query_system/src/dep_graph/graph.rs

@@ -1,9 +1,8 @@
 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;
 use rustc_data_structures::sync::{AtomicU32, AtomicU64, Lock, Lrc};
 use rustc_data_structures::unord::UnordMap;
 use rustc_index::IndexVec;
@@ -134,7 +133,6 @@ pub fn new(
 
         // 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,
@@ -143,7 +141,6 @@ pub fn new(
 
         // 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(),
@@ -196,7 +193,7 @@ pub fn is_fully_enabled(&self) -> bool {
 
     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)
         }
     }
 
@@ -372,13 +369,8 @@ pub(crate) fn with_task<Ctxt: HasDepContext<Deps = D>, A: Debug, R>(
             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(
+        let (dep_node_index, prev_and_color) =
-            dcx.profiler(),
+            self.current.intern_node(&self.previous, key, edges, current_fingerprint);
-            &self.previous,
-            key,
-            edges,
-            current_fingerprint,
-        );
 
         hashing_timer.finish_with_query_invocation_id(dep_node_index.into());
 
@@ -443,12 +435,7 @@ pub(crate) fn with_anon_task<Tcx: DepContext<Deps = D>, OP, R>(
                     hash: self.current.anon_id_seed.combine(hasher.finish()).into(),
                 };
 
-                self.current.intern_new_node(
+                self.current.intern_new_node(target_dep_node, task_deps, Fingerprint::ZERO)
-                    cx.profiler(),
-                    target_dep_node,
-                    task_deps,
-                    Fingerprint::ZERO,
-                )
             }
         };
 
@@ -585,13 +572,8 @@ pub fn with_feed_task<Ctxt: DepContext<Deps = D>, A: Debug, R: Debug>(
             });
 
             // Intern the new `DepNode` with the dependencies up-to-now.
-            let (dep_node_index, prev_and_color) = data.current.intern_node(
+            let (dep_node_index, prev_and_color) =
-                cx.profiler(),
+                data.current.intern_node(&data.previous, node, edges, current_fingerprint);
-                &data.previous,
-                node,
-                edges,
-                current_fingerprint,
-            );
 
             hashing_timer.finish_with_query_invocation_id(dep_node_index.into());
 
@@ -871,11 +853,8 @@ fn try_mark_previous_green<Qcx: QueryContext<Deps = D>>(
 
         // 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(
+        let dep_node_index =
-            qcx.dep_context().profiler(),
+            self.current.promote_node_and_deps_to_current(&self.previous, prev_dep_node_index);
-            &self.previous,
-            prev_dep_node_index,
-        );
 
         // ... emitting any stored diagnostic ...
 
@@ -974,19 +953,15 @@ pub fn exec_cache_promotions<Tcx: DepContext>(&self, tcx: Tcx) {
 
     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),
             )
         }
     }
 
-    pub fn finish_encoding(&self, profiler: &SelfProfilerRef) -> FileEncodeResult {
+    pub fn finish_encoding(&self) -> FileEncodeResult {
-        if let Some(data) = &self.data {
+        if let Some(data) = &self.data { data.current.encoder.finish() } else { Ok(0) }
-            data.current.encoder.steal().finish(profiler)
-        } else {
-            Ok(0)
-        }
     }
 
     pub(crate) fn next_virtual_depnode_index(&self) -> DepNodeIndex {
@@ -1069,7 +1044,7 @@ struct EdgeIndex {}
 /// manipulating both, we acquire `new_node_to_index` or `prev_index_to_index`
 /// first, and `data` second.
 pub(super) struct CurrentDepGraph<D: Deps> {
-    encoder: Steal<GraphEncoder<D>>,
+    encoder: GraphEncoder<D>,
     new_node_to_index: Sharded<FxHashMap<DepNode, DepNodeIndex>>,
     prev_index_to_index: Lock<IndexVec<SerializedDepNodeIndex, Option<DepNodeIndex>>>,
 
@@ -1100,12 +1075,6 @@ pub(super) struct CurrentDepGraph<D: Deps> {
     /// 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<EventId>,
 }
 
 impl<D: Deps> CurrentDepGraph<D> {
@@ -1140,17 +1109,14 @@ fn new(
 
         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: 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(),
@@ -1165,7 +1131,6 @@ fn new(
             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,
         }
     }
 
@@ -1183,7 +1148,6 @@ fn record_edge(&self, dep_node_index: DepNodeIndex, key: DepNode, fingerprint: F
     #[inline(always)]
     fn intern_new_node(
         &self,
-        profiler: &SelfProfilerRef,
         key: DepNode,
         edges: EdgesVec,
         current_fingerprint: Fingerprint,
@@ -1191,8 +1155,7 @@ fn intern_new_node(
         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 =
+                let dep_node_index = self.encoder.send(key, current_fingerprint, edges);
-                    self.encoder.borrow().send(profiler, key, current_fingerprint, edges);
                 entry.insert(dep_node_index);
                 dep_node_index
             }
@@ -1206,16 +1169,11 @@ fn intern_new_node(
 
     fn intern_node(
         &self,
-        profiler: &SelfProfilerRef,
         prev_graph: &SerializedDepGraph,
         key: DepNode,
         edges: EdgesVec,
         fingerprint: Option<Fingerprint>,
     ) -> (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();
@@ -1223,8 +1181,7 @@ fn intern_node(
                 let dep_node_index = match prev_index_to_index[prev_index] {
                     Some(dep_node_index) => dep_node_index,
                     None => {
-                        let dep_node_index =
+                        let dep_node_index = self.encoder.send(key, fingerprint, edges);
-                            self.encoder.borrow().send(profiler, key, fingerprint, edges);
                         prev_index_to_index[prev_index] = Some(dep_node_index);
                         dep_node_index
                     }
@@ -1261,7 +1218,7 @@ fn intern_node(
             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 +1226,6 @@ fn intern_node(
 
     fn promote_node_and_deps_to_current(
         &self,
-        profiler: &SelfProfilerRef,
         prev_graph: &SerializedDepGraph,
         prev_index: SerializedDepNodeIndex,
     ) -> DepNodeIndex {
@@ -1286,7 +1242,7 @@ fn promote_node_and_deps_to_current(
                     .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.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);

compiler/rustc_query_system/src/dep_graph/serialized.rs

@@ -504,7 +504,8 @@ fn finish(self, profiler: &SelfProfilerRef) -> FileEncodeResult {
 }
 
 pub struct GraphEncoder<D: Deps> {
-    status: Lock<EncoderState<D>>,
+    profiler: SelfProfilerRef,
+    status: Lock<Option<EncoderState<D>>>,
     record_graph: Option<Lock<DepGraphQuery>>,
 }
 
@@ -514,10 +515,11 @@ pub fn new(
         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));
+        let status = Lock::new(Some(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)) {
@@ -531,7 +533,8 @@ pub(crate) fn print_incremental_info(
         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));
@@ -580,18 +583,18 @@ pub(crate) fn print_incremental_info(
 
     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)
+        self.status.lock().as_mut().unwrap().encode_node(&node, &self.record_graph)
     }
 
-    pub fn finish(self, profiler: &SelfProfilerRef) -> FileEncodeResult {
+    pub fn finish(&self) -> FileEncodeResult {
-        let _prof_timer = profiler.generic_activity("incr_comp_encode_dep_graph");
+        let _prof_timer = self.profiler.generic_activity("incr_comp_encode_dep_graph_finish");
-        self.status.into_inner().finish(profiler)
+
+        self.status.lock().take().unwrap().finish(&self.profiler)
     }
 }