perf(ci): drop the redundant target cache from the Test job - #338
Merged
Conversation
The Test job restored and saved a 2.7 GB `target/` directory through actions/cache on every run: 95s to pull it down at ~47 MB/s and 80s to push it back, so 175s of the job's 19 minutes was cache transfer. That predates kache. kache is now the compiler cache and reported a 96.4% hit rate on the same run, served from a local 80 GiB store by reflink — so the work the target cache exists to avoid is already avoided, far more cheaply than a 2.7 GB round trip over the network. Also raise the job's timeout from 30 to 45 minutes. It is not a fix for anything, it is a guard: this job hit exactly 30 minutes on the first run of soma#330 with every test passing and roughly three minutes of work left, so it was already brushing the limit. If dropping the cache turns out to cost more than it saved, the job should get slower rather than go red. Measurable either way: the Test job was 19m02s with the cache. MCP Smoke caches `target/` the same way and is deliberately left alone, so it stays a control for this change.
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
1 participant
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.
Follow-up to #336. With
Soma Contractsdown from ~18min to ~3min, theTestjob is the long pole at 19m02s. Profiled it the same way.Where the 19 minutes goes
cargo nextest run --profile cicargo test --docThe change
Cache Cargopulls a 2.7 GBtarget/directory down at ~47 MB/s and pushes it back afterwards — 175s, roughly 15% of the job, spent purely on transfer.That step predates kache. On the very same run, kache reported:
kache is the compiler cache now, it hits ~96% from a local store via reflink, and it costs nothing to "restore". Paying a 2.7 GB network round trip to avoid compilation that kache already avoids is redundant.
Why the timeout also moves
30 → 45 minutes. Not a fix — a guard. This job hit exactly 30 minutes on the first run of #330, with every test passing and ~3 minutes of work left, so it was already brushing the ceiling. If my read is wrong and the target cache was pulling more weight than the numbers suggest, the job should get slower, not red.
How to tell if this was right
Test= 19m02s with the cache.target/identically and is deliberately untouched, so it acts as a control.If
Testdoes not improve, revert this — the measurement is the point, and it is one file.Not addressed
cargo test --doc(174s) and the wasm conformance step (173s) are real work, not overhead. They could move to parallel jobs to shorten the critical path, but each new job pays ~200s of runner setup and takes a slot from a small self-hosted pool, so that trade needs its own measurement rather than being bundled here.