From 0b6923ba8ea2c5437effa831efe629f11c0c9929 Mon Sep 17 00:00:00 2001 From: Daria Sukhonina Date: Fri, 19 Dec 2025 15:17:18 +0300 Subject: [PATCH] Allow green-red query coloring race and mark no_hash queries green if deps are green too --- compiler/rustc_data_structures/src/steal.rs | 2 +- compiler/rustc_middle/src/dep_graph/mod.rs | 4 +- compiler/rustc_query_impl/src/plumbing.rs | 25 +- .../src/dep_graph/dep_node.rs | 11 +- .../rustc_query_system/src/dep_graph/graph.rs | 225 +++++++++++------- .../rustc_query_system/src/dep_graph/mod.rs | 10 +- .../src/dep_graph/serialized.rs | 106 +++++---- .../rustc_query_system/src/query/plumbing.rs | 26 +- 8 files changed, 254 insertions(+), 155 deletions(-) diff --git a/compiler/rustc_data_structures/src/steal.rs b/compiler/rustc_data_structures/src/steal.rs index afa9bc36f2c5e..0e354305c965e 100644 --- a/compiler/rustc_data_structures/src/steal.rs +++ b/compiler/rustc_data_structures/src/steal.rs @@ -55,7 +55,7 @@ impl Steal { #[track_caller] pub fn steal(&self) -> T { - let value_ref = &mut *self.value.try_write().expect("stealing value which is locked"); + let mut value_ref = self.value.write(); let value = value_ref.take(); value.expect("attempt to steal from stolen value") } diff --git a/compiler/rustc_middle/src/dep_graph/mod.rs b/compiler/rustc_middle/src/dep_graph/mod.rs index 781e3e442e64c..1334c8c4c8b95 100644 --- a/compiler/rustc_middle/src/dep_graph/mod.rs +++ b/compiler/rustc_middle/src/dep_graph/mod.rs @@ -12,8 +12,8 @@ pub use dep_node::{DepKind, DepNode, DepNodeExt, dep_kinds, label_strs}; pub(crate) use dep_node::{make_compile_codegen_unit, make_compile_mono_item, make_metadata}; pub use rustc_query_system::dep_graph::debug::{DepNodeFilter, EdgeFilter}; pub use rustc_query_system::dep_graph::{ - DepContext, DepGraphQuery, DepNodeIndex, Deps, SerializedDepGraph, SerializedDepNodeIndex, - TaskDepsRef, WorkProduct, WorkProductId, WorkProductMap, hash_result, + DepContext, DepGraphQuery, DepNodeIndex, Deps, MarkFrame, SerializedDepGraph, + SerializedDepNodeIndex, TaskDepsRef, WorkProduct, WorkProductId, WorkProductMap, hash_result, }; pub type DepGraph = rustc_query_system::dep_graph::DepGraph; diff --git a/compiler/rustc_query_impl/src/plumbing.rs b/compiler/rustc_query_impl/src/plumbing.rs index 39b6fac4ebc0b..acd04f33362c6 100644 --- a/compiler/rustc_query_impl/src/plumbing.rs +++ b/compiler/rustc_query_impl/src/plumbing.rs @@ -13,8 +13,8 @@ use rustc_hir::limit::Limit; use rustc_index::Idx; use rustc_middle::bug; use rustc_middle::dep_graph::{ - self, DepContext, DepKind, DepKindStruct, DepNode, DepNodeIndex, SerializedDepNodeIndex, - dep_kinds, + self, DepContext, DepKind, DepKindStruct, DepNode, DepNodeIndex, MarkFrame, + SerializedDepNodeIndex, dep_kinds, }; use rustc_middle::query::Key; use rustc_middle::query::on_disk_cache::{ @@ -489,7 +489,12 @@ where value } -fn force_from_dep_node<'tcx, Q>(query: Q, tcx: TyCtxt<'tcx>, dep_node: DepNode) -> bool +fn force_from_dep_node<'tcx, Q>( + query: Q, + tcx: TyCtxt<'tcx>, + dep_node: DepNode, + frame: &MarkFrame<'_>, +) -> bool where Q: QueryConfig>, { @@ -512,7 +517,7 @@ where ); if let Some(key) = Q::Key::recover(tcx, &dep_node) { - force_query(query, QueryCtxt::new(tcx), key, dep_node); + force_query(query, QueryCtxt::new(tcx), key, dep_node, frame); true } else { false @@ -540,8 +545,8 @@ where is_anon, is_eval_always, fingerprint_style, - force_from_dep_node: Some(|tcx, dep_node, _| { - force_from_dep_node(Q::config(tcx), tcx, dep_node) + force_from_dep_node: Some(|tcx, dep_node, _, frame| { + force_from_dep_node(Q::config(tcx), tcx, dep_node, frame) }), try_load_from_on_disk_cache: Some(|tcx, dep_node| { try_load_from_on_disk_cache(Q::config(tcx), tcx, dep_node) @@ -853,7 +858,7 @@ macro_rules! define_queries { is_anon: false, is_eval_always: false, fingerprint_style: FingerprintStyle::Unit, - force_from_dep_node: Some(|_, dep_node, _| bug!("force_from_dep_node: encountered {:?}", dep_node)), + force_from_dep_node: Some(|_, dep_node, _, _| bug!("force_from_dep_node: encountered {:?}", dep_node)), try_load_from_on_disk_cache: None, name: &"Null", } @@ -865,7 +870,7 @@ macro_rules! define_queries { is_anon: false, is_eval_always: false, fingerprint_style: FingerprintStyle::Unit, - force_from_dep_node: Some(|_, dep_node, _| bug!("force_from_dep_node: encountered {:?}", dep_node)), + force_from_dep_node: Some(|_, dep_node, _, _| bug!("force_from_dep_node: encountered {:?}", dep_node)), try_load_from_on_disk_cache: None, name: &"Red", } @@ -876,7 +881,7 @@ macro_rules! define_queries { is_anon: false, is_eval_always: false, fingerprint_style: FingerprintStyle::Unit, - force_from_dep_node: Some(|tcx, _, prev_index| { + force_from_dep_node: Some(|tcx, _, prev_index, _| { tcx.dep_graph.force_diagnostic_node(QueryCtxt::new(tcx), prev_index); true }), @@ -890,7 +895,7 @@ macro_rules! define_queries { is_anon: true, is_eval_always: false, fingerprint_style: FingerprintStyle::Opaque, - force_from_dep_node: Some(|_, _, _| bug!("cannot force an anon node")), + force_from_dep_node: Some(|_, _, _, _| bug!("cannot force an anon node")), try_load_from_on_disk_cache: None, name: &"AnonZeroDeps", } diff --git a/compiler/rustc_query_system/src/dep_graph/dep_node.rs b/compiler/rustc_query_system/src/dep_graph/dep_node.rs index bdd1d5f3e88a9..178ec6a55cbc8 100644 --- a/compiler/rustc_query_system/src/dep_graph/dep_node.rs +++ b/compiler/rustc_query_system/src/dep_graph/dep_node.rs @@ -65,6 +65,7 @@ use rustc_hir::definitions::DefPathHash; use rustc_macros::{Decodable, Encodable}; use super::{DepContext, FingerprintStyle, SerializedDepNodeIndex}; +use crate::dep_graph::graph::MarkFrame; use crate::ich::StableHashingContext; /// This serves as an index into arrays built by `make_dep_kind_array`. @@ -276,8 +277,14 @@ pub struct DepKindStruct { /// with kind `MirValidated`, we know that the GUID/fingerprint of the `DepNode` /// is actually a `DefPathHash`, and can therefore just look up the corresponding /// `DefId` in `tcx.def_path_hash_to_def_id`. - pub force_from_dep_node: - Option bool>, + pub force_from_dep_node: Option< + fn( + tcx: Tcx, + dep_node: DepNode, + prev_index: SerializedDepNodeIndex, + frame: &MarkFrame<'_>, + ) -> bool, + >, /// Invoke a query to put the on-disk cached value in memory. pub try_load_from_on_disk_cache: Option, diff --git a/compiler/rustc_query_system/src/dep_graph/graph.rs b/compiler/rustc_query_system/src/dep_graph/graph.rs index 8634274c3a751..aeb7500b763aa 100644 --- a/compiler/rustc_query_system/src/dep_graph/graph.rs +++ b/compiler/rustc_query_system/src/dep_graph/graph.rs @@ -26,6 +26,7 @@ use super::query::DepGraphQuery; use super::serialized::{GraphEncoder, SerializedDepGraph, SerializedDepNodeIndex}; use super::{DepContext, DepKind, DepNode, Deps, HasDepContext, WorkProductId}; use crate::dep_graph::edges::EdgesVec; +use crate::dep_graph::serialized::EdgeTargetsIter; use crate::ich::StableHashingContext; use crate::query::{QueryContext, QuerySideEffect}; @@ -66,13 +67,23 @@ pub struct MarkFrame<'a> { parent: Option<&'a MarkFrame<'a>>, } -#[derive(Debug)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(super) enum DepNodeColor { Green(DepNodeIndex), - Red, + Red(() /* DepNodeIndex */), Unknown, } +impl DepNodeColor { + pub(super) fn as_green(self) -> Option { + match self { + DepNodeColor::Green(dep_node_index) => Some(dep_node_index), + DepNodeColor::Red(_) => None, + DepNodeColor::Unknown => None, + } + } +} + pub(crate) struct DepGraphData { /// The new encoding of the dependency graph, optimized for red/green /// tracking. The `current` field is the dependency graph of only the @@ -139,9 +150,12 @@ impl DepGraph { ); assert_eq!(red_node_index, DepNodeIndex::FOREVER_RED_NODE); if prev_graph_node_count > 0 { - colors.insert_red(SerializedDepNodeIndex::from_u32( - DepNodeIndex::FOREVER_RED_NODE.as_u32(), - )); + colors + .try_mark_red( + SerializedDepNodeIndex::from_u32(DepNodeIndex::FOREVER_RED_NODE.as_u32()), + DepNodeIndex::FOREVER_RED_NODE, + ) + .unwrap(); } DepGraph { @@ -260,7 +274,7 @@ impl DepGraph { hash_result: Option, &R) -> Fingerprint>, ) -> (R, DepNodeIndex) { match self.data() { - Some(data) => data.with_task(key, cx, arg, task, hash_result), + Some(data) => data.with_task(key, cx, arg, task, hash_result, None), None => (task(cx, arg), self.next_virtual_depnode_index()), } } @@ -321,6 +335,7 @@ impl DepGraphData { arg: A, task: fn(Ctxt, A) -> R, hash_result: Option, &R) -> Fingerprint>, + frame: Option<&MarkFrame<'_>>, ) -> (R, DepNodeIndex) { // If the following assertion triggers, it can have two reasons: // 1. Something is wrong with DepNode creation, either here or @@ -348,7 +363,8 @@ impl DepGraphData { }; let dcx = cx.dep_context(); - let dep_node_index = self.hash_result_and_alloc_node(dcx, key, edges, &result, hash_result); + let dep_node_index = + self.hash_result_and_alloc_node(*dcx, key, edges, &result, hash_result, frame, false); (result, dep_node_index) } @@ -435,17 +451,20 @@ impl DepGraphData { /// Intern the new `DepNode` with the dependencies up-to-now. fn hash_result_and_alloc_node, R>( &self, - cx: &Ctxt, + cx: Ctxt, node: DepNode, edges: EdgesVec, result: &R, hash_result: Option, &R) -> Fingerprint>, + frame: Option<&MarkFrame<'_>>, + feed: bool, ) -> DepNodeIndex { let hashing_timer = cx.profiler().incr_result_hashing(); let current_fingerprint = hash_result.map(|hash_result| { cx.with_stable_hashing_context(|mut hcx| hash_result(&mut hcx, result)) }); - let dep_node_index = self.alloc_and_color_node(node, edges, current_fingerprint); + let dep_node_index = + self.alloc_and_color_node(cx, node, edges, current_fingerprint, frame, feed); hashing_timer.finish_with_query_invocation_id(dep_node_index.into()); dep_node_index } @@ -563,8 +582,8 @@ impl DepGraph { // // For sanity, we still check that the loaded stable hash and the new one match. if let Some(prev_index) = data.previous.node_to_index_opt(&node) { - let dep_node_index = data.colors.current(prev_index); - if let Some(dep_node_index) = dep_node_index { + let color = data.colors.get(prev_index); + if let DepNodeColor::Green(dep_node_index) = color { crate::query::incremental_verify_ich( cx, data, @@ -585,6 +604,7 @@ impl DepGraph { return dep_node_index; } + debug_assert_eq!(DepNodeColor::Unknown, color); } let mut edges = EdgesVec::new(); @@ -599,7 +619,7 @@ impl DepGraph { } }); - data.hash_result_and_alloc_node(&cx, node, edges, result, hash_result) + data.hash_result_and_alloc_node(cx, node, edges, result, hash_result, None, true) } else { // Incremental compilation is turned off. We just execute the task // without tracking. We still provide a dep-node index that uniquely @@ -711,17 +731,21 @@ impl DepGraphData { Fingerprint::ZERO, std::iter::once(DepNodeIndex::FOREVER_RED_NODE).collect(), true, + false, ); // This will just overwrite the same value for concurrent calls. qcx.store_side_effect(dep_node_index, side_effect); }) } - fn alloc_and_color_node( + fn alloc_and_color_node>( &self, + cx: Ctxt, key: DepNode, edges: EdgesVec, fingerprint: Option, + frame: Option<&MarkFrame<'_>>, + feed: bool, ) -> DepNodeIndex { if let Some(prev_index) = self.previous.node_to_index_opt(&key) { // Determine the color and index of the new `DepNode`. @@ -733,13 +757,22 @@ impl DepGraphData { } else { // This is a red node: it existed in the previous compilation, its query // was re-executed, but it has a different result from before. + if !feed && !cx.is_eval_always(key.kind) { + debug_assert_eq!( + self.try_mark_previous_green(cx, prev_index, &key, frame), + None + ); + } + false } } else { - // This is a red node, effectively: it existed in the previous compilation - // session, its query was re-executed, but it doesn't compute a result hash - // (i.e. it represents a `no_hash` query), so we have no way of determining - // whether or not the result was the same as before. + if !feed && !cx.is_eval_always(key.kind) { + if let Some(green) = self.try_mark_previous_green(cx, prev_index, &key, frame) { + return green; + } + } + false }; @@ -752,6 +785,7 @@ impl DepGraphData { fingerprint, edges, is_green, + feed, ); self.current.record_node(dep_node_index, key, fingerprint); @@ -762,10 +796,13 @@ impl DepGraphData { } } - fn promote_node_and_deps_to_current(&self, prev_index: SerializedDepNodeIndex) -> DepNodeIndex { - self.current.debug_assert_not_in_new_nodes(&self.previous, prev_index); - - let dep_node_index = self.current.encoder.send_promoted(prev_index, &self.colors); + fn promote_node_and_deps_to_current( + &self, + prev_index: SerializedDepNodeIndex, + prev_deps: EdgeTargetsIter<'_>, + ) -> DepNodeIndex { + let dep_node_index = + self.current.encoder.send_promoted(prev_index, prev_deps, &self.colors); #[cfg(debug_assertions)] self.current.record_edge( @@ -849,22 +886,22 @@ impl DepGraphData { match self.colors.get(prev_index) { DepNodeColor::Green(dep_node_index) => Some((prev_index, dep_node_index)), - DepNodeColor::Red => None, + DepNodeColor::Red(_) => None, DepNodeColor::Unknown => { // This DepNode and the corresponding query invocation existed // in the previous compilation session too, so we can try to // mark it as green by recursively marking all of its // dependencies green. - self.try_mark_previous_green(qcx, prev_index, dep_node, None) + self.try_mark_previous_green(*qcx.dep_context(), prev_index, dep_node, None) .map(|dep_node_index| (prev_index, dep_node_index)) } } } - #[instrument(skip(self, qcx, parent_dep_node_index, frame), level = "debug")] - fn try_mark_parent_green>( + #[instrument(skip(self, cx, parent_dep_node_index, frame), level = "debug")] + fn try_mark_parent_green>( &self, - qcx: Qcx, + cx: Ctxt, parent_dep_node_index: SerializedDepNodeIndex, frame: &MarkFrame<'_>, ) -> Option<()> { @@ -882,7 +919,7 @@ impl DepGraphData { debug!("dependency {:?} was immediately green", get_dep_dep_node()); return Some(()); } - DepNodeColor::Red => { + DepNodeColor::Red(_) => { // We found a dependency the value of which has changed // compared to the previous compilation session. We cannot // mark the DepNode as green and also don't need to bother @@ -897,14 +934,14 @@ impl DepGraphData { // We don't know the state of this dependency. If it isn't // an eval_always node, let's try to mark it green recursively. - if !qcx.dep_context().is_eval_always(dep_dep_node.kind) { + if !cx.is_eval_always(dep_dep_node.kind) { debug!( "state of dependency {:?} ({}) is unknown, trying to mark it green", dep_dep_node, dep_dep_node.hash, ); let node_index = - self.try_mark_previous_green(qcx, parent_dep_node_index, dep_dep_node, Some(frame)); + self.try_mark_previous_green(cx, parent_dep_node_index, dep_dep_node, Some(frame)); if node_index.is_some() { debug!("managed to MARK dependency {dep_dep_node:?} as green"); @@ -914,7 +951,7 @@ impl DepGraphData { // We failed to mark it green, so we try to force the query. debug!("trying to force dependency {dep_dep_node:?}"); - if !qcx.dep_context().try_force_from_dep_node(*dep_dep_node, parent_dep_node_index, frame) { + if !cx.try_force_from_dep_node(*dep_dep_node, parent_dep_node_index, frame) { // The DepNode could not be forced. debug!("dependency {dep_dep_node:?} could not be forced"); return None; @@ -925,14 +962,14 @@ impl DepGraphData { debug!("managed to FORCE dependency {dep_dep_node:?} to green"); return Some(()); } - DepNodeColor::Red => { + DepNodeColor::Red(_) => { debug!("dependency {dep_dep_node:?} was red after forcing"); return None; } DepNodeColor::Unknown => {} } - if let None = qcx.dep_context().sess().dcx().has_errors_or_delayed_bugs() { + if let None = cx.sess().dcx().has_errors_or_delayed_bugs() { panic!("try_mark_previous_green() - Forcing the DepNode should have set its color") } @@ -951,10 +988,10 @@ impl DepGraphData { } /// Try to mark a dep-node which existed in the previous compilation session as green. - #[instrument(skip(self, qcx, prev_dep_node_index, frame), level = "debug")] - fn try_mark_previous_green>( + #[instrument(skip(self, cx, prev_dep_node_index, frame), level = "debug")] + fn try_mark_previous_green>( &self, - qcx: Qcx, + cx: Ctxt, prev_dep_node_index: SerializedDepNodeIndex, dep_node: &DepNode, frame: Option<&MarkFrame<'_>>, @@ -962,14 +999,14 @@ impl DepGraphData { let frame = MarkFrame { index: prev_dep_node_index, parent: frame }; // We never try to mark eval_always nodes as green - debug_assert!(!qcx.dep_context().is_eval_always(dep_node.kind)); + debug_assert!(!cx.is_eval_always(dep_node.kind)); debug_assert_eq!(self.previous.index_to_node(prev_dep_node_index), *dep_node); let prev_deps = self.previous.edge_targets_from(prev_dep_node_index); - for dep_dep_node_index in prev_deps { - self.try_mark_parent_green(qcx, dep_dep_node_index, &frame)?; + for dep_dep_node_index in prev_deps.clone() { + self.try_mark_parent_green(cx, dep_dep_node_index, &frame)?; } // If we got here without hitting a `return` that means that all @@ -980,7 +1017,7 @@ 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.promote_node_and_deps_to_current(prev_dep_node_index); + let dep_node_index = self.promote_node_and_deps_to_current(prev_dep_node_index, prev_deps); // ... and finally storing a "Green" entry in the color map. // Multiple threads can all write the same color here @@ -994,7 +1031,7 @@ impl DepGraph { /// Returns true if the given node has been marked as red during the /// current compilation session. Used in various assertions pub fn is_red(&self, dep_node: &DepNode) -> bool { - matches!(self.node_color(dep_node), DepNodeColor::Red) + matches!(self.node_color(dep_node), DepNodeColor::Red(_)) } /// Returns true if the given node has been marked as green during the @@ -1031,7 +1068,7 @@ impl DepGraph { let dep_node = data.previous.index_to_node(prev_index); tcx.try_load_from_on_disk_cache(dep_node); } - DepNodeColor::Unknown | DepNodeColor::Red => { + DepNodeColor::Unknown | DepNodeColor::Red(_) => { // We can skip red nodes because a node can only be marked // as red if the query result was recomputed and thus is // already in memory. @@ -1252,22 +1289,6 @@ impl CurrentDepGraph { dep_node_index } - - #[inline] - fn debug_assert_not_in_new_nodes( - &self, - prev_graph: &SerializedDepGraph, - prev_index: SerializedDepNodeIndex, - ) { - if let Some(ref nodes_in_current_session) = self.nodes_in_current_session { - debug_assert!( - !nodes_in_current_session - .lock() - .contains_key(&prev_graph.index_to_node(prev_index)), - "node from previous graph present in new node collection" - ); - } - } } #[derive(Debug, Clone, Copy)] @@ -1331,19 +1352,26 @@ pub(super) struct DepNodeColorMap { } // All values below `COMPRESSED_RED` are green. -const COMPRESSED_RED: u32 = u32::MAX - 1; +const COMPRESSED_RED_BIT: u32 = 1 << 31; const COMPRESSED_UNKNOWN: u32 = u32::MAX; impl DepNodeColorMap { fn new(size: usize) -> DepNodeColorMap { - debug_assert!(COMPRESSED_RED > DepNodeIndex::MAX_AS_U32); - DepNodeColorMap { values: (0..size).map(|_| AtomicU32::new(COMPRESSED_UNKNOWN)).collect() } + DepNodeColorMap { + values: IndexVec::from_fn_n(|_| AtomicU32::new(COMPRESSED_UNKNOWN), size), + } } #[inline] - pub(super) fn current(&self, index: SerializedDepNodeIndex) -> Option { - let value = self.values[index].load(Ordering::Relaxed); - if value <= DepNodeIndex::MAX_AS_U32 { Some(DepNodeIndex::from_u32(value)) } else { None } + pub(super) fn current(&self, index: SerializedDepNodeIndex) -> DepNodeColor { + let value = self.values[index].load(Ordering::Acquire); + if value == COMPRESSED_UNKNOWN { + DepNodeColor::Unknown + } else if value & COMPRESSED_RED_BIT == 0 { + DepNodeColor::Green(DepNodeIndex::from_u32(value)) + } else { + DepNodeColor::Red(()) + } } /// This tries to atomically mark a node green and assign `index` as the new @@ -1355,41 +1383,56 @@ impl DepNodeColorMap { prev_index: SerializedDepNodeIndex, index: DepNodeIndex, ) -> Result<(), DepNodeIndex> { - let value = &self.values[prev_index]; - match value.compare_exchange( - COMPRESSED_UNKNOWN, - index.as_u32(), - Ordering::Relaxed, - Ordering::Relaxed, - ) { + use Ordering::*; + + debug_assert!(index != DepNodeIndex::FOREVER_RED_NODE); + + let index = index.as_u32(); + assert_eq!(index & COMPRESSED_RED_BIT, 0); + let green = index; + + match self.values[prev_index].compare_exchange(COMPRESSED_UNKNOWN, green, Release, Acquire) + { Ok(_) => Ok(()), - Err(v) => Err({ - assert_ne!(v, COMPRESSED_RED, "tried to mark a red node as green"); - DepNodeIndex::from_u32(v) - }), + Err(red) if red & COMPRESSED_RED_BIT != 0 => { + let green = red & !COMPRESSED_RED_BIT; + if let Err(other) = + self.values[prev_index].compare_exchange(red, green, Release, Acquire) + { + assert_eq!(other, green) + } + + Err(DepNodeIndex::from_u32(green)) + } + Err(green) => Err(DepNodeIndex::from_u32(green)), } } #[inline] pub(super) fn get(&self, index: SerializedDepNodeIndex) -> DepNodeColor { - let value = self.values[index].load(Ordering::Acquire); - // Green is by far the most common case. Check for that first so we can succeed with a - // single comparison. - if value < COMPRESSED_RED { - DepNodeColor::Green(DepNodeIndex::from_u32(value)) - } else if value == COMPRESSED_RED { - DepNodeColor::Red - } else { - debug_assert_eq!(value, COMPRESSED_UNKNOWN); - DepNodeColor::Unknown - } + self.current(index) } #[inline] - pub(super) fn insert_red(&self, index: SerializedDepNodeIndex) { - let value = self.values[index].swap(COMPRESSED_RED, Ordering::Release); - // Sanity check for duplicate nodes - assert_eq!(value, COMPRESSED_UNKNOWN, "trying to encode a dep node twice"); + pub(super) fn try_mark_red( + &self, + prev_index: SerializedDepNodeIndex, + index: DepNodeIndex, + ) -> Result<(), DepNodeIndex> { + use Ordering::*; + + let index = index.as_u32(); + assert_eq!(index & COMPRESSED_RED_BIT, 0); + let red = index | COMPRESSED_RED_BIT; + + match self.values[prev_index].compare_exchange(COMPRESSED_UNKNOWN, red, Release, Acquire) { + Ok(_) => Ok(()), + // Sanity check for duplicate nodes + Err(red) if red & COMPRESSED_RED_BIT != 0 => { + panic!("trying to encode a dep node twice for index {prev_index:?}") + } + Err(green) => Err(DepNodeIndex::from_u32(green)), + } } } @@ -1424,7 +1467,9 @@ fn panic_on_forbidden_read(data: &DepGraphData, dep_node_index: DepN // First try to find the dep node among those that already existed in the // previous session and has been marked green for prev_index in data.colors.values.indices() { - if data.colors.current(prev_index) == Some(dep_node_index) { + if let DepNodeColor::Green(c) = data.colors.current(prev_index) + && c == dep_node_index + { dep_node = Some(data.previous.index_to_node(prev_index)); break; } diff --git a/compiler/rustc_query_system/src/dep_graph/mod.rs b/compiler/rustc_query_system/src/dep_graph/mod.rs index d648415c9fc67..612b823bbb7cc 100644 --- a/compiler/rustc_query_system/src/dep_graph/mod.rs +++ b/compiler/rustc_query_system/src/dep_graph/mod.rs @@ -9,7 +9,9 @@ use std::panic; pub use dep_node::{DepKind, DepKindStruct, DepNode, DepNodeParams, WorkProductId}; pub(crate) use graph::DepGraphData; -pub use graph::{DepGraph, DepNodeIndex, TaskDepsRef, WorkProduct, WorkProductMap, hash_result}; +pub use graph::{ + DepGraph, DepNodeIndex, MarkFrame, TaskDepsRef, WorkProduct, WorkProductMap, hash_result, +}; pub use query::DepGraphQuery; use rustc_data_structures::profiling::SelfProfilerRef; use rustc_data_structures::sync::DynSync; @@ -17,7 +19,7 @@ use rustc_session::Session; pub use serialized::{SerializedDepGraph, SerializedDepNodeIndex}; use tracing::instrument; -use self::graph::{MarkFrame, print_markframe_trace}; +use self::graph::print_markframe_trace; use crate::ich::StableHashingContext; pub trait DepContext: Copy { @@ -67,7 +69,9 @@ pub trait DepContext: Copy { ) -> bool { let cb = self.dep_kind_info(dep_node.kind); if let Some(f) = cb.force_from_dep_node { - match panic::catch_unwind(panic::AssertUnwindSafe(|| f(self, dep_node, prev_index))) { + match panic::catch_unwind(panic::AssertUnwindSafe(|| { + f(self, dep_node, prev_index, frame) + })) { Err(value) => { if !value.is::() { print_markframe_trace(self.dep_graph(), frame); diff --git a/compiler/rustc_query_system/src/dep_graph/serialized.rs b/compiler/rustc_query_system/src/dep_graph/serialized.rs index 0012bf79a1f5d..eb0e2ac0e77e8 100644 --- a/compiler/rustc_query_system/src/dep_graph/serialized.rs +++ b/compiler/rustc_query_system/src/dep_graph/serialized.rs @@ -109,27 +109,53 @@ pub struct SerializedDepGraph { session_count: u64, } +#[derive(Clone)] +pub struct EdgeTargetsIter<'a> { + edges_left: u32, + raw: &'a [u8], + bytes_per_index: usize, + mask: u32, +} + +impl<'a> ExactSizeIterator for EdgeTargetsIter<'a> { + fn len(&self) -> usize { + self.edges_left.try_into().unwrap() + } +} + +impl<'a> Iterator for EdgeTargetsIter<'a> { + type Item = SerializedDepNodeIndex; + + fn next(&mut self) -> Option { + if self.edges_left == 0 { + return None; + } + self.edges_left -= 1; + // Doing this slicing in this order ensures that the first bounds check suffices for + // all the others. + let index = &self.raw[..DEP_NODE_SIZE]; + self.raw = &self.raw[self.bytes_per_index..]; + let index = u32::from_le_bytes(index.try_into().unwrap()) & self.mask; + Some(SerializedDepNodeIndex::from_u32(index)) + } + + fn size_hint(&self) -> (usize, Option) { + let len = self.len(); + (len, Some(len)) + } +} + impl SerializedDepGraph { #[inline] - pub fn edge_targets_from( - &self, - source: SerializedDepNodeIndex, - ) -> impl Iterator + Clone { + pub fn edge_targets_from<'a>(&'a self, source: SerializedDepNodeIndex) -> EdgeTargetsIter<'a> { let header = self.edge_list_indices[source]; - let mut raw = &self.edge_list_data[header.start()..]; + let raw = &self.edge_list_data[header.start()..]; let bytes_per_index = header.bytes_per_index(); // LLVM doesn't hoist EdgeHeader::mask so we do it ourselves. let mask = header.mask(); - (0..header.num_edges).map(move |_| { - // Doing this slicing in this order ensures that the first bounds check suffices for - // all the others. - let index = &raw[..DEP_NODE_SIZE]; - raw = &raw[bytes_per_index..]; - let index = u32::from_le_bytes(index.try_into().unwrap()) & mask; - SerializedDepNodeIndex::from_u32(index) - }) + EdgeTargetsIter { edges_left: header.num_edges, raw, bytes_per_index, mask } } #[inline] @@ -485,16 +511,17 @@ impl NodeInfo { node: DepNode, index: DepNodeIndex, fingerprint: Fingerprint, - prev_index: SerializedDepNodeIndex, + edges: EdgeTargetsIter<'_>, colors: &DepNodeColorMap, - previous: &SerializedDepGraph, ) -> usize { - let edges = previous.edge_targets_from(prev_index); - let edge_count = edges.size_hint().0; + let edge_count = edges.len(); // Find the highest edge in the new dep node indices - let edge_max = - edges.clone().map(|i| colors.current(i).unwrap().as_u32()).max().unwrap_or(0); + let edge_max = edges + .clone() + .map(|i| colors.current(i).as_green().unwrap().as_u32()) + .max() + .unwrap_or(0); let header = SerializedNodeHeader::::new(node, index, fingerprint, edge_max, edge_count); e.write_array(header.bytes); @@ -506,7 +533,7 @@ impl NodeInfo { let bytes_per_index = header.bytes_per_index(); for node_index in edges { - let node_index = colors.current(node_index).unwrap(); + let node_index = colors.current(node_index).as_green().unwrap(); e.write_with(|dest| { *dest = node_index.as_u32().to_le_bytes(); bytes_per_index @@ -681,6 +708,7 @@ impl EncoderState { &self, index: DepNodeIndex, prev_index: SerializedDepNodeIndex, + prev_deps: EdgeTargetsIter<'_>, record_graph: &Option>, colors: &DepNodeColorMap, local: &mut LocalEncoderState, @@ -692,21 +720,15 @@ impl EncoderState { node, index, fingerprint, - prev_index, + prev_deps.clone(), colors, - &self.previous, ); self.flush_mem_encoder(&mut *local); self.record( node, index, edge_count, - |this| { - this.previous - .edge_targets_from(prev_index) - .map(|i| colors.current(i).unwrap()) - .collect() - }, + |_| prev_deps.map(|i| colors.current(i).as_green().unwrap()).collect(), record_graph, &mut *local, ); @@ -890,6 +912,7 @@ impl GraphEncoder { fingerprint: Fingerprint, edges: EdgesVec, is_green: bool, + feed: bool, ) -> DepNodeIndex { let _prof_timer = self.profiler.generic_activity("incr_comp_encode_dep_graph"); let node = NodeInfo { node, fingerprint, edges }; @@ -898,20 +921,21 @@ impl GraphEncoder { let index = self.status.next_index(&mut *local); - if is_green { - // Use `try_mark_green` to avoid racing when `send_promoted` is called concurrently - // on the same index. - match colors.try_mark_green(prev_index, index) { - Ok(()) => (), - Err(dep_node_index) => return dep_node_index, - } + let res = if is_green { + colors.try_mark_green(prev_index, index) } else { - colors.insert_red(prev_index); + colors + .try_mark_red(prev_index, index) + .inspect_err(|_| assert!(!feed, "tried to feed a green query {node:?}")) + }; + match res { + Ok(()) => { + self.status.bump_index(&mut *local); + self.status.encode_node(index, &node, &self.record_graph, &mut *local); + index + } + Err(dep_node_index) => dep_node_index, } - - self.status.bump_index(&mut *local); - self.status.encode_node(index, &node, &self.record_graph, &mut *local); - index } /// Encodes a node that was promoted from the previous graph. It reads the information directly @@ -923,6 +947,7 @@ impl GraphEncoder { pub(crate) fn send_promoted( &self, prev_index: SerializedDepNodeIndex, + prev_deps: EdgeTargetsIter<'_>, colors: &DepNodeColorMap, ) -> DepNodeIndex { let _prof_timer = self.profiler.generic_activity("incr_comp_encode_dep_graph"); @@ -938,6 +963,7 @@ impl GraphEncoder { self.status.encode_promoted_node( index, prev_index, + prev_deps, &self.record_graph, colors, &mut *local, diff --git a/compiler/rustc_query_system/src/query/plumbing.rs b/compiler/rustc_query_system/src/query/plumbing.rs index dea47c8fa787e..0059c32dadd08 100644 --- a/compiler/rustc_query_system/src/query/plumbing.rs +++ b/compiler/rustc_query_system/src/query/plumbing.rs @@ -20,7 +20,7 @@ use tracing::instrument; use super::{QueryConfig, QueryStackFrameExtra}; use crate::HandleCycleError; -use crate::dep_graph::{DepContext, DepGraphData, DepNode, DepNodeIndex, DepNodeParams}; +use crate::dep_graph::{DepContext, DepGraphData, DepNode, DepNodeIndex, DepNodeParams, MarkFrame}; use crate::ich::StableHashingContext; use crate::query::caches::QueryCache; use crate::query::job::{QueryInfo, QueryJob, QueryJobId, QueryJobInfo, QueryLatch, report_cycle}; @@ -347,6 +347,7 @@ fn try_execute_query( span: Span, key: Q::Key, dep_node: Option, + frame: Option<&MarkFrame<'_>>, ) -> (Q::Value, Option) where Q: QueryConfig, @@ -382,7 +383,7 @@ where // Drop the lock before we start executing the query drop(state_lock); - execute_job::<_, _, INCR>(query, qcx, state, key, key_hash, id, dep_node) + execute_job::<_, _, INCR>(query, qcx, state, key, key_hash, id, dep_node, frame) } Entry::Occupied(mut entry) => { match &mut entry.get_mut().1 { @@ -419,6 +420,7 @@ fn execute_job( key_hash: u64, id: QueryJobId, dep_node: Option, + frame: Option<&MarkFrame<'_>>, ) -> (Q::Value, Option) where Q: QueryConfig, @@ -437,6 +439,7 @@ where key, dep_node, id, + frame, ) } else { execute_job_non_incr(query, qcx, key, id) @@ -528,6 +531,7 @@ fn execute_job_incr( key: Q::Key, mut dep_node_opt: Option, job_id: QueryJobId, + frame: Option<&MarkFrame<'_>>, ) -> (Q::Value, DepNodeIndex) where Q: QueryConfig, @@ -568,6 +572,7 @@ where key, |(qcx, query), key| query.compute(qcx, key), query.hash_result(), + frame, ) }); @@ -824,7 +829,9 @@ where { debug_assert!(!qcx.dep_context().dep_graph().is_fully_enabled()); - ensure_sufficient_stack(|| try_execute_query::(query, qcx, span, key, None).0) + ensure_sufficient_stack(|| { + try_execute_query::(query, qcx, span, key, None, None).0 + }) } #[inline(always)] @@ -852,7 +859,7 @@ where }; let (result, dep_node_index) = ensure_sufficient_stack(|| { - try_execute_query::<_, _, true>(query, qcx, span, key, dep_node) + try_execute_query::<_, _, true>(query, qcx, span, key, dep_node, None) }); if let Some(dep_node_index) = dep_node_index { qcx.dep_context().dep_graph().read_index(dep_node_index) @@ -860,8 +867,13 @@ where Some(result) } -pub fn force_query(query: Q, qcx: Qcx, key: Q::Key, dep_node: DepNode) -where +pub fn force_query( + query: Q, + qcx: Qcx, + key: Q::Key, + dep_node: DepNode, + frame: &MarkFrame<'_>, +) where Q: QueryConfig, Qcx: QueryContext, { @@ -875,6 +887,6 @@ where debug_assert!(!query.anon()); ensure_sufficient_stack(|| { - try_execute_query::<_, _, true>(query, qcx, DUMMY_SP, key, Some(dep_node)) + try_execute_query::<_, _, true>(query, qcx, DUMMY_SP, key, Some(dep_node), Some(frame)) }); }