Enable ThinLTO for rustc on x86_64-apple-darwin

[lqd]

Local measurements seemed to show an improvement on a couple benchmarks, so I'd like to test real CI builds, and see if the builder doesn't timeout with the expected slight increase in build times.

Let's start with x64 rustc ThinLTO, and then figure out the file structure to configure LLVM ThinLTO. Maybe we'll then try aarch64 builds since that also looked good locally.

[lqd]

@bors try

[bors]

⌛ Trying commit 39bf1507ecf95e4f5b066de65cf9929a7650e1c3 with merge b4d23f6f6fd8602735600f2aa54e884005c277e8...

[bors]

☀️ Try build successful - checks-actions
Build commit: b4d23f6f6fd8602735600f2aa54e884005c277e8 (b4d23f6f6fd8602735600f2aa54e884005c277e8)

[lqd]

The job completed in 2h57 which is maybe too close to a timeout ? Looking at the first couple pages of successful CI jobs, the range was from 2h16 to 2h45.

[lqd]

(I've removed the CI hacks)

The only x86 darwin system I have access to is pretty noisy (old iMac), making the majority of the rustc-perf benchmarks to be avoided.

So I've tried the longer benchmarks, on full clean builds (not only the leaf crate like the perf collector does) it seems that on this machine the ThinLTOed rustc b4d23f6f6fd8602735600f2aa54e884005c277e8 is generally faster than the baseline 0da281b6068a7d889ae89a9bd8991284cc9b7535 by 1-2%. (Sometimes more, e.g. diesel-1.4.8, image-0.24.1 or keccak on check builds).

Two things would be interesting to know:

• whether CI can afford this change on the macOS builder ?
• whether such changes to rustc/llvm on the x86 or aarch64 builders, should wait for us to add mac support to rustc-perf, or if a benchmark on a less noisy environment would be enough, assuming the results were positive there ? I've wanted to ask @thomcc's expertise and guidance on how we should support macs in rustc-perf.

For the two questions above, I'll r? @Mark-Simulacrum to have your thoughts if that's OK.

[Mark-Simulacrum]

I'm not too worried about claiming this is a win for Macs if it was a win for Linux; that seems like a pretty reasonable claim in my eyes. Especially if we can have some validation on noisier machines.

That said I think making this switch will probably bring too high a cost at this time, given that these runners dominate our runtimes. It's a little bit annoying in the sense that making the change for just beta or similar might drive down our runtimes on nightly and give more room for this kind of optimization... I think if we don't see headroom through other ways (e.g., different CI runners, optimizations to the things running in them, etc) then it might be worth trying that. It feels relatively safe as a beta-exclusive (i.e. not on nightly or stable), since beta sees very little usage anyway.

[lqd]

That makes sense, thanks a bunch.

I'll mark this blocked for now rather than close it outright (but feel free to do so if you prefer): so that we can revisit in the future and decide which of the two paths to take then.

[lqd]

With the new osx builders from #105212, I believe we should now have the capacity to enable this change: @rustbot label -S-blocked

What do you think @Mark-Simulacrum, can we land this ?
@rustbot ready

[Mark-Simulacrum]

Let's merge this, mostly so that we can get some data gathered on what the hit is. We'll reconsider if it is a significant increase that begins to drive the slowest builds again. For the record:

So looks like we're around 1 hour per apple dist builder right now.

@bors r+

[bors]

📌 Commit 3a085f7 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

[bors]

⌛ Testing commit 3a085f7 with merge 32d70cc436f06f18c510b2e964db6f7c21b5bbf3...

[bors]

💔 Test failed - checks-actions

[ehuss]

@bors retry

Failed to update toolstate
(has been fixed)

[bors]

⌛ Testing commit 3a085f7 with merge bdb07a8...

[bors]

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing bdb07a8 to master...

[rust-timer]

Finished benchmarking commit (bdb07a8): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	3.0%	[3.0%, 3.0%]	1
Improvements ✅ (primary)	-0.9%	[-0.9%, -0.9%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.9%	[-0.9%, -0.9%]	1

Cycles

This benchmark run did not return any relevant results for this metric.