Download, rather than sccache-cache, LLVM in CI #99967
Conversation
ba64590
to
00cd775
Compare
|
☔ The latest upstream changes (presumably #99529) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Noticed that this changed a bunch of different dockerfiles - I remember @pietroalbini suggesting a while ago that we could use named profiles (like the ones in x.py setup) for CI, maybe that would help reduce the burden here? I'm willing to put some time into that. |
|
Note that this is just changing the CI config (we don't use docker on mac or windows). Ultimately I think in this case if this works I'll stick it in src/ci/run.sh gated by isMacos or something like that, so I'm not too worried about this being gated on profiles of any kind. |
a6c02e5
to
0dac895
Compare
This comment has been minimized.
This comment has been minimized.
0dac895
to
bfd544a
Compare
|
r? @jyn514 This shouldn't be r+'d as is but I'll hold off on actually dropping the do not merge commit until we finish with any changes you suggest, so that we can actually test this change seems to work. |
There was a problem hiding this comment.
This is awesome! Would be very curious to see how much time it saves :)
It also makes me wonder if we can do the same for download-rustc - most PRs will modify compiler/, but we already have if-unchanged and I think @rylev has suggested "library only rollups" that would make it a lot more common to be able to reuse the cache. Future work though, llvm is already a great first step :)
Related: I've been seeing various emails between Joel Marcey and @pietroalbini about how we use a truly enomous amount of S3 credits on static.rlo, I wonder if we could save in costs by redirecting downloads for |
bf83297
to
28e70ad
Compare
|
Presuming CI passes I think this should be good for another review and presuming that looks good, I'll drop the apple-1 commit. |
|
Looks great! r=me with the last commit dropped :) very excited to see how much time this saves |
28e70ad
to
197218b
Compare
|
@bors r+ rollup=never |
|
📌 Commit 197218b10d2d87c902d83623ed05f29f4011b23f has been approved by It is now in the queue for this repository. |
|
@bors r- r=jyn514 |
|
📌 Commit 197218b10d2d87c902d83623ed05f29f4011b23f has been approved by It is now in the queue for this repository. |
|
⌛ Testing commit ec09f02e221cb57d8f35a49cb11c5bacb9c619d0 with merge bc26f7fb5b85c73b6a523859d101320571d72a9b... |
|
💔 Test failed - checks-actions |
This comment has been minimized.
This comment has been minimized.
See comment added for details on the test builder restriction. This is primarily intended for macOS CI, but is likely to be a slight win on other builders too.
ec09f02
to
d7b91c3
Compare
|
@bors r=jyn514 rollup=never |
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (fb80d2b): comparison url. Instruction count
Max RSS (memory usage)Results
CyclesResults
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. @rustbot label: -perf-regression Footnotes |
|
🎉 🎉 |
…k-Simulacrum Fix git safe-directory path for docker images This fixes the path for configuring the git safe.directory setting inside docker images. AFAIK, `~/gitconfig` without a dot is not something that git uses ([ref](https://git-scm.com/docs/git-config)). This was needed in my environment to avoid the ` fatal: detected dubious ownership in repository at '/checkout'` error. For context, this was added in rust-lang#99967.
…k-Simulacrum Fix git safe-directory path for docker images This fixes the path for configuring the git safe.directory setting inside docker images. AFAIK, `~/gitconfig` without a dot is not something that git uses ([ref](https://git-scm.com/docs/git-config)). This was needed in my environment to avoid the ` fatal: detected dubious ownership in repository at '/checkout'` error. For context, this was added in rust-lang#99967.
Rollup merge of rust-lang#127839 - ehuss:safe-directory-docker, r=Mark-Simulacrum Fix git safe-directory path for docker images This fixes the path for configuring the git safe.directory setting inside docker images. AFAIK, `~/gitconfig` without a dot is not something that git uses ([ref](https://git-scm.com/docs/git-config)). This was needed in my environment to avoid the ` fatal: detected dubious ownership in repository at '/checkout'` error. For context, this was added in rust-lang#99967.
My hope/expectation is that we can do better than sccache in CI for cached builds -- currently it looks like on macOS those still take upwards of 10-11 minutes, which is a significant amount of time that we could potentially cut.
This enables this mode for all non-dist builders; this should avoid any problems with the artifacts we distribute, while also providing for faster test builders (since they'll make use of PGO'd LLVM on the platforms we do that on, which is hopefully a nice win). It slightly increases the chance of test builders starting to fail only after a PR is merged (if PGO changes runtime behavior), but that should hopefully never happen, so I think this is worthwhile.
Measurements on the PR for apple-1 don't show any noticeable improvement in CI times, but those can be pretty noisy -- I'm inclined to land this since it should pretty much always be better and we can reconsider if that ever turns out not to be the case.