Skip to content

docs(ci): the shared-key comment stated a fact the first warm run disproved - #618

Merged
wenzowski merged 1 commit into
mainfrom
claude/ci-performance-degradation-rplznx
Aug 21, 2026
Merged

wenzowski merged 1 commit into
mainfrom
claude/ci-performance-degradation-rplznx

Conversation

@wenzowski

@wenzowski wenzowski commented Aug 21, 2026 •

Copy link
Copy Markdown
Contributor

A one-comment correction, landed on its own because the thing it corrects is already on main.

96a665a added shared-key to the windows job and claimed — in the comment and in its commit message — that the composed cache key stayed byte-identical because the value matches key, so no existing entry was orphaned.

The first warm run produced the counter-evidence:

before  v0-rust-windows-windows-Windows_NT-x64-d44ea756-e905c9ae
after   v0-rust-windows-Windows_NT-x64-d44ea756-97231082

shared-key replaces the job-id component; it does not substitute the same word into it. One component where there were two, so adding it did orphan every prior entry.

The effect is harmless. Those entries were per-PR and unreadable across pull requests anyway, 40 of them had already been deleted to get under the 10GB cap, and the migration costs one cold build on each side. The comment is not harmless. This repository's gates exist because a claim nobody checked is worse than no claim, and a comment asserting a measured fact has to assert the one that was measured.

Worth naming the shape, since it is the argument for landing measurements rather than reasoning about them: the run I added to take a measurement is what disproved my own reasoning about the change that added it, inside one cycle.

While the warm run is on record

Run 32496045039, cache-warm-windows, green: restore 218s, cargo nextest run --no-run 345s, save 32s — 629s wall, ~21 billed minutes at 2×. Cold by construction, so the worst case rather than the answer. v0-rust-windows-Windows_NT-x64-d44ea756-97231082 is now on refs/heads/main at 182.2MB, total cache 7.18GB.

That 345s compile-only against 501s compile-plus-run also refines CLOUD-813: Windows execution is ~156s, 31% of the step, not the small share implied when nextest moved it by two seconds. Recorded there as a hypothesis, not a measurement.

Verification

mise run zizmor green, ci-local-parity green, and the comment is checked against the two live keys from the cache API rather than against the docs.

DO-NOT-CLOSE CLOUD-840


Generated by Claude Code

Summary by CodeRabbit

  • Documentation
    • Corrected the Windows Rust cache documentation to accurately describe how shared-key affects cache keys.
    • Clarified that changing the key format makes previous cache entries unavailable.

@linear-code

linear-code Bot commented Aug 21, 2026 •

Copy link
Copy Markdown
CLOUD-840 Every PR compiles the Rust tree cold: the caches exist, are byte-identical across PRs, and no PR can read another's — so `windows` writes 182MB per run that nothing ever restores

Why

CLOUD-813 measured cargo test against cargo nextest run on windows-latest and got 499s against 501s — two seconds. A runner 22% faster at executing this suite on Linux cannot move the job carrying 50.6% of the bill, which is only possible if execution is a small share of that step. The ~8m20s is compilation, and that is what this row is about.

Measured 2026-08-21 from the Actions cache API, not inferred from step timings:

  • The Windows rust cache key is stable and identical across pull requests — v0-rust-windows-windows-Windows_NT-x64-d44ea756-e905c9ae — present on refs/pull/572, 601, 604, 607 and 608, each about 182MB. Five copies of the same bytes.
  • It is absent from refs/heads/main. GitHub scopes a cache read to the run's own ref plus the base branch, so no pull request can read another's copy and none has a base-branch copy to inherit.
  • So every Windows run restores in 7s — a miss — compiles cold, and then spends 70s writing 182MB that only that pull request could ever read. The branch is deleted at merge.
  • The only rust caches on main belong to perf, because perf.yml runs on a schedule. rust.yml and ci.yml trigger on pull_request only, so theirs never land there.
  • The repository holds 10.64 GB against GitHub's 10 GB cap — 6.69 GB rust, 3.95 GB mise across 40 entries. Eviction is live and LRU, so the duplicates are evicting each other.

This reverses a recorded decision, and that decision named the conditions.

CLOUD-176 considered base-branch warming and recommended against it: about 178s of job-time per merge to save about 60s on one pull request's first run, "roughly 3× the job-time it saves." That costing lists four legs — ci, cross and darwin-link twice — all Linux, all billed at 1×. It predates the windows job entirely, and the arithmetic does not survive its arrival.

It also wrote down when to revisit, and both conditions now hold:

  • "PRs routinely land on their first CI run" — land merges on the first green lap, which is the ordinary case rather than the exception.
  • "the cold-vs-warm gap grows well past ~15s per job" — the Windows gap is most of 8m20s, at 2× billing.

Recording the reversal rather than quietly re-filing, because CLOUD-176's recommendation is written down and someone will find it.

