Skip to content

Commit

Permalink
Do not fetch the DepNode to mark nodes green.
Browse files Browse the repository at this point in the history
  • Loading branch information
cjgillot committed Aug 10, 2024
1 parent 55bdbab commit 381c7a3
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 36 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1056,7 +1056,7 @@ pub fn determine_cgu_reuse<'tcx>(tcx: TyCtxt<'tcx>, cgu: &CodegenUnit<'tcx>) ->
// know that later). If we are not doing LTO, there is only one optimized
// version of each module, so we re-use that.
let dep_node = cgu.codegen_dep_node(tcx);
tcx.dep_graph.assert_nonexistent_node(&dep_node, || {
tcx.dep_graph.assert_nonexistent_node(dep_node, || {
format!(
"CompileCodegenUnit dep-node for CGU `{}` already exists before marking.",
cgu.name()
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_query_system/src/dep_graph/edges.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,13 @@ impl EdgesVec {
Self::default()
}

#[inline]
pub(crate) fn eval_always() -> Self {
let mut vec = EdgesVec::new();
vec.push(DepNodeIndex::FOREVER_RED_NODE);
vec
}

#[inline]
pub(crate) fn push(&mut self, edge: DepNodeIndex) {
self.max = self.max.max(edge.as_u32());
Expand Down
58 changes: 23 additions & 35 deletions compiler/rustc_query_system/src/dep_graph/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ impl<D: Deps> DepGraphData<D> {
task: fn(Ctxt, A) -> R,
hash_result: Option<fn(&mut StableHashingContext<'_>, &R) -> Fingerprint>,
) -> (R, DepNodeIndex) {
self.assert_nonexistent_node(&key, || {
self.assert_nonexistent_node(key, || {
format!(
"forcing query with already existing `DepNode`\n\
- query-key: {arg:?}\n\
Expand All @@ -356,7 +356,7 @@ impl<D: Deps> DepGraphData<D> {

let with_deps = |task_deps| D::with_deps(task_deps, || task(cx, arg));
let (result, edges) = if cx.dep_context().is_eval_always(key.kind) {
(with_deps(TaskDepsRef::EvalAlways), EdgesVec::new())
(with_deps(TaskDepsRef::EvalAlways), EdgesVec::eval_always())
} else {
let task_deps = Lock::new(TaskDeps {
#[cfg(debug_assertions)]
Expand Down Expand Up @@ -630,12 +630,12 @@ impl<D: Deps> DepGraph<D> {
impl<D: Deps> DepGraphData<D> {
fn assert_nonexistent_node<S: std::fmt::Display>(
&self,
_dep_node: &DepNode,
_dep_node: DepNode,
_msg: impl FnOnce() -> S,
) {
#[cfg(debug_assertions)]
if let Some(seen_dep_nodes) = &self.current.seen_dep_nodes {
let seen = seen_dep_nodes.lock().contains(_dep_node);
let seen = seen_dep_nodes.lock().contains(&_dep_node);
assert!(!seen, "{}", _msg());
}
}
Expand Down Expand Up @@ -748,7 +748,7 @@ impl<D: Deps> DepGraphData<D> {
// 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, prev_index, None)
.map(|dep_node_index| (prev_index, dep_node_index))
}
}
Expand All @@ -762,47 +762,40 @@ impl<D: Deps> DepGraphData<D> {
frame: Option<&MarkFrame<'_>>,
) -> Option<()> {
let dep_dep_node_color = self.colors.get(parent_dep_node_index);
let dep_dep_node = &self.previous.index_to_node(parent_dep_node_index);
let dep_dep_node = || self.previous.index_to_node(parent_dep_node_index);

match dep_dep_node_color {
Some(DepNodeColor::Green(_)) => {
// This dependency has been marked as green before, we are
// still fine and can continue with checking the other
// dependencies.
debug!("dependency {dep_dep_node:?} was immediately green");
debug!("dependency {:?} was immediately green", dep_dep_node());
return Some(());
}
Some(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
// with checking any of the other dependencies.
debug!("dependency {dep_dep_node:?} was immediately red");
debug!("dependency {:?} was immediately red", dep_dep_node());
return None;
}
None => {}
}

// 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) {
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, frame);
// We don't know the state of this dependency. Let's try to mark it green recursively.
debug!("state of dependency {:?} is unknown, trying to mark it green", dep_dep_node());
let node_index = self.try_mark_previous_green(qcx, parent_dep_node_index, frame);

if node_index.is_some() {
debug!("managed to MARK dependency {dep_dep_node:?} as green",);
return Some(());
}
if node_index.is_some() {
debug!("managed to MARK dependency {:?} as green", dep_dep_node());
return Some(());
}

// We failed to mark it green, so we try to force the query.
let dep_dep_node = dep_dep_node();
debug!("trying to force dependency {dep_dep_node:?}");
if !qcx.dep_context().try_force_from_dep_node(*dep_dep_node, frame) {
if !qcx.dep_context().try_force_from_dep_node(dep_dep_node, frame) {
// The DepNode could not be forced.
debug!("dependency {dep_dep_node:?} could not be forced");
return None;
Expand Down Expand Up @@ -846,23 +839,18 @@ impl<D: Deps> DepGraphData<D> {
&self,
qcx: Qcx,
prev_dep_node_index: SerializedDepNodeIndex,
dep_node: &DepNode,
frame: Option<&MarkFrame<'_>>,
) -> Option<DepNodeIndex> {
let frame = MarkFrame { index: prev_dep_node_index, parent: frame };
let dep_node = || self.previous.index_to_node(prev_dep_node_index);

#[cfg(not(parallel_compiler))]
self.assert_nonexistent_node(dep_node, || {
format!("trying to mark existing {dep_node:?} as green")
self.assert_nonexistent_node(dep_node(), || {
format!("trying to mark existing {:?} as green", dep_node())
});
#[cfg(not(parallel_compiler))]
debug_assert!(self.colors.get(prev_dep_node_index).is_none());

// We never try to mark eval_always nodes as green
debug_assert!(!qcx.dep_context().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 {
Expand All @@ -889,8 +877,8 @@ impl<D: Deps> DepGraphData<D> {
#[cfg(not(parallel_compiler))]
debug_assert!(
self.colors.get(prev_dep_node_index).is_none(),
"DepGraph::try_mark_previous_green() - Duplicate DepNodeColor \
insertion for {dep_node:?}"
"DepGraph::try_mark_previous_green() - Duplicate DepNodeColor insertion for {:?}",
dep_node()
);

if side_effects.maybe_any() {
Expand All @@ -903,7 +891,7 @@ impl<D: Deps> DepGraphData<D> {
// Multiple threads can all write the same color here
self.colors.insert(prev_dep_node_index, DepNodeColor::Green(dep_node_index));

debug!("successfully marked {dep_node:?} as green");
debug!("successfully marked {:?} as green", dep_node());
Some(dep_node_index)
}

Expand Down Expand Up @@ -950,7 +938,7 @@ impl<D: Deps> DepGraph<D> {

pub fn assert_nonexistent_node<S: std::fmt::Display>(
&self,
dep_node: &DepNode,
dep_node: DepNode,
msg: impl FnOnce() -> S,
) {
if cfg!(debug_assertions)
Expand Down

0 comments on commit 381c7a3

Please sign in to comment.