Make "Assemble stage1 compiler" orders of magnitude faster (take 2) #97268

jyn514 · 2022-05-22T05:22:18Z

This used to take upwards of 5 seconds for me locally. I found that the culprit was copying the downloaded LLVM shared object:

[22:28:03] Install "/home/jnelson/rust-lang/rust/build/x86_64-unknown-linux-gnu/ci-llvm/lib/libLLVM-14-rust-1.62.0-nightly.so" to "/home/jnelson/rust-lang/rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/lib/libLLVM-14-rust-1.62.0-nightly.so"
[22:28:09]   c Sysroot { compiler: Compiler { stage: 1, host: x86_64-unknown-linux-gnu(x86_64-unknown-linux-gnu) } }

It turned out that install() used full copies unconditionally. Change it to try using a hard-link before falling back to copying.

Panic if we generate a symbolic link in a tarball
Change install to use copy internally, like in my previous PR
Change copy to dereference symbolic links, which avoids the previous regression in Make "Assemble stage1 compiler" orders of magnitude faster #96803.

I also took the liberty of fixing x dist llvm-tools to work even if you don't call x build previously.

rust-highfive · 2022-05-22T05:22:21Z

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

src/bootstrap/dist.rs

Mark-Simulacrum · 2022-05-23T13:22:30Z

@bors r+

bors · 2022-05-23T13:22:35Z

📌 Commit 460ad720eacb3c5eb6522ddf2f31826969809e9b has been approved by Mark-Simulacrum

Mark-Simulacrum · 2022-05-23T13:57:53Z

@bors r-

Let's hold off until we confirm the permissions strategy here.

jyn514 · 2022-05-26T15:52:28Z

Ok, I updated the PR to:

Panic if we generate a symbolic link in a tarball
Change install to use copy internally, like in my previous PR
Change copy to dereference symbolic links, which avoids the previous regression.

There is no more special casing file.is_symlink or llvm_from_ci anywhere in dist. In terms of permissions, install will now change the permissions of both the source and destination when using hard links; we control the source files, so I don't think it will be an issue.

@rustbot ready

jyn514 · 2022-05-26T15:56:47Z

Panic if we generate a symbolic link in a tarball

I tested this by cherry-picking b2f0e38ffe210a0099faad9b1594feb6fcf0b2da onto #96803 and making sure that x dist llvm-tools panicked.

Mark-Simulacrum · 2022-05-28T19:31:43Z

@bors try

bors · 2022-05-28T19:31:53Z

⌛ Trying commit fb469766fcc0d11aa86736a677fca4a18bf43be9 with merge 4e819be6a73e5fd42d55a1f5d61c69264ae74cbc...

bors · 2022-05-28T21:22:30Z

💔 Test failed - checks-actions

jyn514 · 2022-05-29T05:26:15Z

This error is strange. src/tools/cargo/tests/testsuite/cargo_add/add-basic.in doesn't exist in tree, but the error doesn't say "file not found". Maybe it's generated somehow? I don't know in which step this is going wrong ...

@epage do you know how add-basic.in is generated?

jyn514 · 2022-05-29T20:25:04Z

Strangely, I can't seem to find a component that rustup-toolchain-install-master can't install - I tried -c rustc-src and -c rustc-nightly-src, which both say file not found, and -c rust-src works fine.

Looks like neither rustup nor rustup-toolchain-install-master are able to install it - they assume the target has x86_64-unknown-linux-gnu, but it doesn't for this source tarball.

$ curl -I https://ci-artifacts.rust-lang.org/rustc-builds/bef2b7cd1c7bcb3393f10d5752fcf9ee3026bce8/rustc-nightly-x86_64-unknown-linux-gnu.tar.xz
HTTP/2 200 
$ curl -I https://ci-artifacts.rust-lang.org/rustc-builds/bef2b7cd1c7bcb3393f10d5752fcf9ee3026bce8/rustc-nightly-src.tar.xz
HTTP/2 200 
$ rustup component add rustc-src --target ''
error: toolchain 'nightly-x86_64-unknown-linux-gnu' does not contain component 'rustc-src' for target ''; did you mean 'rust-src'?

I'm going to go ahead and special-case it.

jyn514 · 2022-06-08T03:10:38Z

@bors try

bors · 2022-06-08T03:10:47Z

⌛ Trying commit 3213672cf8e60bb8440700bfc8e553a81a08de20 with merge bc0fb8974a74c2c48b91bde70bd794ee2998a310...

bors · 2022-06-08T05:01:24Z

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

jyn514 · 2022-06-08T05:05:12Z

Ok cool! This should be ready for review then.

@rustbot ready

src/bootstrap/dist.rs

src/bootstrap/tarball.rs

Mark-Simulacrum · 2022-06-18T14:38:48Z