Refinement — Ready (warm the base branch, and get under the cap so the warm entry survives)

  • Source of truth (§1). The Actions cache API for what exists and where, and the windows job's own step timings before and after for what it buys. Neither is a number a human types, and the cost side is measured on the runner that pays it rather than inferred from Linux.
  • Computable predicate (§2). After warming, a v0-rust-windows-… entry exists on refs/heads/main, and the first run of a fresh pull request restores it rather than reporting a miss. The gate is the restore, not the wall clock: a run that restores and is still slow is a different finding, and a run that is fast because nothing changed is not evidence.
  • No new trigger, and AGENTS.md is not bent (§2). release-plz.yml already runs on push: branches: [main] and is the only workflow that does. CLOUD-176 identifies it as the attachment point. The rule this repository holds is that CI does not re-run on main — a warm job compiles to fill a cache and asserts nothing, so it is not a second verdict on an already-tested SHA.
  • The cap is part of the work, not a footnote (§2). A cache the pull requests never read is pure eviction pressure on the one they would. 3.95 GB of mise tool caches across 40 entries and roughly 0.9 GB of duplicate Windows rust caches are the bulk of the overflow. Warming without reclaiming that space buys an entry that is evicted before it is read.
  • Deliberately not in scope (§2). The runner swap — CLOUD-813, settled. The toolchain-install overhead — CLOUD-812. The paths filter's omissions — CLOUD-828.
  • Effect (§3). write — CI configuration only. No verb, no command surface, no change to what any gate proves.
  • Output and exit (§5). Unchanged; a warm job asserts nothing and must not be able to red a landing.
  • Commit / bump (§6). ci(cache) — no bump. Nothing under crates/.
  • Test obligation (§7). The cost and the benefit both stated as measurements before the change is kept: the warm job's own billed minutes, against the first-run delta on a fresh pull request measured against the 499s baseline on record. A warm cycle that costs more than it saves is the outcome CLOUD-176 predicted and must be recorded as such rather than absorbed.
  • Blockers (§8). None. relatedTo CLOUD-176 (the decision this reverses, and whose revisit clause authorises it), CLOUD-813 (whose negative result is what redirects the effort here), CLOUD-812 (the overhead layer of the same ledger), CLOUD-398 (the job graph half).

Acceptance

  • A v0-rust-windows-… cache exists on refs/heads/main.
  • The first Rust run of a fresh pull request reports a cache hit rather than a 7s miss, shown against a run that did not.
  • Total cache size is under the 10 GB cap, so the warm entry is not evicted before it is read.
  • The warm cycle's billed minutes and the per-run saving are both recorded, whichever way the trade falls.
  • The Windows job stops writing 182MB on every run that nothing restores.

Review in Linear

@coderabbitai

coderabbitai Bot commented Aug 21, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 87f4c6f8-00be-4d47-bec9-b8d8c97961fe

📥 Commits

Reviewing files that changed from the base of the PR and between f6f30bb and 82301e3.

📒 Files selected for processing (1)
  • .github/workflows/rust.yml
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/rust.yml

Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The Windows Rust cache documentation now describes the measured cache-key formats. It states that shared-key replaces the job component and that existing entries migrate to the new format.

Changes

Rust cache documentation

Layer / File(s) Summary
Windows Rust cache-key format
.github/workflows/rust.yml
The documentation explains the before-and-after key formats, the replacement of the job component by shared-key, and the migration of existing cache entries.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to 82301

This localized documentation correction does not change workflow behavior, and no actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the CI documentation correction and the measured shared-key behavior that prompted it.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/ci-performance-degradation-rplznx

Comment @coderabbitai help to get the list of available commands.

…proved

`96a665a` added `shared-key` to the `windows` job and claimed, in the comment
and in its commit message, that the composed cache key stayed byte-identical
because the value matches `key` — so nothing existing was orphaned.

The two live entries say otherwise:

  before  v0-rust-windows-windows-Windows_NT-x64-d44ea756-e905c9ae
  after   v0-rust-windows-Windows_NT-x64-d44ea756-97231082

`shared-key` REPLACES the job-id component; it does not substitute the same word
into it. One component where there were two. So adding it did orphan every prior
entry.

The effect is harmless — those entries were per-PR and unreadable across pull
requests anyway, 40 of them had already been deleted to get under the 10GB cap,
and the migration costs one cold build on each side. What is not harmless is the
comment: this repository's gates exist because a claim nobody checked is worse
than no claim, and a comment asserting a measured fact has to assert the one
that was actually measured.

The first warm run is what produced the counter-evidence, which is the argument
for landing a measurement rather than reasoning about it:
`v0-rust-windows-Windows_NT-x64-d44ea756-97231082`, 182.2MB, now on
`refs/heads/main`.

Refs: CLOUD-840
@wenzowski
wenzowski marked this pull request as ready for review August 21, 2026 16:02
@wenzowski
wenzowski force-pushed the claude/ci-performance-degradation-rplznx branch from eac8369 to 82301e3 Compare August 21, 2026 16:02
@sonarqubecloud

Copy link
Copy Markdown

@wenzowski

Copy link
Copy Markdown
Contributor Author

/fast-forward

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant