perf: optimize CI pipeline: caching, artifact sharing, and fixture reuse #542
perf: optimize CI pipeline: caching, artifact sharing, and fixture reuse #542guha-rahul wants to merge 14 commits intoblockblaz:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Optimizes GitHub Actions CI runtime by upgrading caching infrastructure and adding fixture reuse to avoid regenerating LeanSpec fixtures when inputs haven’t changed.
Changes:
- Upgraded
actions/cacheusage fromv3tov4across workflows. - Added a new cache for
leanSpec/fixturesand conditionally skipped fixture generation on cache hits (CI workflow).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| .github/workflows/risc0.yml | Upgrades Zig package caching action to actions/cache@v4. |
| .github/workflows/ci.yml | Upgrades Zig cache to v4 and adds LeanSpec fixtures caching + conditional fixture generation. |
| .github/workflows/auto-release.yml | Upgrades Zig package caching action to actions/cache@v4. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
.github/workflows/ci.yml
Outdated
| restore-keys: | | ||
| ${{ runner.os }}-leanspec- |
There was a problem hiding this comment.
restore-keys is likely counterproductive here: if the primary key doesn’t match, cache-hit will be false and the next step runs uv run fill --clean, which will delete the restored leanSpec/fixtures anyway. This can add time by downloading a large stale cache only to immediately wipe it. Consider removing restore-keys (or dropping --clean if the intent is to reuse partial restores).
| restore-keys: | | |
| ${{ runner.os }}-leanspec- |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
.github/workflows/ci.yml:152
buildnow depends ondeps, butdepsis a matrix job (ubuntu + macos). In GitHub Actions, a matrix job ID inneedsgates on all matrix runs, so eachbuildmatrix run will wait for bothdepsOS runs to finish. This increases the critical path and couples the OS lanes. Consider either removing theneedsedge (rely on cross-run caches), or splittingdepsinto separate job IDs per OS so each lane only waits for its own warmup.
build:
needs: deps
runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [ubuntu-latest, macos-latest]
.github/workflows/ci.yml:297
testnow depends ondeps, but sincedepsis a matrix job,needs: depswill make eachtestmatrix run wait for alldepsmatrix runs (both OSes). This likely increases overall wall-clock time and reduces parallelism. Suggest removing this dependency or restructuringdepsinto per-OS job IDs to avoid cross-OS gating.
test:
name: test
needs: deps
runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [ubuntu-latest, macos-latest]
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
.github/workflows/ci.yml
Outdated
|
|
||
| dummy-prove: | ||
| name: Dummy prove | ||
| needs: deps |
There was a problem hiding this comment.
dummy-prove now depends on deps, but because deps is a matrix job, this will gate each dummy-prove matrix run on completion of both deps OS runs. That reduces concurrency and can lengthen the pipeline. Consider dropping the dependency or splitting deps into per-OS jobs so each lane only waits on its matching warmup.
| needs: deps |
.github/workflows/ci.yml
Outdated
| uses: actions/cache@v4 | ||
| with: | ||
| path: leanSpec/fixtures | ||
| key: ${{ runner.os }}-leanspec-${{ hashFiles('.gitmodules') }} |
There was a problem hiding this comment.
The LeanSpec fixtures cache key is based on hashFiles('.gitmodules'), which won't change when the leanSpec submodule revision changes. Once a cache entry exists for this key, future updates to LeanSpec (or to the fixture generation logic) can keep reusing stale fixtures and also won't be able to save an updated cache under the same key. Use a key that tracks the LeanSpec revision/inputs (e.g., hash relevant files under leanSpec/ like its lockfile/scripts, or otherwise incorporate the submodule commit) so the cache invalidates correctly.
| key: ${{ runner.os }}-leanspec-${{ hashFiles('.gitmodules') }} | |
| key: ${{ runner.os }}-leanspec-${{ hashFiles('leanSpec/**') }} |
This pr
tracks figure and fix the CI long CI runs #385