Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only use the new DepNode hashmap for anonymous nodes. #109050

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Do not fetch the DepNode to mark nodes green.
  • Loading branch information
cjgillot committed Dec 2, 2024
commit 694b77342191f59d568f7446849184bede45b1d2
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 @@ -1088,7 +1088,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
50 changes: 19 additions & 31 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> {

cjgillot marked this conversation as resolved.
Show resolved Hide resolved
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,15 +839,10 @@ 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 };

// 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 dep_node = || self.previous.index_to_node(prev_dep_node_index);

let prev_deps = self.previous.edge_targets_from(prev_dep_node_index);

Expand Down Expand Up @@ -889,7 +877,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 @@ -936,7 +924,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