ci: Conditionally build parallel compiler on try

[alexcrichton]

This commit configures Travis/AppVeyor to conditionally compile parallel
compilers on @bors: try. This is an experiment currently to see how
this plays out, but the intention is that if the commit message contains
the term "parallel-compiler" then when @bors: try is issued it will
perform differently than the try branch does today, building three
compilers: Linux, macOS, and Windows. We currently have no try
builders for macOS or Windows due to typical capacity issues, so it's
intended that this is only very sparingly used from time to time when
necessary.

cc #48685, tracking issue for parallel compilation

[rust-highfive]

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

[alexcrichton]

@bors: try

Let's see what happens...

[alexcrichton]

@bors: try

[bors]

⌛ Trying commit 4382c25 with merge 9743253127ed10bcfb5b046647599d211f0b0a0b...

[alexcrichton]

@bors: try

Ok I'm gonna run a test real quick and see what happens if we just enable compiling a parallel compiler but it defaults to 1 thread instead of num_cpus. I'm curious what sort of perf slowdown we'll see

[bors]

⌛ Trying commit c4fa1e0 with merge cb23a46f60b0233f366367dd11b5fdc241bce328...

[Zoxc]

There's another parallel compiler bug introduced in #58929, but it should still work with a single thread.

[bors]

☀️ Try build successful - checks-travis
Build commit: cb23a46f60b0233f366367dd11b5fdc241bce328

[alexcrichton]

Let's see if we can get some ballpark numbers...

@rust-timer build cb23a46f60b0233f366367dd11b5fdc241bce328

[rust-timer]

Success: Queued cb23a46f60b0233f366367dd11b5fdc241bce328 with parent eab3eb3, comparison URL.

[rust-timer]

Finished benchmarking try commit cb23a46f60b0233f366367dd11b5fdc241bce328

[alexcrichton]

Ok so that looks like it's a blanket 2-3% slowdown across the board if we enable parallel compilation and peg rustc to just one thread. That's... frankly amazing!

I think given those kind of numbers it might actually be feasible to just turn this on by default for all releases. We'd get a small slowdown which we'd quickly be able to recover many times over by bumping up the default number of threads.

@Zoxc I'm curious, do you think that rustc might be ready for this transition? Concretely I'd imagine that we'd turn on the parallel_compiler#[cfg] by default for CI (like done in this PR), but we'd still default to -Zthreads=1 (or something like that). We'd afterwards likely quickly remove the #[cfg(parallel_compiler)] annotations, only using the parallel versions.

Later once ready we could default threads to greater than 1, but until then we could allow testing on nightly via the unstable -Zthreads option for local investigations.

[alexcrichton]

I've also started some chat on zulip about this!

[mati865]

Wall-time doesn't look convincing, maybe there is a lot of blocking?

[alexcrichton]

Ah yes sorry my mistake! The interesting metric here indeed is wall time, not instructions executed. That comparison URL is located here and is indeed unfortunately less inspiring, showing a blanket ~10% slowdown, which I think is a bit too serious to land just now.

@Zoxc in addition to the question above about whether the compiler is ready for this, are you aware of low hanging fruit for optimizing?

[alexcrichton]

Hm well so it's also probably worthwhile noting the magnitude of changes here, it looks like all 10%+ changes are in tiny crates, regressing from 5 to 6 seconds for example. Larger crates like servo ones do regress but still on the order of a second in a 15 second compilation already, so I don't think it's necessarily damning evidence

[mati865]

packed-simd already takes very long time to build and it regresses here by ~10%. That's quite worrying and could be worth checking.

[Zoxc]

Turning on parallel_compiler with a single thread should be fine from a correctness standpoint.

I'm not aware of any low hanging fruit, there might be some though as I've mostly focused on making the code run in parallel. Maybe @nnethercote wants to help here and look for some excessive locking?

I'm working on getting rid of Arcs in the query system, but I suspect the overhead comes mostly from locking, and maybe some from Rayon. I can measure this though.

[nnethercote]

One distinct downside of (true, multi-thread) parallelization is that it makes benchmarking much harder. Wall time is the true metric of interest, but it's so noisy that e.g. 1% improvements/regression are impossible to spot. Currently we can use instruction counts as a proxy for wall time, because instruction counts have much less variation, and they're a pretty good proxy. But true parallelization will majorly reduce the usefulness of instruction counts as a metric.

It's a hard problem, I'm not sure what to do about it. It's one reason why I'm biased toward trying to improve coarse-grained parallelization (e.g. pipelining) rather than fine-grained parallelization.

[alexcrichton]

@nnethercote that's a good point! I suspect though we can solve it by always passing -Zthreads=1 to benchmarks on perf.r-l.o?

[alexcrichton]

@Zoxc I don't mind also doing some profiling to look into this, the packed_simd case pointed out by @mati865 is a good one to look into, so I'll try to investigate that and post some results here tomorrow

[nnethercote]

@alexcrichton: Let's assume we reach the point where we are shipping a parallel compiler.
Passing -Zthreads=1 would mean that instruction counts is still a good proxy for wall time, which is good... but it would also mean that we are measuring something that we aren't shipping, which is bad :(

[Zoxc]

I opened #59649, #59647, #59644 and #59641 to find out what the cause of the regressions are on a high level.

[Zoxc]

Seems like the locks are to blame for most of the regressions.

[alexcrichton]

Ok I think that we've identified an actionable way to go (thanks @Zoxc) so I don't think we're going to want to take this approach where @bors: try builds parallel compilers and we recommend rustup-toolchain-install-master for testing.

To that end I'm going to close this and I've opened up #59667 to track further work.