ci: parallelize docker + forge cache + mold linker (toward sub-2min CI)#86
Open
koko1123 wants to merge 3 commits into
Open
ci: parallelize docker + forge cache + mold linker (toward sub-2min CI)#86koko1123 wants to merge 3 commits into
koko1123 wants to merge 3 commits into
Conversation
Drop the docker job's needs so it no longer runs as a serial tail after every test job; the 'test' aggregate gate still requires it. Also poll the container for readiness instead of a fixed 'sleep 15' in the image smoke test.
Cache tests/contracts/{out,cache} keyed on the .sol sources + foundry.toml so
forge build is a no-op when the mocks are unchanged, saving a solc compile in
each of the three Foundry-dependent test jobs.
Install mold in the cached builder layer and route the linker to it via RUSTFLAGS (keeping -D warnings) for the dependency cook and the final build, speeding the link of the alloy + aws-sdk binary on the docker job -- the critical-path pole, which now runs in parallel with the tests. Verified locally: image builds with mold 1.10.1 and the binary boots and serves /.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR adds Foundry output caching to three CI test jobs, allows the Docker job to run in parallel while retaining aggregate gating, replaces fixed startup delay with readiness polling, and configures Rust builds to use the mold linker. ChangesCI and Docker workflow
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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.
Push toward sub-2-minute PR CI, driven by benchmarks rather than guesswork.
What the benchmark showed (Apple M4; GHA
ubuntu-latest4-vCPU ≈ ~3× slower)Two facts drive the plan: (1) jobs are compile/link-bound, not test-bound; (2) the
dockerjob was the tall pole and ran as a serial tail (needs: [all tests]), so warm wall-clock was ~max(tests) + docker≈ 4min — you can't get near 2min with a heavy job stacked on the end.Changes (this PR)
dockerruns in parallel with the tests (dropped itsneeds). Wall-clock becomesmax(job)instead oftests → docker. Thetestaggregate gate still listsdocker, so a broken image still blocks the PR. Also replaced the image smoke test's fixedsleep 15with a readiness poll.tests/contracts/{out,cache}keyed on the.solsources) soforge buildis a no-op in the three Foundry jobs when the mocks are unchanged.RUSTFLAGSroutes the linker to it (keeping-D warnings) for the cook + final build. Speeds the alloy+aws binary link on the (now-parallel) docker pole. Verified locally: builds with mold 1.10.1 and the binary boots.Paired with #85
#85 adds
opt-level = 2for release (I/O-bound service; measured 26.5s → 19.4s, ~27% off the crate recompile). #85 + this PR together attack the docker pole: parallel + opt-2 compile + mold link.Honest scope: why mold is NOT in the CI test jobs (yet)
I deliberately scoped mold to the image build. In the CI test jobs, installing mold costs ~8s (apt) which roughly cancels the link savings, and setting
RUSTFLAGScold-invalidates the sharedrust-cache(cikey) — so it's not a clear win and it degrades the next few runs. If we want it there, the right form is a prebuilt-binary install (not apt) and accepting the one-time cache reset — happy to do it as a follow-up if you want to chase the last few seconds.Realistic expectation
Warm, with docker parallelized + opt-2 + mold + forge-cache, the tall pole becomes either the heaviest test job or the parallel docker build, landing ≈ 90–120s — sub-2-min is achievable on warm runs, borderline. It won't hold on a run that changes
Cargo.lock/Dockerfile(cache miss → cold). The one bigger lever left if you want it reliably under 2min is not building the image on every PR (gate the docker job to Dockerfile/deps changes) — flagged, not done here.Summary by CodeRabbit
Bug Fixes
Performance
CI Improvements