Add the parallel front-end test suite by ywxt · Pull Request #143953 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation20 Commits4 Checks11 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
jieyouxu is currently at their maximum review capacity.
They may take a while to respond.
Area: The compiletest test runner
Area: Issues & PRs about the rust-lang/rust repository itself
Area: The testsuite used to check the correctness of rustc
Status: Awaiting review from the assignee but also interested parties.
Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Relevant to the compiler team, which will review and decide on the PR/issue.
labels
Some changes occurred in src/tools/opt-dist
cc @Kobzol
Some changes occurred in src/tools/compiletest
cc @jieyouxu
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. RUSTC_PARALLEL_TEST.
| 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); |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should add a timeout here to prevent CI from getting stuck in a deadlock.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or does the current test set already have it? @jieyouxu
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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:
- Do we disable offending test
- Or in extreme cases, disable the whole test suite?
ywxt and others added 4 commits
Co-authored-by: SparrowLii liyuan179@huawei.com
This change is large enough that it needs a major change proposal before starting work.
Status: Blocked on something else such as an RFC or other implementation work.
and removed S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
labels
to give a little bit more of background:
while fuzzing for bugs in the parallel compiler, sometimes you hit weird
- crashes
- deadlocks
- diagnostic changes
- differences in codegen
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 N tests x M threads at the same time.
Usually we can run N tests at a time, but with threads, does that turn into 8*200 = up to 1600 threads at a time when running the suite?
Right now I can mostly rely on x.py test -j 1 not completely hogging my system but how would -j 1 work when we try to run 100 parallel ui tests with 20-200 threads?
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 agree this that we need a proper solution. Although every test can have a specified number of iteration, it isn't available on it.
Add the parallel front-end test suit Add the parallel front-end test suite
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.
This was referenced
Jul 26, 2025
This was referenced
Jul 31, 2025
#142949 is still reproducible for me on 1.91.0-nightly (ec7c02612 2025-08-05).
bors added a commit that referenced this pull request
Fix parallel rustc not being reproducible due to unstable sorts of items
Currently, A tuple (DefId, SymbolName) is used to determine the order of items in the final binary. However DefId is expected as non-deterministic, which leads to some not reproducible issues under parallel compilation. (See #140425 (comment))
Theoretically, we don't need the sorting because the order of these items is already deterministic.
However, codegen tests reply on the same order of items between in binary and source.
So here we added a new option codegen-source-order to indicate whether sorting based on the order in source. For codegen tests, items are sorted according to the order in the source code, whereas in the normal path, no sorting is performed.
Specially, for codegen tests, in preparation for parallel compilation potentially being enabled by default in the future, we use Span replacing DefId to make the order deterministic.
This PR is purposed to fix #140425, but seemly works on #140413 too.
This behavior hasn't added into any test until we have a test suit for the parallel frontend. (See #143953)
Related discussion: Zulip #144576
Update #113349
r? @oli-obk
cc @lqd @cramertj @matthiaskrgr @Zoxc @SparrowLii @bjorn3 @cjgillot @joshtriplett
bors added a commit that referenced this pull request
Fix parallel rustc not being reproducible due to unstable sorts of items
Currently, A tuple (DefId, SymbolName) is used to determine the order of items in the final binary. However DefId is expected as non-deterministic, which leads to some not reproducible issues under parallel compilation. (See #140425 (comment))
Theoretically, we don't need the sorting because the order of these items is already deterministic.
However, codegen tests reply on the same order of items between in binary and source.
So here we added a new option codegen-source-order to indicate whether sorting based on the order in source. For codegen tests, items are sorted according to the order in the source code, whereas in the normal path, no sorting is performed.
Specially, for codegen tests, in preparation for parallel compilation potentially being enabled by default in the future, we use Span replacing DefId to make the order deterministic.
This PR is purposed to fix #140425, but seemly works on #140413 too.
This behavior hasn't added into any test until we have a test suit for the parallel frontend. (See #143953)
Related discussion: Zulip #144576
Update #113349
r? @oli-obk
cc @lqd @cramertj @matthiaskrgr @Zoxc @SparrowLii @bjorn3 @cjgillot @joshtriplett
bors added a commit that referenced this pull request
Fix parallel rustc not being reproducible due to unstable sorts of items
Currently, A tuple (DefId, SymbolName) is used to determine the order of items in the final binary. However DefId is expected as non-deterministic, which leads to some not reproducible issues under parallel compilation. (See #140425 (comment))
Theoretically, we don't need the sorting because the order of these items is already deterministic.
However, codegen tests reply on the same order of items between in binary and source.
So here we added a new option codegen-source-order to indicate whether sorting based on the order in source. For codegen tests, items are sorted according to the order in the source code, whereas in the normal path, no sorting is performed.
Specially, for codegen tests, in preparation for parallel compilation potentially being enabled by default in the future, we use Span replacing DefId to make the order deterministic.
This PR is purposed to fix #140425, but seemly works on #140413 too.
This behavior hasn't added into any test until we have a test suit for the parallel frontend. (See #143953)
Related discussion: Zulip #144576
Update #113349
r? @oli-obk
cc @lqd @cramertj @matthiaskrgr @Zoxc @SparrowLii @bjorn3 @cjgillot @joshtriplett
bors added a commit that referenced this pull request
Fix parallel rustc not being reproducible due to unstable sorts of items
Currently, A tuple (DefId, SymbolName) is used to determine the order of items in the final binary. However DefId is expected as non-deterministic, which leads to some not reproducible issues under parallel compilation. (See #140425 (comment))
Theoretically, we don't need the sorting because the order of these items is already deterministic.
However, codegen tests reply on the same order of items between in binary and source.
So here we added a new option codegen-source-order to indicate whether sorting based on the order in source. For codegen tests, items are sorted according to the order in the source code, whereas in the normal path, no sorting is performed.
Specially, for codegen tests, in preparation for parallel compilation potentially being enabled by default in the future, we use Span replacing DefId to make the order deterministic.
This PR is purposed to fix #140425, but seemly works on #140413 too.
This behavior hasn't added into any test until we have a test suit for the parallel frontend. (See #143953)
Related discussion: Zulip #144576
Update #113349
r? @oli-obk
cc @lqd @cramertj @matthiaskrgr @Zoxc @SparrowLii @bjorn3 @cjgillot @joshtriplett
github-actions bot pushed a commit to rust-lang/rustc-dev-guide that referenced this pull request
Fix parallel rustc not being reproducible due to unstable sorts of items
Currently, A tuple (DefId, SymbolName) is used to determine the order of items in the final binary. However DefId is expected as non-deterministic, which leads to some not reproducible issues under parallel compilation. (See rust-lang/rust#140425 (comment))
Theoretically, we don't need the sorting because the order of these items is already deterministic.
However, codegen tests reply on the same order of items between in binary and source.
So here we added a new option codegen-source-order to indicate whether sorting based on the order in source. For codegen tests, items are sorted according to the order in the source code, whereas in the normal path, no sorting is performed.
Specially, for codegen tests, in preparation for parallel compilation potentially being enabled by default in the future, we use Span replacing DefId to make the order deterministic.
This PR is purposed to fix rust-lang/rust#140425, but seemly works on rust-lang/rust#140413 too.
This behavior hasn't added into any test until we have a test suit for the parallel frontend. (See rust-lang/rust#143953)
Related discussion: Zulip rust-lang/rust#144576
Update rust-lang/rust#113349
r? @oli-obk
cc @lqd @cramertj @matthiaskrgr @Zoxc @SparrowLii @bjorn3 @cjgillot @joshtriplett
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.
I say this because in majority of cases bug is still noticeably present in code, but is no longer triggered due to inconsistent reproducibility of parallel-compiler bugs. Whenever in the future bug becomes active once again, it will bring down CI, and would require outside interference (for example #142949). I would like to suggest on such occasion to disable failing test to swiftly unblock CI and open a separate issue to document bug. But if you dare so, you have to explicitly state a good reason to reenable a particular test, like if panicking/buggy code have been heavily modified.
I understand that this is not the most infallible approach, but we all have to remember there will never be bug-free rustc. We should first of all document a bug and then have time to work on a solution relatively to that bug's impact.
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.
I say this because in majority of cases bug is still noticeably present in code, but is no longer triggered due to inconsistent reproducibility of parallel-compiler bugs. Whenever in the future bug becomes active once again, it will bring down CI, and would require outside interference (for example #142949). I would like to suggest on such occasion to disable failing test to swiftly unblock CI and open a separate issue to document bug. But if you dare so, you have to explicitly state a good reason to reenable a particular test, like if panicking/buggy code have been heavily modified.
I understand that this is not the most infallible approach, but we all have to remember there will never be bug-free rustc. We should first of all document a bug and then have time to work on a solution relatively to that bug's impact.
I agree this. I'll cleanup it when we have a detailed design of this test suite. (rust-lang/compiler-team#906)
Status: PR has a compiler MCP and is waiting for the compiler MCP to complete.
Status: This is awaiting some action (such as code changes or more information) from the author.
and removed S-blocked
Status: Blocked on something else such as an RFC or other implementation work.
Status: PR has a compiler MCP and is waiting for the compiler MCP to complete.
Status: This is awaiting some action (such as code changes or more information) from the author.
This change is large enough that it needs a major change proposal before starting work.
labels
Labels
Area: The compiletest test runner
Area: Issues & PRs about the rust-lang/rust repository itself
Area: parallel compiler
Area: The testsuite used to check the correctness of rustc
Status: PR has a compiler MCP and is waiting for the compiler MCP to complete.
Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Relevant to the compiler team, which will review and decide on the PR/issue.