rustc_thread_pool: Make `CoreLatch::set` use `SeqCst` instead of `AcqRel` by cuviper · Pull Request #155609 · rust-lang/rust

cuviper · 2026-04-21T17:52:42Z

Every other modification of this variable uses SeqCst, which is justified in the sleep README. This particular choice of AcqRel was not discussed during its addition in rayon-rs/rayon#746, nor rayon-rs/rfcs#5, so I suspect was simply an oversight from earlier development. We probably do want this to participate in the same sequential consistency.

The only other ordering difference is CoreLatch::probe's load with Acquire, which should be fine because this doesn't need consistency with the sleep counters.

See also rayon-rs/rayon#1297. As I commented there, I think in practice this would be quite rare to cause any problems, but it could be a source of non-deterministic bugs on targets with weak memory ordering.

…Rel` Every other modification of this variable uses `SeqCst`, which is justified in the sleep README. This particular choice of `AcqRel` was not discussed during its addition in rayon-rs/rayon#746, nor rayon-rs/rfcs#5, so I suspect was simply an oversight from earlier development. We probably do want this to participate in the same sequential consistency. The only other ordering difference is `CoreLatch::probe`'s load with `Acquire`, which should be fine because this doesn't need consistency with the sleep counters. See also rayon-rs/rayon#1297. As I commented there, I think in practice this would be quite rare to cause any problems, but it *could* be a source of non-deterministic bugs on targets with weak memory ordering.

rustbot · 2026-04-21T17:52:48Z

r? @jieyouxu

rustbot has assigned @jieyouxu.
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

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: compiler
compiler expanded to 72 candidates
Random selection from 18 candidates

rustbot · 2026-04-21T17:52:49Z

⚠️ Warning ⚠️

There are issue links (such as #123) in the commit messages of the following commits.
Please move them to the PR description, to avoid spamming the issues with references to the commit, and so this bot can automatically canonicalize them to avoid issues with subtree.
- 29ccf67

jieyouxu

Make sense to me, thanks.

@bors r+ rollup=never (just in case)

View changes since this review

rust-bors · 2026-04-22T01:53:44Z

📌 Commit 29ccf67 has been approved by jieyouxu

It is now in the queue for this repository.

rustc_thread_pool: Make `CoreLatch::set` use `SeqCst` instead of `AcqRel` Every other modification of this variable uses `SeqCst`, which is justified in the sleep README. This particular choice of `AcqRel` was not discussed during its addition in rayon-rs/rayon#746, nor rayon-rs/rfcs#5, so I suspect was simply an oversight from earlier development. We probably do want this to participate in the same sequential consistency. The only other ordering difference is `CoreLatch::probe`'s load with `Acquire`, which should be fine because this doesn't need consistency with the sleep counters. See also rayon-rs/rayon#1297. As I commented there, I think in practice this would be quite rare to cause any problems, but it *could* be a source of non-deterministic bugs on targets with weak memory ordering.

bjorn3 · 2026-04-22T15:24:10Z

CI has been running for over 5h30min.

rust-bors · 2026-04-22T15:46:57Z

💥 Test timed out after 21600s

cuviper · 2026-04-22T16:17:09Z

The timeout was aarch64-apple -- which could conceivably be affected as an arch with weaker memory ordering, but this change should only be more strict. So I hope that was just spurious CI, but let's try first...

@bors try jobs=aarch64-apple

rustc_thread_pool: Make `CoreLatch::set` use `SeqCst` instead of `AcqRel` try-job: aarch64-apple

rust-bors · 2026-04-22T19:22:13Z

☀️ Try build successful (CI)
Build commit: 9dab398 (9dab398030c4363ac24019a1862d452856ef80ab, parent: cf1817bc6ecd2d14ca492247c804bad31948dd56)

JonathanBrouwer · 2026-04-22T19:33:52Z

@bors retry

jieyouxu · 2026-04-23T01:49:17Z

The timeout was aarch64-apple

The aarch64-apple job sometimes randomly not progressing is known, never quite figured out why

rust-bors · 2026-04-23T08:37:50Z

☀️ Test successful - CI
Approved by: jieyouxu
Duration: 3h 7m 30s
Pushing 92c7010 to main...

github-actions · 2026-04-23T08:40:38Z

What is this?

This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing 5095b44 (parent) -> 92c7010 (this PR)

Test differences

Show 4 test diffs

4 doctest diffs were found. These are ignored, as they are noisy.

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard 92c7010294007f9bb819f0ddd9d9e9b5ccd5714a --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

i686-gnu-nopt-2: 1h 41m -> 2h 17m (+35.9%)
dist-aarch64-apple: 1h 46m -> 2h 18m (+30.2%)
aarch64-apple: 3h 42m -> 2h 46m (-25.0%)
i686-gnu-nopt-1: 2h 22m -> 1h 51m (-21.8%)
x86_64-mingw-2: 2h 13m -> 2h 40m (+20.8%)
dist-apple-various: 1h 48m -> 1h 31m (-15.5%)
x86_64-gnu-gcc: 1h 2m -> 1h 11m (+14.5%)
tidy: 2m 31s -> 2m 50s (+12.9%)
pr-check-2: 39m 24s -> 43m 52s (+11.3%)
x86_64-gnu-miri: 1h 28m -> 1h 37m (+9.8%)

How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

rust-timer · 2026-04-23T09:19:42Z

Finished benchmarking commit (92c7010): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This perf run didn't have relevant results for this metric.

Max RSS (memory usage)

Results (secondary -4.2%)

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)	-	-	0
Improvements ✅ (secondary)	-4.2%	[-5.2%, -2.7%]	3
All ❌✅ (primary)	-	-	0

Cycles

This perf run didn't have relevant results for this metric.

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 494.354s -> 500.795s (1.30%)
Artifact size: 394.34 MiB -> 394.31 MiB (-0.01%)

rustbot assigned jieyouxu Apr 21, 2026

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 21, 2026

jieyouxu approved these changes Apr 22, 2026

View reviewed changes

rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 22, 2026

This comment has been minimized.

Sign in to view

rust-bors Bot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 22, 2026

This comment has been minimized.

Sign in to view

rust-bors Bot pushed a commit that referenced this pull request Apr 22, 2026

Auto merge of #155609 - cuviper:thread_pool-seq-cst, r=<try>

9dab398

rustc_thread_pool: Make `CoreLatch::set` use `SeqCst` instead of `AcqRel` try-job: aarch64-apple

rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 22, 2026

This comment has been minimized.

Sign in to view

rust-bors Bot added merged-by-bors This PR was explicitly merged by bors. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 23, 2026

rust-bors Bot merged commit 92c7010 into rust-lang:main Apr 23, 2026
13 checks passed

rustbot added this to the 1.97.0 milestone Apr 23, 2026

cuviper deleted the thread_pool-seq-cst branch June 1, 2026 15:54

Uh oh!

Conversation

cuviper commented Apr 21, 2026

Uh oh!

rustbot commented Apr 21, 2026

Uh oh!

rustbot commented Apr 21, 2026

Uh oh!

jieyouxu left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rust-bors Bot commented Apr 22, 2026

Uh oh!

This comment has been minimized.

bjorn3 commented Apr 22, 2026

Uh oh!

rust-bors Bot commented Apr 22, 2026

Uh oh!

cuviper commented Apr 22, 2026

Uh oh!

This comment has been minimized.

rust-bors Bot commented Apr 22, 2026

Uh oh!

JonathanBrouwer commented Apr 22, 2026

Uh oh!

jieyouxu commented Apr 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

rust-bors Bot commented Apr 23, 2026

Uh oh!

Uh oh!

github-actions Bot commented Apr 23, 2026

Test differences

Job duration changes

Uh oh!

rust-timer commented Apr 23, 2026

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jieyouxu left a comment •

edited by rustbot

Loading

jieyouxu commented Apr 23, 2026 •

edited

Loading