r=me with comments resolved (let me know if you feel there's a need for another review, but I expect resolution to be pretty non-noteworthy)

This avoids regressions in rustup-toolchain-install-master

This used to take upwards of 5 seconds for me locally. I found that the culprit was copying the downloaded LLVM shared object: ``` [22:28:03] Install "/home/jnelson/rust-lang/rust/build/x86_64-unknown-linux-gnu/ci-llvm/lib/libLLVM-14-rust-1.62.0-nightly.so" to "/home/jnelson/rust-lang/rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/lib/libLLVM-14-rust-1.62.0-nightly.so" [22:28:09] c Sysroot { compiler: Compiler { stage: 1, host: x86_64-unknown-linux-gnu(x86_64-unknown-linux-gnu) } } ``` It turned out that `install()` used full copies unconditionally. Change it to use `copy()` internally, which uses hard links instead when available. Note that this has a change in behavior: Installing a file will also change permissions on the source, not just the destination, if hard links are used. To avoid changing the behavior on symlinks for existing code, I introduce a new function `copy_internal` which only dereferences symlinks when told to do so.

jyn514 · 2022-06-19T20:55:57Z

@bors r=Mark-Simulacrum

bors · 2022-06-19T20:55:58Z

📌 Commit 057eab7 has been approved by Mark-Simulacrum

bors · 2022-06-19T22:22:10Z

⌛ Testing commit 057eab7 with merge 611e7b9...

bors · 2022-06-20T00:40:01Z

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

rust-timer · 2022-06-20T01:58:06Z

Finished benchmarking commit (611e7b9): comparison url.

Instruction count

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

Max RSS (memory usage)

Results

Primary benchmarks: 🎉 relevant improvement found
Secondary benchmarks: 🎉 relevant improvements found

	mean¹	max	count²
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	N/A	N/A	0
Improvements 🎉 (primary)	-3.7%	-3.7%	1
Improvements 🎉 (secondary)	-3.7%	-4.7%	3
All 😿🎉 (primary)	-3.7%	-3.7%	1

Cycles

Results

Primary benchmarks: no relevant changes found
Secondary benchmarks: 😿 relevant regression found

	mean¹	max	count²
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	3.1%	3.1%	1
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	N/A	N/A	0
All 😿🎉 (primary)	N/A	N/A	0

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

the arithmetic mean of the percent change ↩ ↩²
number of relevant changes ↩ ↩²

rust-highfive assigned Mark-Simulacrum May 22, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 22, 2022

Mark-Simulacrum reviewed May 22, 2022

View reviewed changes

src/bootstrap/dist.rs Outdated Show resolved Hide resolved

jyn514 force-pushed the faster-assemble branch from 9d4a495 to c5d6a75 Compare May 22, 2022 21:54

This comment has been minimized.

Sign in to view

jyn514 force-pushed the faster-assemble branch from c5d6a75 to 460ad72 Compare May 22, 2022 22:11

apiraino added the T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. label May 23, 2022

Mark-Simulacrum reviewed May 23, 2022

View reviewed changes

src/bootstrap/dist.rs Outdated Show resolved Hide resolved

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 23, 2022

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 23, 2022

jyn514 force-pushed the faster-assemble branch from 460ad72 to cc9cea4 Compare May 26, 2022 15:50

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 26, 2022

jyn514 force-pushed the faster-assemble branch from cc9cea4 to fb46976 Compare May 26, 2022 16:01

This comment has been minimized.

Sign in to view

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 28, 2022

jyn514 force-pushed the faster-assemble branch from ee3b7e8 to 3213672 Compare May 29, 2022 20:34

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 8, 2022

Mark-Simulacrum reviewed Jun 18, 2022

View reviewed changes

src/bootstrap/dist.rs Outdated Show resolved Hide resolved

Mark-Simulacrum reviewed Jun 18, 2022

View reviewed changes

src/bootstrap/tarball.rs Outdated Show resolved Hide resolved

Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 18, 2022

jyn514 force-pushed the faster-assemble branch 2 times, most recently from 52e52e7 to 3905e37 Compare June 19, 2022 20:51

jyn514 and others added 2 commits June 19, 2022 15:54

Panic if dist generates a symbolic link in a generated tarball

b9eedea

This avoids regressions in rustup-toolchain-install-master

jyn514 force-pushed the faster-assemble branch from 3905e37 to 057eab7 Compare June 19, 2022 20:54

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 19, 2022

bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 20, 2022

bors merged commit 611e7b9 into rust-lang:master Jun 20, 2022

rustbot added this to the 1.63.0 milestone Jun 20, 2022

jyn514 deleted the faster-assemble branch June 20, 2022 00:42

Make "Assemble stage1 compiler" orders of magnitude faster (take 2) #97268

Make "Assemble stage1 compiler" orders of magnitude faster (take 2) #97268

Uh oh!

Conversation

jyn514 commented May 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rust-highfive commented May 22, 2022

Uh oh!

Uh oh!

This comment has been minimized.

Uh oh!

Mark-Simulacrum commented May 23, 2022

Uh oh!

bors commented May 23, 2022

Uh oh!

Mark-Simulacrum commented May 23, 2022

Uh oh!

jyn514 commented May 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jyn514 commented May 26, 2022

Uh oh!

Mark-Simulacrum commented May 28, 2022

Uh oh!

bors commented May 28, 2022

Uh oh!

This comment has been minimized.

bors commented May 28, 2022

Uh oh!

jyn514 commented May 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jyn514 commented May 29, 2022

Uh oh!

jyn514 commented Jun 8, 2022

Uh oh!

bors commented Jun 8, 2022

Uh oh!

bors commented Jun 8, 2022

Uh oh!

jyn514 commented Jun 8, 2022

Uh oh!

Uh oh!

Uh oh!

Mark-Simulacrum commented Jun 18, 2022

Uh oh!

jyn514 commented Jun 19, 2022

Uh oh!

bors commented Jun 19, 2022

Uh oh!

bors commented Jun 19, 2022

Uh oh!

bors commented Jun 20, 2022

Uh oh!

rust-timer commented Jun 20, 2022

Instruction count

Max RSS (memory usage)

Cycles

Footnotes

Uh oh!

Uh oh!

jyn514 commented May 22, 2022 •

edited

Loading

jyn514 commented May 26, 2022 •

edited

Loading

jyn514 commented May 29, 2022 •

edited

Loading