Add the parallel front-end test suite#143953
Conversation
|
|
rustbot
added
A-compiletest
This comment has been minimized.
This comment has been minimized.
|
do we have some kind of kill switch to quickly disable the entire parallel suite in case of spurious-looking one-in-50 test failures happening in ci? |
|
@matthiaskrgr That sounds reasonable. I think controlling it through a env variable is a feasible method， e.g. |
| for _ in 0..iteration_count { | ||
| let proc_res = self.compile_test(WillExecute::No, emit_metadata); | ||
| // Ensure there is no ICE during parallel complication. | ||
| self.check_no_compiler_crash(&proc_res, false); |
There was a problem hiding this comment.
I think we should add a timeout here to prevent CI from getting stuck in a deadlock.
There was a problem hiding this comment.
Or does the current test set already have it? @jieyouxu
There was a problem hiding this comment.
The current compiletest executor is basically libtest's. That is, tests are ran under test threads. There's a test running for too long warning message, but no hard kill-after-timeout, which IIUC is hard to implement with this setup because it'd require some kind of collaborative scheme.
For existing tests we've massaged them to not take that long, or disabled them if e.g. the compile time truly that long pointing to an issue tracking it.
|
I'm not at PC until much later today, but @SparrowLii I think it's worth opening an MCP for adding this test suite, especially because we may observe its failures in CI in unrelated PRs. I'm still in favor of adding this test suite, but I would like to establish some consensus on what to do about test failures:
|
Co-authored-by: SparrowLii <liyuan179@huawei.com>
jieyouxu
added
needs-mcp
|
to give a little bit more of background: while fuzzing for bugs in the parallel compiler, sometimes you hit weird
that only happen one in 10/ 20 / 30 / 40 times. so even IF you have a test for something, you may actually regress it in some cases, but you only notice 30 runs later when a completely different area of the compiler is touched which will be extremely confusing. I don't have a solution for this, but I'm not sure if "run everything 50 times just to be sure" is a scalable solution. I'm also a bit worried about the implications of running Right now I can mostly rely on |
I agree this that we need a proper solution. Although every test can have a specified number of iteration, it isn't available on it. |
ywxt
changed the title
|
Opening a MCP for this is definitely a good first step. I think that for testing the parallel frontend specifically, something akin to a 24/7 running fuzzing suite on a separate repository might be a better fit than having something that runs in r-l/r's auto CI jobs. |
I agree this. I'll cleanup it when we have a detailed design of this test suite. (rust-lang/compiler-team#906) |
|
#142064 was reproduced during the crater parallel frontend test run (#146237): I've tried to build LinusThorsell/rustensor using this branch's compiler and with That's another example that any parallel test-suite can only detect bugs, but not rule them out. |
Dylan-DPC
added
S-waiting-on-MCP
|
I suggest this change to be also added: diff --git a/src/tools/compiletest/src/executor.rs b/src/tools/compiletest/src/executor.rs
index df64f12784f..740be4d97d3 100644
--- a/src/tools/compiletest/src/executor.rs
+++ b/src/tools/compiletest/src/executor.rs
@@ -8,7 +8,7 @@
use std::sync::{Arc, Mutex, mpsc};
use std::{env, hint, io, mem, panic, thread};
-use crate::common::{Config, TestPaths};
+use crate::common::{Config, TestMode, TestPaths};
mod deadline;
mod json;
@@ -19,7 +19,8 @@ pub(crate) fn run_tests(config: &Config, tests: Vec<CollectedTest>) -> bool {
// Iterator yielding tests that haven't been started yet.
let mut fresh_tests = (0..).map(TestId).zip(&filtered);
- let concurrency = get_concurrency();
+ // Run each parallel test sequentialy to maximize tests' thread utilization
+ let concurrency = if config.mode != TestMode::Parallel { get_concurrency() } else { 1 };
assert!(concurrency > 0);
let concurrent_capacity = concurrency.min(filtered.len());
|
|
After that and a rebase I think this PR will be good to merge. |
petrochenkov
added
the
S-waiting-on-review
|
Prior to merging this, let's try a few try jobs as a smoke-check: |
| "ignore-x86_64-pc-windows-gnu", | ||
| "ignore-x86_64-unknown-linux-gnu", | ||
| "incremental", | ||
| "iteration-count", |
There was a problem hiding this comment.
Suggestion: please document this directive in rustc-dev-guide
| only_hosts: true, | ||
| }); | ||
|
|
||
| test!(Parallel { path: "tests/parallel", mode: "parallel", suite: "parallel", default: true }); |
There was a problem hiding this comment.
Suggestion: and please also document in rustc-dev-guide (https://rustc-dev-guide.rust-lang.org/tests/compiletest.html#test-suites):
- The purpose of this test suite, in particular what this test suite intends to catch. I'm not sure what the current intention is, i.e. is it still Add the parallel front-end test suite #143953 (comment)?
I think parallel rustc test suite should only include code from issues, which do not reproduce at all. For example if issue can be reproduced on some previous commit but not on the current master branch - unless it has been explicitly fixed by someone, should stay as an active issue to be solved.
- What to do about failures (for instance in unrelated PRs), probably aggressively disable + file an issue?
rustbot
added
S-waiting-on-author
|
Reminder, once the PR becomes ready for a review, use |
|
@ywxt could you add all the tests from here to |
Okay, I'll likely start this in the next week. |
too many open deadlock issues. I'm not sure if they really haven fixed. |
That doesn't prevent adding the tests. |
|
But tests in ui\parallel-rustc use -Zthreads by default. if we remove the option, it doesn't make sense to run parallel tests as the single thread.
Get Outlook for Android<https://aka.ms/AAb9ysg>
…________________________________
From: ***@***.*** ***@***.***> on behalf of Vadim Petrochenkov ***@***.***>
Sent: Tuesday, March 24, 2026 6:45:58 PM
To: rust-lang/rust ***@***.***>
Cc: Rua ***@***.***>; Mention ***@***.***>
Subject: Re: [rust-lang/rust] Add the parallel front-end test suite (PR #143953)
[https://avatars.githubusercontent.com/u/5751617?s=20&v=4]petrochenkov left a comment (rust-lang/rust#143953)<#143953 (comment)>
@ywxt<https://github.com/ywxt> could you add all the tests from here to tests\ui\parallel-rustc as single-threaded tests? (In a separate PR.) Then they will also be run as parallel tests when --parallel-frontend-threads is passed. (Once ui tests with --parallel-frontend-threads run on CI we could probably close this PR.)
too many open deadlock issues. I'm not sure if they really haven fixed.
That doesn't prevent adding the tests.
If some of the added tests fails on CI, then we'll know for sure that the issue is not fixed.
—
Reply to this email directly, view it on GitHub<#143953?email_source=notifications&email_token=AGFKPG6NDLU6BKRAYOZOUQD4SJRONA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIMJRG4YTQNBUG442M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-4117184479>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AGFKPG3LIFO4VEX5XMJ46O34SJRONAVCNFSM6AAAAACWOCLXLGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DCMJXGE4DINBXHE>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
Adding test with -Zthreads may block the regular CI
…________________________________
From: 落花 流水 ***@***.***>
Sent: Tuesday, March 24, 2026 6:55:55 PM
To: rust-lang/rust ***@***.***>
Subject: Re: [rust-lang/rust] Add the parallel front-end test suite (PR #143953)
But tests in ui\parallel-rustc use -Zthreads by default. if we remove the option, it doesn't make sense to run parallel tests as the single thread.
Get Outlook for Android<https://aka.ms/AAb9ysg>
________________________________
From: ***@***.*** ***@***.***> on behalf of Vadim Petrochenkov ***@***.***>
Sent: Tuesday, March 24, 2026 6:45:58 PM
To: rust-lang/rust ***@***.***>
Cc: Rua ***@***.***>; Mention ***@***.***>
Subject: Re: [rust-lang/rust] Add the parallel front-end test suite (PR #143953)
[https://avatars.githubusercontent.com/u/5751617?s=20&v=4]petrochenkov left a comment (rust-lang/rust#143953)<#143953 (comment)>
@ywxt<https://github.com/ywxt> could you add all the tests from here to tests\ui\parallel-rustc as single-threaded tests? (In a separate PR.) Then they will also be run as parallel tests when --parallel-frontend-threads is passed. (Once ui tests with --parallel-frontend-threads run on CI we could probably close this PR.)
too many open deadlock issues. I'm not sure if they really haven fixed.
That doesn't prevent adding the tests.
If some of the added tests fails on CI, then we'll know for sure that the issue is not fixed.
—
Reply to this email directly, view it on GitHub<#143953?email_source=notifications&email_token=AGFKPG6NDLU6BKRAYOZOUQD4SJRONA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIMJRG4YTQNBUG442M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-4117184479>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AGFKPG3LIFO4VEX5XMJ46O34SJRONAVCNFSM6AAAAACWOCLXLGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DCMJXGE4DINBXHE>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Not by default (there's no such logic in compiletest), the tests add the
If they are added to the tests suite, they will run with multiple threads when |
|
Thanks for your explanation.
I'm gonna finish them tomorrow.
…________________________________
From: ***@***.*** ***@***.***> on behalf of Vadim Petrochenkov ***@***.***>
Sent: Tuesday, March 24, 2026 7:04:41 PM
To: rust-lang/rust ***@***.***>
Cc: Rua ***@***.***>; Mention ***@***.***>
Subject: Re: [rust-lang/rust] Add the parallel front-end test suite (PR #143953)
[https://avatars.githubusercontent.com/u/5751617?s=20&v=4]petrochenkov left a comment (rust-lang/rust#143953)<#143953 (comment)>
But tests in ui\parallel-rustc use -Zthreads by default.
Not by default (there's no such logic in compiletest), the tests add the -Zthreads options individually.
The newly added tests can be added without the -Zthreads options.
it doesn't make sense to run parallel tests as the single thread.
If they are added to the tests suite, they will run with multiple threads when --parallel-frontend-threads is passed.
(But running them in single-threaded mode is also slightly useful, to make sure that they don't fail for some other reason.)
―
Reply to this email directly, view it on GitHub<#143953?email_source=notifications&email_token=AGFKPG5BGGTCJGVLDD7EBY34SJTUTA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIMJRG4ZTANBVGMY2M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-4117304531>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AGFKPG7CMDQEWDAOC5QPANL4SJTUTAVCNFSM6AAAAACWOCLXLGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DCMJXGMYDINJTGE>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
12 participants
View all comments
MCP: rust-lang/compiler-team#906
update #118698
update #113349
This PR is the follow-up to #132051 .
Now tests for the parallel front-end have been a dedicated test suite(named "parallel").
The related issues below have been irreproducible now.
value must be in cache after waiting#111528deadlock detected without any query#120759deadlock detected as we're unable to find a query cycle to break#129911internal error: entered unreachable code#142064collecting exported symbols for crate/collect_and_partition_mono_items#118205assertion failed: found_cycle#115223no index for a field#120760variances_ofreturns&[ty::Variance]' #124423only 'variances_of' returns '&[ty::Variance]'#127971Could not send CguMessage to main thread#142949None#120786attempted to read from stolen value: rustc_middle::thir::Thir:-Zthir-unsafeck=yes#111520Unexpected type for constructor#120601This PR cleans up deadlock and ICE issues, although some reported problems related to reproducible builds still remain.
r? @jieyouxu
cc @SparrowLii @lqd