Allow green-red query coloring race #150156

zetanumbers · 2025-12-19T12:56:36Z

Fixes #150018
Fixes #142778
Fixes #141540

This PR allows parallel front-end's green-red node coloring processes in the query dep graph to race. Recall that whenever one thread tries to mark query node green and then fails to reconstruct a subquery's key to force it, parent query is tried to be forced or executed too. The subquery could be marked green by the thread during execution the parent query because key is now present. Other thread during this process is able to mark green parent query itself. And so after some time the first thread finishes the execution and finds in its place a green node. This is bad for no_hash queries since those are conservatively presumed to be red but actually queries must be deterministic anyway and so no_hash nodes could be colored green if its deps are green too after its execution finishes. As such we can consider green color to be "stronger" that the red color and thus former would replace the latter.

TODO: update rustc-dev-guide query docs and rustc comments on no_hash modifier

I have left some refactor-ish changes I did during my dirty debugging. Let me know if you would want those to be redone or removed.

Steps to reproduce

Quote from #150018, more details there:

Do a clean build x build library --stage 1 at 52fda2e, then git checkout to ba45f0b and without cleaning try rebuilding it the same way again. And don't forget to set RUSTFLAGS_BOOTSTRAP to -Zthreads=n where n > 1.

… deps are green too

rustbot · 2025-12-19T12:56:41Z

r? @petrochenkov

rustbot has assigned @petrochenkov.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

petrochenkov · 2025-12-19T13:06:04Z

cc @Zoxc @cjgillot @nnethercote

zetanumbers · 2025-12-19T13:21:09Z

My first idea was to add a special "locked" color, but I wasn't sure what was even happening in there because coloring races currently are considered to be normal anyway.

matthiaskrgr · 2025-12-19T13:51:13Z

@bors try @rust-timer queue

Allow green-red query coloring race

zetanumbers · 2025-12-19T14:58:58Z

I ran rustc incremental builds as described in reproduction steps section for several hours with -Zthreads, debug=true and -Zincremental_verify_ich and it's looking good.

rust-bors · 2025-12-19T16:07:32Z

☀️ Try build successful (CI)
Build commit: bb5a19e (bb5a19e1ef3ed803b9a2acd98a3b223334df5063, parent: 0f0d850e9146f53b16fb1c6a56ed2f821962ff94)

rust-timer · 2025-12-19T16:48:58Z

Finished benchmarking commit (bb5a19e): comparison URL.

Overall result: ❌ regressions - please read the text below

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	2.3%	[0.3%, 6.6%]	117
Regressions ❌ (secondary)	1.7%	[0.2%, 4.9%]	75
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	2.3%	[0.3%, 6.6%]	117

Max RSS (memory usage)

