CI - Runner caching improvements - #5798
Open
bfops wants to merge 73 commits into
Open
Conversation
…o bfops/test-runner
…o bfops/test-runner # Conflicts: # .cargo/config.toml # .github/workflows/ci.yml # tools/ci/commands/module-latest-deps/src/main.rs # tools/ci/commands/run-spacetime/src/main.rs # tools/ci/commands/test/src/main.rs # tools/ci/commands/typescript-test/src/main.rs # tools/ci/common/src/lib.rs # tools/gen-bindings/src/main.rs
bfops
commented
Aug 27, 2026
bfops
commented
Aug 27, 2026
bfops
commented
Aug 27, 2026
bfops
commented
Aug 27, 2026
Signed-off-by: Zeke Foppa <196249+bfops@users.noreply.github.com>
bfops
commented
Aug 27, 2026
Signed-off-by: Zeke Foppa <196249+bfops@users.noreply.github.com>
bfops
marked this pull request as ready for review
August 27, 2026 20:28
joshua-spacetime
self-requested a review
August 27, 2026 20:33
joshua-spacetime
requested changes
Aug 28, 2026
joshua-spacetime
left a comment
joshua-spacetime
left a comment
Collaborator
There was a problem hiding this comment.
The numbers are fantastic, but I still have concerns over git-restore-mtime.
Comment on lines
+119
to
+130
| # cargo has a behavior that's a bit unfortunate for us here. It does not know the mtime of the files that | ||
| # were used to build its artifacts; it just knows the mtime of the artifact. | ||
| # So, if you have a sequence of events like: | ||
| # 1. cache source commit X happens at time X | ||
| # 2. PR commit Y happens at time Y > X | ||
| # 3. cache is warmed from commit X at time Z > Y | ||
| # | ||
| # cargo then sees "artifact built at time Z is more recent than time Y, so it must be up to date". | ||
| # | ||
| # So here we restore committed mtimes to reuse warm artifacts, then touch files that | ||
| # differ from the warmed commit so they will look more recent than the artifact, | ||
| # and cargo will properly rebuild anything depending on those files. |
Collaborator
There was a problem hiding this comment.
I really find git-restore-mtime to be problematic for several reasons, but to name a few:
- The rust compiler doesn't use it (it uses sccache).
- Cargo itself doesn't appear to recommend it (it recommends sccache).
git-restore-mtimeopenly documents what seem to be quite relevant short-comings.- The timestamps that are being compared come from different clocks.
It just seems really brittle, and so I'd prefer not to rely on it if we don't have to. I'm happy to revisit it though if we see that sccache still leaves a lot of savings on the table.
Comment on lines
-604
to
-605
| - *set-native-cache-keys | ||
| - &restore-jemalloc |
Collaborator
There was a problem hiding this comment.
It might be better to remove these native lib caches separately after updating the image. Especially if you're not planning to update the linux and windows images at the same time?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of Changes
This only changes things for the linux runners.
cargo ciinstead of calling the ci subcommand binaries directlymasterRemaining cache work
ringand its dependents rebuild because nested Cargo processes inherit the parenttools/cipackage'sCARGO_MANIFEST_DIR; that package-specific environment leaks into the nested builds and changes their fingerprints. This accounts for 13.70s and 10.96s Clippy builds, a 10.31s CLI-doc build, and an 88s plus two roughly 40s Test Suite builds.reused 0because the image does not contain the pnpm store used by these installs. The jobs download 2,801 packages each, except Smoketests, which downloads 429.wasm-bindgen-cliare absent from the image. The job downloads 31 MB of Node plus 355 MB of wasm binaries and spends 15.36s compilingwasm-bindgen-clifrom source.modules/sdk-test-procedurecompiling for 6.66s, Blackholio's server for 7.82s, the latest-compatible module for 7.48s, and the TypeScript helper tools for 1.76s. The latest-compatible module also intentionally resolves newer compatible crate versions rather than the repository lockfile's exact versions.templatesdirectory itself. Checkout changes that directory mtime, whilegit restore-mtimerestores tracked file mtimes rather than directory metadata, so Cargo rebuilds the CLI even when template contents are unchanged. This is the sole Rust rebuild in Upload build artifacts (30.56s).Next steps
API and ABI breaking changes
None
Expected complexity level and risk
2
Testing
Just the existing CI. The performance table is the testing.