Cut PR build time: stop deleting the Hugo image cache, drop GOGC=3 - #21060
Cut PR build time: stop deleting the Hugo image cache, drop GOGC=3#21060CamSoper wants to merge 4 commits into
Conversation
Successful `Pull Request` runs have been landing at 14-17 minutes. The
"Build and deploy" step is 15m45s of a 17m12s run, and the Hugo build
alone is 9m05s of that.
Two causes, plus two missing guardrails.
Hugo's processed-image cache was being deleted 0.2s after it was
restored. `make ci_pull_request` runs `make ensure`, `ensure` depends on
`clean`, and `scripts/clean.sh` ran `rm -rf resources` -- the exact
directory actions/cache had just restored:
15:59:59.20 Cache restored from key: hugo-resources-342b83e3 (89 MB)
15:59:59.37 ./scripts/clean.sh
Hugo then re-encoded every blog feature image from scratch on every run.
The cache still restored and saved cleanly each time, so the waste never
showed up in the job log -- only in the template metrics, where
blog/feature-image.html averaged 994ms per call across 718 calls
(11m53s cumulative). Measured against a 6,549-page Hugo 0.157.0 harness
built from the real partial and all 149 real feature.png bundles, that
partial costs 593ms/call cold and 343us/call warm. `clean.sh` now skips
`rm -rf resources` and `yarn cache clean` when CI is set; the remaining
operations are no-ops on a fresh clone, and local behavior is unchanged.
`hugo mod clean` was verified not to touch resources/ under this repo's
module config, so the guard is sufficient.
Hugo also ran under GOGC=3, which collects once the heap grows 3% over
live heap. On the same harness with a warm cache that costs 1.7-3x in
wall time (9.4-10.0s vs 5.6-5.8s) to save ~35% peak RSS, and the penalty
grows with heap size. GOMEMLIMIT=12GiB expresses the actual intent -- do
not exhaust the 16GB runner -- without paying for a full GC behind every
allocation. Fixing the image cache also lowers peak memory, since image
decoding was the allocation-heavy part.
build-and-deploy.yml restores the same hugo-resources cache and runs the
same clean via `make ci_push`, so master deploys were hitting both
problems too and get the same fix. The cached contents were always valid
-- they were just discarded before use -- so PRs benefit immediately
without a warm-up period.
Also adds two guardrails this workflow was missing:
- A concurrency group with cancel-in-progress. Pushing three commits in
a row ran three full builds to completion; branches did exactly that
four times today. Preview buckets are keyed by PR number and head sha,
so a cancelled run can only orphan its own, which pr-closed.yml and
bucket-cleanup.yml already reap.
- The ci-build-duration-alert.sh check that build-and-deploy.yml already
has. PR builds had none, which is how they drifted to ~17 minutes
without anyone being told. Threshold 15m, above the expected new
baseline; worth tightening once that baseline settles.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X5pkk2XuQW6T84W6XHFiyM
|
Your site preview for commit ff95472 is ready! 🎉 http://www-testing-pulumi-docs-origin-pr-21060-ff95472e.s3-website.us-west-2.amazonaws.com |
github-actions
Bot
added
review:triaging
![unblocked[bot]](https://avatars.githubusercontent.com/in/166219?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No issues found
About Unblocked
Unblocked has been set up to automatically review your team's pull requests to identify genuine bugs and issues.
📖 Documentation — Learn more in our docs.
💬 Ask questions — Mention @unblocked to request a review or summary, or ask follow-up questions.
👍 Give feedback — React to comments with 👍 or 👎 to help us improve.
⚙️ Customize — Adjust settings in your preferences.
Pre-merge Review — Last updated 2026-08-24T21:58:51ZTip Summary: This is a CI/build-infrastructure PR, not a content change: it stops Independently confirmed for this commit: Review confidence:
Investigation log
🔍 Verification trail12 claims extracted · 2 verified · 2 unverifiable · 3 contradicted · 1 detector findings
🚨 Outstanding in this PRNo outstanding findings.
|
github-actions
Bot
added
review:outstanding-issues
Review findings (all four verified against the repo before changing anything): Outstanding — the concurrency comment named the wrong sweeper. PR preview buckets are `www-testing-*`, created by this workflow's `environment: testing` job. `bucket-cleanup.yml` runs under `environment: production`; the daily sweep for testing buckets is `bucket-cleanup-testing.yml`. Corrected. Outstanding — BUILD-AND-DEPLOY.md L4010 still documented `GOGC | 3 | Workflow`, which no build path sets after the previous commit. Replaced with the GOMEMLIMIT row. Low-confidence — GOMEMLIMIT leaked into local builds. Correct and worth fixing: `make build` (Makefile L79) and scripts/laptop-deploy.sh L54 both invoke build-site.sh, and a 12GiB ceiling sized for the CI runner is no ceiling at all on a 16GB laptop. Now set only under CI, and an explicit GOMEMLIMIT always wins so a constrained machine can cap it further. Local builds fall back to Go's defaults. Low-confidence — whether `hugo mod clean` undoes the new guard. Tested both ways: with a populated resources/_gen it exits 0 and preserves every entry, in a bare harness and against this repo's real `module:` config on the pinned 0.157.0. Rather than rest on that, moved it inside the guard. It is a no-op on a fresh runner anyway (nothing is in the module cache that early), so CI loses nothing and the question stops being askable. Plus the two follow-ups the previous commit only documented: - Stamp the pinned Hugo version into the hugo-resources cache key. Verified that 0.156.0 reuses 0.157.0's cached output byte-for-byte, so without this an upgrade keeps serving images encoded by the older version indefinitely. Applied identically in pull-request.yml and build-and-deploy.yml -- they must match or PRs stop inheriting master's cache. Kept as a literal rather than interpolating the version pin: a failed interpolation there would silently install the wrong Hugo, a far worse failure than key drift, which only costs a stale encoder. - Add `hugo --gc` to prune cache entries the build no longer references, since nothing else reclaims superseded ones. Deploy path only, and only in CI, so a PR build cannot evict entries master still needs. Note: the new cache key means the first run after this lands finds no `hugo-resources-0.157.0-*` and repopulates from cold, so expect one ~17-minute build before the ~7-minute baseline returns. Verified by extracting the real conditional block and running it against a stub hugo: PR path gets no --gc, both deploy paths get it, local builds get neither --gc nor GOMEMLIMIT, and a preset GOMEMLIMIT survives. clean.sh under CI preserves resources/ and runs neither yarn cache clean nor hugo mod clean; unset, it still does all three. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X5pkk2XuQW6T84W6XHFiyM
github-actions
Bot
added
review:stale
|
@claude #update-review All four findings addressed in dda8e00, plus the two follow-ups the PR body had listed as out of scope. I verified each finding against the repo before changing anything — all four were correct. 🚨 [L13] 🚨 [L37]
Follow-ups now included:
Verification: I extracted the real conditional block from On the Generated by Claude Code |
github-actions
Bot
added
review:in-progress
|
🤖 Review updated on @CamSoper's request. |
github-actions
Bot
added
review:outstanding-issues
…ment Second review pass found the version stamp covered two of the four workflows that cache `path: resources`. It was right, and the miss had two consequences: testing-build-and-deploy.yml (L91/L93) and pulumi-cli-docs.yml (L151/L153) kept the stale-encoder bug the stamp exists to fix, and their saves landed in a namespace the two updated workflows no longer restore from, so that cache work was written and never read. The comment also entrenched the wrong list for whoever upgrades Hugo next. All four now use `hugo-resources-0.157.0-`, and all four pin hugo-version 0.157.0. The comment names the full set so the next upgrade bumps them together. Also corrected the --gc comment, which claimed a narrower scope than the code has. The guard is on the non-preview `else`, so it covers every non-preview CI build -- the two deploy workflows plus any CI job running `make build`, which pulumi-cli-docs.yml L246 does. That is fine rather than a bug: all of them build the full site and reference the same set of entries. PR preview builds remain the deliberate exclusion. The comment now describes the predicate the code actually tests. Verified by re-running the extracted conditional against a stub hugo, including the pulumi-cli-docs case (CI set, DEPLOYMENT_ENVIRONMENT unset) the review identified: it takes the production branch and gets --gc, as the new comment says. PR preview gets none; local builds get neither --gc nor GOMEMLIMIT. All four workflows parse and their hugo pins match the new key prefix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X5pkk2XuQW6T84W6XHFiyM
github-actions
Bot
added
review:stale
|
@claude #update-review Both findings from the second pass addressed in 9437eb1. The outstanding one was a real miss on my part, not a comment problem. 🚨 [L105] Version stamp covered two of four workflows. Fixed properly. Confirmed exactly four workflows cache
Verification: re-ran the extracted conditional against a stub Thanks for the empty-array check on Still expect one cold build after this lands, now across all four workflows rather than two, since nothing has written a Generated by Claude Code |
github-actions
Bot
added
review:in-progress
|
🤖 Review updated on @CamSoper's request. |
github-actions
Bot
added
review:no-blockers
The literal `hugo-resources-0.157.0-` prefix moved the drift risk rather than removing it: revving Hugo meant remembering to bump the version in eight more places (four keys plus four restore-keys), and forgetting one reintroduces exactly the bug the stamp exists to prevent -- silently serving images encoded by the previous Hugo. peaceiris/actions-hugo already runs well before the cache step in all four workflows (verified: hugo@3<resolve@9<cache@10 in pull-request, 4<10<11 in build-and-deploy and testing-build-and-deploy, 9<13<14 in pulumi-cli-docs), so the key can be derived from the binary that will actually produce the cache. Revving hugo-version now updates the key on its own, and the four workflows cannot drift apart because none of them states the version. Deliberately does not interpolate into the `hugo-version:` pins themselves. A failed interpolation there would silently install the wrong Hugo, which is worse than a stale encoder; deriving downstream keeps that risk at zero while removing the drift. The guard is load-bearing and was wrong on the first attempt. `sed -E` echoes its input back unchanged when the pattern does not match, so `[ -n "$ver" ]` passed on garbage -- a build with an unrecognizable version banner would have keyed every workflow into one unversioned `hugo-resources--` namespace, the exact failure being defended against. Fixed with `sed -nE ... p` (non-match yields empty) and by checking emptiness before appending `-extended`, so the guard cannot pass on a value that is only the suffix. Exercised against Hugo 0.157.0 and 0.156.0 (both -> `<semver>-extended`), a garbage banner, and empty output; the last two now exit 1. Note this does not make revving Hugo a one-line change. `0.157.0` is still hardcoded in ~17 places -- mise.toml twice (Linux and macOS pins) and 15 `hugo-version:` pins across workflows, including several this PR never touched. Collapsing those onto a single source of truth is worth doing separately; this change just stops the cache keys from being part of that problem. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X5pkk2XuQW6T84W6XHFiyM
github-actions
Bot
added
review:stale
|
@claude #update-review Pushed ff95472, replacing the literal The literal moved the drift risk rather than removing it — revving Hugo meant remembering eight more places (four keys plus four restore-keys), and missing one silently reintroduces exactly the bug the stamp exists to prevent. - name: Resolve Hugo version for the cache key
id: hugo-version
run: |
hv="$(hugo version)"
ver="$(printf '%s' "$hv" | sed -nE 's/^hugo v([0-9]+\.[0-9]+\.[0-9]+).*/\1/p')"
[ -n "$ver" ] || { echo "could not parse hugo version from: $hv" >&2; exit 1; }
case "$hv" in *+extended*) ver="$ver-extended" ;; esac
echo "version=$ver" >> "$GITHUB_OUTPUT"Revving One thing worth a close look, since it's the load-bearing part and I got it wrong first. My initial guard was Verification: step ordering asserted programmatically from the parsed YAML in all four ( Also moved the pre-existing "Persist Hugo's processed-image cache" comment back down to the cache step, since the new step had come between it and what it describes. Not fixed here, and worth knowing: this does not make revving Hugo a one-line change. Generated by Claude Code |
github-actions
Bot
added
review:in-progress
|
🤖 Review updated on @CamSoper's request. |
github-actions
Bot
added
review:no-blockers
4 participants
Proposed changes
Successful
Pull Requestruns have been landing at 14–17 minutes. Profiling run 32500502599 (17m12s): "Build and deploy" is 15m45s of it, and the Hugo build alone is 9m05s.Two causes, plus two missing guardrails. Three runs on this branch isolate each fix, because the middle one ran cold with the GC change:
blog/feature-image.htmlGOGC=3(before)GOMEMLIMIT(15484)GOMEMLIMIT(15480)The GC change is worth ~4m50s, the cache fix ~5m28s, and they're close to independent.
make ensurealso dropped 1m53s → 1m05s from the preserved yarn cache.1. The Hugo image cache was deleted 0.2s after it was restored
make ci_pull_request→make ensure,ensuredepends onclean, andscripts/clean.shranrm -rf resources— the exact directoryactions/cachehad just restored:Hugo then re-encoded every blog feature image from scratch on every run. The cache still restored and saved cleanly each time, so the waste never showed up in the job log — only in the template metrics, where
blog/feature-image.htmlaveraged 994ms per call across 718 calls.clean.shnow skipsyarn cache clean,rm -rf resources, andhugo mod cleanwhenCIis set. The rest are no-ops on a fresh clone, and local behavior is unchanged.hugo mod cleantested clean on the pinned 0.157.0 (exits 0, preserves everyresources/_genentry, against this repo's realmodule:config) but is inside the guard anyway — it's a no-op that early on a fresh runner, so CI loses nothing and the question can't come back.Ruled out along the way, so they don't muddy future diagnosis: the per-run preview
baseURLdoes not bust the cache, nor do fresh-clone mtimes. AndProcessed images │ 2755counts.Processcalls, not encodes — it reads 2755 both before and after this change, so it is not a cache-health signal.2.
GOGC=3build-site.shran Hugo underGOGC=3, collecting once the heap grows 3% over live heap. Measured on a 6,549-page Hugo 0.157.0 harness with a warm cache:GOGC=3(before)GOGC=100GOGC=100+GOMEMLIMIT1.7–3× slower for ~35% peak RSS saved, and the penalty grows with heap size.
GOMEMLIMIT=12GiBexpresses the actual intent — don't exhaust the 16GB runner — without paying a full GC behind every allocation. Set only under CI, sincebuild-site.shis also the local build path (make build,scripts/laptop-deploy.sh) where a 12GiB ceiling would be no ceiling at all on a 16GB laptop; an explicitGOMEMLIMITalways wins.--templateMetricswas also measured and costs nothing detectable, so it stays.3 & 4. Two guardrails this workflow was missing
cancel-in-progress. Pushing three commits in a row ran three full builds to completion; branches did exactly that four times in one day. Already demonstrated on this PR — run 15483 was cancelled when the next commit landed. Preview buckets are keyed by PR number and head sha, so a cancelled run can only orphan its own —pr-closed.ymldeletes every*-pr-<num>-*bucket on close andbucket-cleanup-testing.ymlsweeps thewww-testing-*previews daily. Both cache steps run withsave-always: false, so a cancelled run cannot save a partial cache.ci-build-duration-alert.sh, whichbuild-and-deploy.ymlalready has and PR builds did not. That is how these drifted to ~17 minutes without anyone being told; the script's own header notes the build grew ~60% over three weeks in July the same way. Threshold 15m — it reportedBuild and deploy took 10m (655s) ... threshold is 15mon the cold run, quiet as intended, but would have caught the old 15m45s.5 & 6. Cache hardening (added after review)
Making the cache actually live activates two latent issues that didn't matter while it was being nuked. Both verified, both fixed here:
path: resources—pull-request.yml,build-and-deploy.yml,testing-build-and-deploy.yml,pulumi-cli-docs.yml— and all four now key and restore onhugo-resources-0.157.0-and pinhugo-version: '0.157.0'. They must move together or they stop sharing a cache; the comment in each names the full set. Kept as a literal rather than interpolating the pin: a failed interpolation there would silently install the wrong Hugo, a worse failure than key drift.hugo --gcprunes entries the build no longer references, which is what bounds growth — nothing else reclaims superseded ones. Applied to every non-previewbuild under CI: the two deploys plus any CI job runningmake build(pulumi-cli-docs.ymldoes, and it's a full-site build, so it prunes against the same reference set). PR preview builds are the deliberate exclusion — they share the namespace, and a narrower view could drop entries the others need.Blast radius
build-and-deploy.ymlrestores the samehugo-resourcescache and runs the samecleanviamake ci_push, so master deploys were hitting both problems too and get the same fix.hugo-resources-0.157.0-*entry yet, so the first run of each of the four repopulates from cold. That's the 12m22s column above, not a regression; the run after returns to ~7 minutes.New profile
Hugo is no longer the bottleneck (48.8s, 12% of the run). What's left, roughly evenly: runner setup 1m01s,
make ensure1m05s,generate-docs-content.js1m01s, then S3 sync 20s / Cypress 23s / search index 21s /pulumi preview35s / redirects 18s. There's no single large win left — going below ~5 minutes means chipping at several things.Unreleased product version (optional)
N/A
Related issues (optional)
N/A