Results (primary -3.5%, secondary -1.5%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-3.5%	[-3.5%, -3.5%]	1
Improvements ✅ (secondary)	-1.5%	[-2.2%, -0.8%]	2
All ❌✅ (primary)	-3.5%	[-3.5%, -3.5%]	1

Cycles

Results (primary 3.0%, secondary 3.4%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	3.0%	[2.9%, 3.1%]	2
Regressions ❌ (secondary)	3.4%	[2.3%, 4.9%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	3.0%	[2.9%, 3.1%]	2

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 479.055s -> 481.828s (0.58%)
Artifact size: 390.50 MiB -> 390.50 MiB (0.00%)

petrochenkov · 2025-12-19T16:42:34Z

compiler/rustc_query_system/src/dep_graph/graph.rs

 pub(super) enum DepNodeColor {
    Green(DepNodeIndex),
-    Red,
+    Red(() /* DepNodeIndex */),


Suggested change

Red(() /* DepNodeIndex */),

Red,

Probably left from some debugging.

petrochenkov · 2025-12-19T16:43:06Z

compiler/rustc_query_system/src/dep_graph/graph.rs

 }

+impl DepNodeColor {
+    pub(super) fn as_green(self) -> Option<DepNodeIndex> {


Suggested change

pub(super) fn as_green(self) -> Option<DepNodeIndex> {

pub(super) fn expect_green(self) -> DepNodeIndex {

The result is always unwrapped.

petrochenkov · 2025-12-19T16:44:48Z

compiler/rustc_query_system/src/dep_graph/graph.rs


                    return dep_node_index;
                }
+                debug_assert_eq!(DepNodeColor::Unknown, color);


Suggested change

debug_assert_eq!(DepNodeColor::Unknown, color);

debug_assert_eq!(color, DepNodeColor::Unknown);

(Sorry.)

petrochenkov · 2025-12-19T16:45:24Z

compiler/rustc_query_system/src/dep_graph/graph.rs

-                // 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) {


Need a comment explaining what's going on here.

petrochenkov · 2025-12-19T16:46:54Z

compiler/rustc_query_system/src/dep_graph/graph.rs

    values: IndexVec<SerializedDepNodeIndex, AtomicU32>,
 }

 // All values below `COMPRESSED_RED` are green.


Outdated comment.

petrochenkov · 2025-12-19T16:48:56Z

compiler/rustc_query_system/src/dep_graph/graph.rs

+
+                Err(DepNodeIndex::from_u32(green))
+            }
+            Err(green) => Err(DepNodeIndex::from_u32(green)),


Need comments explaining when all of the cases above may happen, and what we are asserting.

petrochenkov · 2025-12-19T16:49:40Z

compiler/rustc_query_system/src/dep_graph/graph.rs

+            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)),


Need comments explaining when all of the cases above may happen, and what we are asserting.

petrochenkov · 2025-12-19T16:50:34Z

compiler/rustc_query_system/src/dep_graph/graph.rs

    // 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)


Suggested change

if let DepNodeColor::Green(c) = data.colors.current(prev_index)

if data.colors.current(prev_index) == DepNodeColor::Green(dep_node_index) {

Or remove several derived impls on DepNodeColor as unnecessary.

petrochenkov · 2025-12-19T17:01:23Z

compiler/rustc_query_system/src/dep_graph/serialized.rs


-        if is_green {
-            // Use `try_mark_green` to avoid racing when `send_promoted` is called concurrently
-            // on the same index.


Need to keep a comment similar to this one for both green and red cases.

petrochenkov · 2025-12-19T17:04:02Z

compiler/rustc_query_system/src/dep_graph/graph.rs

+        assert_eq!(index & COMPRESSED_RED_BIT, 0);
+        let green = index;
+
+        match self.values[prev_index].compare_exchange(COMPRESSED_UNKNOWN, green, Release, Acquire)


Perhaps just use SeqCst everywhere until the parallel frontend is stabilized?

Or add comments explaining the orderings, although I'm not sure who exactly they will be supposed to convince.

petrochenkov · 2025-12-19T17:12:12Z

As usual, it would be nice to split the work into a few parts (especially given the perf regressions):

Preparatory changes and refactorings (passing the frame and feed around, the named iterator change, the COMPRESSED_RED_BIT change, and few others)
The assert hardening changes
The substantial changes modifying the logic in try_mark and alloc_and_color_node

But it may be better to wait for a high level design review from Zoxc or cjgillot before doing that.

@rustbot author

rustbot · 2025-12-19T17:12:16Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

Allow green-red query coloring race and mark no_hash queries green if…

0b6923b

… deps are green too

rustbot assigned petrochenkov Dec 19, 2025

This was referenced Dec 19, 2025

assertion failed: trying to encode a dep node twice #150018

Open

ICE: rustc_query_system: dep_graph: assertion failed (dep node index out of range) #142778

Open

A dep graph node (mir_drops_elaborated_and_const_checked) does not have an unique index #141540

Open

This comment has been minimized.

Sign in to view

rust-bors bot added a commit that referenced this pull request Dec 19, 2025

Auto merge of #150156 - zetanumbers:allow_red_green_race, r=<try>

bb5a19e

Allow green-red query coloring race

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Dec 19, 2025

This comment has been minimized.

Sign in to view

rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Dec 19, 2025

petrochenkov reviewed Dec 19, 2025

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 19, 2025

	pub(super) fn as_green(self) -> Option<DepNodeIndex> {
	pub(super) fn expect_green(self) -> DepNodeIndex {

	debug_assert_eq!(DepNodeColor::Unknown, color);
	debug_assert_eq!(color, DepNodeColor::Unknown);

	if let DepNodeColor::Green(c) = data.colors.current(prev_index)
	if data.colors.current(prev_index) == DepNodeColor::Green(dep_node_index) {

Uh oh!

Allow green-red query coloring race #150156

Are you sure you want to change the base?

Allow green-red query coloring race #150156

Uh oh!

Conversation

zetanumbers commented Dec 19, 2025 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Steps to reproduce

Uh oh!

rustbot commented Dec 19, 2025

Uh oh!

petrochenkov commented Dec 19, 2025

Uh oh!

zetanumbers commented Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matthiaskrgr commented Dec 19, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

zetanumbers commented Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rust-bors bot commented Dec 19, 2025

Uh oh!

This comment has been minimized.

rust-timer commented Dec 19, 2025

Overall result: ❌ regressions - please read the text below

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petrochenkov commented Dec 19, 2025

Uh oh!

rustbot commented Dec 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

zetanumbers commented Dec 19, 2025 •

edited by rustbot

Loading

zetanumbers commented Dec 19, 2025 •

edited

Loading

zetanumbers commented Dec 19, 2025 •

edited

Loading