Parallel frontend compiler support (perf backend only)

[heinwol]

Supersedes #2421, but only backend (profiler/bencher/database). As said in parent pr, parallel frontend support is implemented as additional dimension with corresponding row added to the database

[Kobzol]

Thanks for the PR! Let's revert the last commit; the CI failure shows why we need to have a custom environment variable, so that it will also work with a stable Rust compiler.

[heinwol]

I'm not sure I quite like a separate environment variable for that; don't we have a better mechanism? Also, maybe a collector run shouldn't silently revert to single-threaded scenario in an unsupported version, which it currently does, for my understanding? However, it seems like it's a common behavior in collector/src/lib.rs::async fn bench_published_artifact:

// ...
    let profiles = if collector::version_supports_doc(&toolchain.id) {
        vec![Profile::Check, Profile::Debug, Profile::Doc, Profile::Opt]
    } else {
        vec![Profile::Check, Profile::Debug, Profile::Opt]
    };
    let scenarios = if collector::version_supports_incremental(&toolchain.id) {
        Scenario::all()
    } else {
        Scenario::all_non_incr()
    };
    let frontend_threads_counts = if collector::version_supports_parallel_frontend(&toolchain.id) {
        FrontendThreads::default_opts()
    } else {
        vec![FrontendThreads(1)]
    };
// ...

So it might be i'm looking too hard into it...

[Kobzol]

bench_published_artifact is only used for benchmarking stable versions of the compiler, which is ~never done locally, and we need to keep compatibility with Rust going all the way back to 1.26.0, so I wouldn't worry about that.

[Kobzol]

Thanks for separating the PR! Did a first review pass. Adding a new benchmark axis is always cumbersome, as it requires changing many places, and this one is also tricky, because it doesn't have a closed set of values, but is just a number. But in general it looks good, just

Please revert all the site changes, the integration with the frontend and also the job queuing system should be done in a follow-up PR. For now everything on production will just assume frontend count 1.

[heinwol]

Thanks for the fast yet sizable review! I'm sorry for calling out for you so early; i'm not sure this pr is quite review-ready. I'll try to resolve what you commented for now and then dive more into how this works! As until now all i did is brainless-ish renaming, lol

[Kobzol]

To be fair, adding a new benchmark axis mostly involves a lot of brainless-ish renaming :) I think that the PR is mostly good to go, though it will require a couple of follow-up PRs to:

1. Add integration in the website backend and frontend
2. Modify the job queue to be able to distinguish that different jobs can have different frontend thread counts

[heinwol]

The dumb part is more or less done, now i'll be dealing with threads argument propagation and fallback legacy paths

EDIT: oh, and the par_n one

[Kobzol]

also, a question: should we include frontend_threads into glossary?

Yes, I think it would be nice to mention it there!

[Kobzol]

Thanks! Did another review pass and tried it locally. I think that the only thing required to fix before we can merge this is the backwards compatibility with the self-profile URLs.

[heinwol]

First, the spectrum of tasks we discussed here is presumably done. Probably some fixes are required still.

Second, rebased. Took a brief look at changes between previous and rebased versions. Seems like there are no semantic conflicts

[heinwol]

CI (or, in general, runtime) failures are due to "master"/"main" mismatch in some parts of code (collector/lib.rs and several files in database) due to #2495. Maybe I could make a pr with fixes?
cc @Kobzol

[Kobzol]

The CI failures are caused by this PR, not by the one you linked, we have merged a bunch of other PRs in the meantime. It is caused by calling version_supports_parallel_frontend even for unreleased compiler artifacts, which won't work.

It is caused by this commit: 887c27b I don't understand what it is supposed to do, I think we should remove that commit and keep the previous behavior.

[heinwol]

Ah, I get it, we can't get information about whether parallel frontend is supported for an artifact with commit sha. Then we have no way to pass information about -Zthreads support to rustc-fake. You were making this point before, i didn't understand it then.

At the same time, we can still run correctly on any published artifact. i.e. when we run benches from bench_published_artifact. Instead of always passing just Vec<FrontendThreads> to run_benchmarks we can also add a flag for whether we have -Zthreads support or not, so that run_benchmarks can delegate this information properly to rustc-fake. This way we won't be calling version_supports_parallel_frontend outside of bench_published_artifact.

So the only time we'll fail is when we bench a legacy non-published artifact, which i suppose is a rare situation.

I'll push a commit for that and then ask you to decide whether it's good or we should just stop thinking about such corner cases and move on

[Kobzol]

Don't worry about the legacy toolchains, we don't have to deal with them for this PR ;)

[heinwol]

Well, it turned out this was harder than i expected. And the change poisoned the entire crate with FrontendThreadsOrLegacy/FrontendThreadsCountsOrLegacy... But at least I had some fun!

[Kobzol]

This is just needlessly complicating the benchmark parameters. Just keep the Vec of counts as it was before. We really don't need to support the released benchmarks here, we have to run them manually maybe once every 5 years, and it will always require a lot of other manual one-time changes to the codebase anyway, so there's no point in complicating the collector for it.

[Kobzol]

Thank you, it looks great! Can you please squash the commits? Then I'll merge it.

[heinwol]

Sure! Into a single one?

[Kobzol]

Yeah, I think that would be fine.

[heinwol]

ahem... Give me a minute to sort this out ;)

[Kobzol]

Thank you very much! Let's see what happens on production :)

[heinwol]

fingers crossed ;)

in the meantime i'll start integrating database query changes and site (presumably in one go? Or maybe not)

[Kobzol]

I think that website will require changes to the compile-time benchmark queries, so that can be done in one go.