perf(ci): persist V8 compile cache across test jobs#6008
Conversation
Set NODE_COMPILE_CACHE on the test, test-egg-bin and test-egg-scripts jobs and restore/save that directory with actions/cache, so Node's V8 compile cache survives within a run (shared by vitest workers and forked child processes) and across runs. Internally the cache is keyed by source hash + Node version, so stale restored entries are safely ignored. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
📝 WalkthroughWalkthroughThree CI jobs ( ChangesCI Node Compile Cache
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Dependency limit exceeded — report not shown. This pull request scan exceeded the 10,000-dependency limit applied to this scan, so the results are incomplete and may be inaccurate. To avoid reporting false positives, Socket has not posted a report. Upgrade your plan to raise the dependency limit and get complete reports, or view the partial scan in the dashboard. Socket is always free for open source. If this is a non-commercial open source project, contact us to request a free Team account. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 89-94: runner.temp is not valid in jobs.<job>.env, so update the
job-level NODE_COMPILE_CACHE definition in the CI workflow to use
github.workspace or move it into step-level env instead. Make the same
adjustment in the other two job-level env blocks that currently reference
runner.temp, and keep the change aligned with the existing cache setup around
NODE_COMPILE_CACHE.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| env: | ||
| # Persisted V8 compile cache (restored/saved by the "Cache Node compile | ||
| # cache" step). Set at job level so vitest workers and any forked child | ||
| # processes inherit NODE_COMPILE_CACHE. | ||
| NODE_COMPILE_CACHE: ${{ runner.temp }}/node-compile-cache | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
runner.temp isn't available in jobs.<job>.env.
Use ${{ github.workspace }} here, or move this to step-level env, and apply the same fix to the other two job-level blocks.
🧰 Tools
🪛 actionlint (1.7.12)
[error] 93-93: context "runner" is not allowed here. available contexts are "github", "inputs", "matrix", "needs", "secrets", "strategy", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details
(expression)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 89 - 94, runner.temp is not valid in
jobs.<job>.env, so update the job-level NODE_COMPILE_CACHE definition in the CI
workflow to use github.workspace or move it into step-level env instead. Make
the same adjustment in the other two job-level env blocks that currently
reference runner.temp, and keep the change aligned with the existing cache setup
around NODE_COMPILE_CACHE.
What
Enable Node's on-disk V8 compile cache (
NODE_COMPILE_CACHE) for the three test jobs (test,test-egg-bin,test-egg-scripts) and persist it withactions/cache:env.NODE_COMPILE_CACHE: ${{ runner.temp }}/node-compile-cacheso the value is inherited by vitest workers and any forked child processes, not just the top-level process.Cache Node compile cachestep (pinnedactions/cache@0057852…# v4, the same pin already used for the utoo store) keyednode-compile-cache-${{ runner.os }}-node${{ matrix.node }}-${{ hashFiles('pnpm-workspace.yaml', '**/package.json') }}with graduatedrestore-keys, so the cache survives within a run (shared across processes) and across runs.typecheckis intentionally excluded — it runstsgo/oxlint/tsdown(Rust), not Node module execution, so there is nothing for the V8 compile cache to help.Safety: Node validates each cache entry by source hash + Node version, so a stale restored entry is simply ignored (cache miss → recompile + rewrite). The change is low-risk and fully reversible.
Honest measurement (please validate on CI before marking ready)
Opened as draft because the local benefit I could measure is ~0 and the CI/Windows benefit is unverified:
egg-bin testrun, which populates 1346 cache files / 7.7 MB) showed no measurable speedup — warm-cache runs landed inside the no-cache noise band (20.3–22.4s), and were marginally slower due toflushCompileCachewrite overhead on exit.transform 1.51s+import 1.56sof a 2.9s run), plus process spawn — and the compile cache only caches the V8 compilation of already-transpiled JS, not the transform itself.How to validate: compare the test-job step durations run-over-run (the existing
Report parallelism metricsjob already surfaces CI timing), and/or check cache hit-rate in the Actions cache UI. Keep if it helps a platform; drop the env+step if not.Scope note
This does not touch
tools/egg-bin/test/coffee.ts, which forks the egg-bin CLI ~109× with a freshly-builtenvthat does not passNODE_COMPILE_CACHEthrough — so the compile cache does not currently reach those forks. That (and the Windowstest-egg-binslowdown generally) is being investigated separately on a Windows machine and is out of scope for this PR.🤖 Generated with Claude Code
Summary by CodeRabbit