use precompiled rustc for non-dist builders #122709

onur-ozkan · 2024-03-18T19:05:11Z

Makes non-dist builders to use precompiled CI rustc by default if they are available for the target triple.

As we are going to make rust.download-rustc=if-unchanged default option with #119899, we need to make sure if-unchanged logic never breaks and works as expected.

As an addition, this will significantly improve the build times on CI when there's no change on the compiler.

blocker for #119899

try-job: x86_64-gnu-nopt
try-job: aarch64-apple

bors · 2024-03-18T20:23:48Z

☔ The latest upstream changes (presumably #122690) made this pull request unmergeable. Please resolve the merge conflicts.

onur-ozkan · 2024-04-04T15:08:14Z

@bors try

…default, r=<try> prefer precompiled rustc for x86_64-gnu *-to be filled-* blocker for rust-lang#119899 r? ghost

bors · 2024-04-04T15:09:26Z

⌛ Trying commit 7099a13 with merge 9ec5616...

bors · 2024-04-04T16:59:40Z

☀️ Try build successful - checks-actions
Build commit: 9ec5616 (9ec5616fdf20bd5f5381043eccffbe26506a80ff)

onur-ozkan · 2024-04-04T17:51:06Z

@bors try

bors · 2024-04-04T17:56:19Z

⌛ Trying commit 731c8eb with merge 3a8cf09...

…default, r=<try> prefer precompiled rustc for x86_64-gnu *-to be filled-* blocker for rust-lang#119899 r? ghost

onur-ozkan · 2024-10-08T15:35:23Z

@bors try

…default, r=<try> use precompiled rustc for non-dist builders Makes non-dist builders to use precompiled CI rustc by default if they are available for the target triple. As we are going to make `rust.download-rustc=if-unchanged` default option with rust-lang#119899, we need to make sure `if-unchanged` logic never breaks and works as expected. As an addition, this will significantly improve the build times on CI when there's no change on the compiler. blocker for rust-lang#119899 try-job: x86_64-gnu-nopt try-job: aarch64-apple

bors · 2024-10-08T15:36:52Z

⌛ Trying commit 4082f9f with merge 47cde5e...

bors · 2024-10-08T16:37:08Z

☀️ Try build successful - checks-actions
Build commit: 47cde5e (47cde5ec82414d45ce945e5de5e7d3937b9744b1)

onur-ozkan · 2024-10-08T16:42:39Z

Nothing changed (in this PR) since the last merge attempt.

@bors r=Mark-Simulacrum

bors · 2024-10-08T16:42:43Z

📌 Commit 4082f9f has been approved by Mark-Simulacrum

It is now in the queue for this repository.

bors · 2024-10-08T18:12:25Z

⌛ Testing commit 4082f9f with merge a49aefc...

bors · 2024-10-08T20:44:42Z

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

rust-timer · 2024-10-08T22:03:44Z

Finished benchmarking commit (a49aefc): 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)

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

Cycles

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

Binary size

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

Bootstrap: 775.795s -> 775.772s (-0.00%)
Artifact size: 329.57 MiB -> 329.50 MiB (-0.02%)

lqd · 2024-10-08T22:29:56Z

It may be unrelated, but it seems the test added here is failing on some of PR CI eg #131418 (comment) ?

src/ci/run.sh

jieyouxu · 2024-10-09T03:15:54Z

This test seems to have caused PR CI failure in #131384 (comment)

onur-ozkan · 2024-10-09T04:33:21Z

It may be unrelated, but it seems the test added here is failing on some of PR CI eg #131418 (comment) ?

It seems to be related. I will look into it today.

onur-ozkan · 2024-10-09T04:50:57Z

#131434 should fix that issue.

…onur-ozkan fix `ci_rustc_if_unchanged_logic` test Kind a typo from rust-lang#122709, which makes `ci_rustc_if_unchanged_logic` test to fail in any PR that has a change in "library" tree (e.g., rust-lang#131418 (comment)). This fixes that. r? ghost

RalfJung · 2024-10-13T14:11:15Z

src/bootstrap/src/core/builder/tests.rs

+    builder.run_step_descriptions(&Builder::get_step_descriptions(config.cmd.kind()), &[]);
+
+    let compiler_path = build.src.join("compiler");
+    let library_path = build.src.join("compiler");


This looks like a copy-paste mistake?

EDIT: Ah, that got fixed in #131434.

RalfJung · 2024-10-13T14:12:44Z

As an addition, this will significantly improve the build times on CI when there's no change on the compiler.

It will save CI time if there is no change in the compiler or the library. Which is to say, hardly ever.

Is there some issue with further discussion or so? I am a bit surprised about this change since the trade-off does not seem worth it for me.

RalfJung · 2024-10-13T14:22:18Z

There's some discussion hidden away above by github, but sadly I could not find a summary and the PR description is apparently not up-to-date. Anyway, let's move this to an issue: #131658.

Kobzol · 2024-10-13T14:26:31Z

It will save CI time if there is no change in the compiler or the library. Which is to say, hardly ever.

Here are some brief stats of commits for the last year:

Total merge/rollup commits: 2825
Merge/rollup PRs that had no change in library nor compiler: 627 (22%)

Total % of commits that contain a change in the given directory
compiler: 1843 (65.24%)
tests: 1676 (59.33%)
library: 992 (35.12%)
src/bootstrap: 424 (15.01%)
src/librustdoc: 353 (12.50%)
src/ci: 190 (6.73%)
<rest>: 1404 (49.70%)

I'm too worried about the fragility of this code, but I'd also like to see how much CI time can this save us.

rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Mar 18, 2024