[Kobzol]

View all comments

Part of https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Enabling.20the.20parallel.20frontend.20by.20default.20on.20nightly/with/624597688.

UI tests will still use one frontend thread by default.

CC @petrochenkov

[petrochenkov]

CI fails due to UI tests, they should be compiled with --jobs-frontend=1 because they require reproducible diagnostics.

[Kobzol]

Wasn't there some flag that reordered the diagnostics, or made compiletest to compare them in an order agnostic way? If at all possible, I think that it would be better if we actually ran (at least most) UI tests under the parallel frontend.

[petrochenkov]

I think that it would be better if we actually ran (at least most) UI tests under the parallel frontend.

We do run them, on the test-x86_64-gnu-parallel-frontend job, in special mode.

Wasn't there some flag that reordered the diagnostics, or made compiletest to compare them in an order agnostic way?

Yeah, but that's not enough for some tests, and those tests are disabled with //@ ignore-parallel-frontend on that job (but run in the default mode).
Output blessing is also not supported in the parallel mode, but we want to keep it in the UI tests by default.

[Kobzol]

Can't we solve blessing by forcing 1 frontend thread when --bless is passed?

[petrochenkov]

I'd rather keep the things as is, it's just good to have precise snapshot testing running on something reproducible.
Besides ui tests, --jobs-frontend=1 will need to be used on mir dump tests, and maybe some other test suites.

[Kobzol]

I don't want to block this PR on that, but I find it a bit concerning that we aren't testing a bunch of nightly stuff on our CI. We already don't test the next trait solver, and now we also wouldn't be testing the parallel frontend, even though both would be enabled on nightly.

I'm happy to do the compiletest/bootstrap work to enable running the parallel frontend everywhere. Do you want that to be the default eventually (and perhaps have a separate one-off job that runs things sequentially, like we have the parallel job today)?

[petrochenkov]

Do you want that to be the default eventually

For ui tests (.stderr snapshot testing) - no, I don't expect diagnostics to become reproducible anytime soon, if at all.
Technically we could make -Zui-testing reset the frontend threads to 1 (or otherwise slow down the compiler to serialize the diagnostics) instead of passing --jobs-frontend=1, but that doesn't seem to make much difference.

[petrochenkov]

If there are enough CI resources we could also run rustc twice on every ui test - once in the default mode (if it's parallel) and once in single-threaded mode for precise .stderr comparisons.
Most of //~ ERROR annotations in .rs files should match in parallel mode too, with exception of some tests involving query cycles.

and perhaps have a separate one-off job that runs things sequentially

So this ^^^ would mean limiting the precise .stderr comparison testing to some x86_64-linux-gnu.
I'm not sure, maybe it won't be that terrible, eventually, but it will skip some parts of the test suite.

[Kobzol]

Maybe I misunderstood how the current tooling works. I thought that:

• There is some flag or logic that can compare the emitted diagnostics in an order-independent way. If that exists, then I think that we don't need precise comparisons? As long as the order is the only thing that changes, we can bless with 1 thread and then compare with multiple threads. Or do you consider the actual precise diagnostics ordering to be load-bearing across the whole test suite, rather than e.g. in a subset of tests which we would test with 1 thread (or both with 1 thread and with the parallel frontend)?
• If there are tests that are truly broken under the parallel frontend, we can just mark them as such, and always run them with 1 thread (rather than skipping them entirely when the parallel frontend is enabled by default).

[petrochenkov]

There is some flag or logic that can compare the emitted diagnostics in an order-independent way.

This is correct.

If that exists, then I think that we don't need precise comparisons?

I think that we do still need them, at least while the single-threaded compiler is the default.
Losing them is bad in the same sense as e.g. anonymizing line numbers in .stderr files (which we do), and similarly to the line numbers we can sacrifice them if there are strong reasons (like diff sizes for line numbers). So they are not exactly load bearing, but good to have, and should be kept if we can afford it.

[petrochenkov]

There is some flag or logic that can compare the emitted diagnostics in an order-independent way.

Although that infra currently compares the output on per-line basis, not on per-diagnostic, so technically the diagnostics can be interspersed and messed up entirely, and the check will still pass.

[Kobzol]

Interesting. The obtain-borrowck.rs ui-fulldeps test now fails, likely because it override borrow check and stores some data in a thread local. But the interesting thing is that it only seems to happen when -Zthreads=1 is used. I presume that rustc behaves slightly differently when -Zthreads is unset, vs when it is set to 1?

[Kobzol]

@rustbot author

@bors try jobs=* nolimit

[Kobzol]

failures:
---- jobserver::runner_inherits_jobserver stdout ----
running /checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/cargo install --path .
running make run -j2
thread 'jobserver::runner_inherits_jobserver' (129035) panicked at src/tools/cargo/tests/testsuite/jobserver.rs:198:10:
test failed running make run -j2
error: process exited with code 2 (expected 0)
--- stdout
make[1]: Entering directory '/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/tmp/cit/src/tools/cargo/testsuite/jobserver/runner_inherits_jobserver/foo'
/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/cargo run
make[1]: Leaving directory '/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/tmp/cit/src/tools/cargo/testsuite/jobserver/runner_inherits_jobserver/foo'
--- stderr
Compiling cargo-jobserver-check v0.0.1 (/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/tmp/cit/src/tools/cargo/testsuite/jobserver/runner_inherits_jobserver/foo)
Finished dev profile [unoptimized + debuginfo] target(s) in 0.22s
Running target/debug/cargo-jobserver-check
thread 'main' (129279) panicked at src/main.rs:26:44:
called Result::unwrap() on an Err value: Os { code: 11, kind: WouldBlock, message: "Resource temporarily unavailable" }
note: run with RUST_BACKTRACE=1 environment variable to display a backtrace
make[1]: *** [Makefile:2: run] Error 101
make[1]: INTERNAL: Exiting with 1 jobserver tokens available; should be 2!
---- jobserver::runner_inherits_jobserver stdout end ----

Possibly looks like a genuine failure with using multiple frontend threads by default? 🤔 I'll rerun the test later.

[rust-bors]

💔 Test for 84b547f failed: CI. Failed job:

• try - test-x86_64-gnu-aux (web logs, enhanced plaintext logs)

[Kobzol]

@bors try jobs=test-x86_64-gnu-aux

[rust-bors]

💔 Test for 544118a failed: CI

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[Kobzol]

@bors try jobs=test-x86_64-gnu-aux

[rust-bors]

☀️ Try build successful (CI)
Build commit: 097c236 (097c2363024207697127883046b49a4509288de9)
Base parent: 9358362 (93583629f7429a9ab9fb807408e66eed81f3e5b1)

[Kobzol]

Hmm, ok. I couldn't reproduce it locally either. Let's try everything then:

@bors try jobs=* nolimit

[rust-bors]

⌛ Trying commit da0f0ea with merge cce8517…

To cancel the try build, run the command @bors try cancel.

Workflow: https://github.com/rust-lang/rust/actions/runs/42405638612

[rust-log-analyzer]

The job test-x86_64-gnu-aux failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)

[petrochenkov]

Blocked on #165506 at least.