ci(ci): cache baseline lib instead of baseline JSON - #506
Conversation
Change the Performance Regression Check job's cache from
`/tmp/perf_baseline_${mode}.json` (the benchmark output JSON, which encoded
*measurements* made on whatever runner first populated the cache for a given
base.sha) to `/tmp/baseline_lib` (the built baseline `libdtvmapi.so`, a
deterministic function of base.sha alone).
Effect: every PR perf-check job now re-runs baseline benchmarks on the same
ubuntu-latest runner that runs the PR benchmarks, eliminating cross-runner
cache-stale noise. The cache-miss path was already same-runner A-B
(`.ci/run_test_suite.sh:312-334` `elif [ -n "$BENCHMARK_BASELINE_LIB" ]`
branch); this change makes cache-hit follow the same dispatch by dropping
the BENCHMARK_BASELINE_CACHE export.
Empirical motivation (PR DTVMStack#493 commit 5c11550 interpreter perf-check, CI
cache hit on base.sha=c644fbe): CI reported +13-18% regressions on 7 synth
benchmarks; local same-machine A-B-A reported all deltas inside the
baseline self-drift band (max +0.6%). The PR's diff was 100% compiler-only,
no `src/evm/` files, so the CI deltas were entirely cross-runner noise.
Also fixes a separate bug in the baseline-build step: `cp build/lib/*`
dereferences the SONAME symlink chain (`libdtvmapi.so` -> `.0.1` -> `.0.1.0`)
producing 3 full copies (~105 MB) plus any libgtest*.a present, instead of
the single ~35 MB real file. Replaced with `cp "$(readlink -f
build/lib/libdtvmapi.so)" /tmp/baseline_lib/libdtvmapi.so`.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Add an in-source comment block in front of the perf-check Restore step so the rationale is discoverable from the workflow file directly. The bench JSON encodes runner-specific measurements; caching it across runners is what produced PR DTVMStack#493's spurious +15% interpreter "regressions". Caching the deterministic build output instead forces same-runner A-B comparison. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Promotes ~/changes/2026-05-15-perf-check-baseline-lib-cache/README.md into the project's docs/changes/ tree as part of opening the upstream PR. Empirical verification across two fork CI runs: - 25897744713 (cache miss, after re-running 3 spdlog-503 transient failures): both perf-check matrix jobs build baseline lib, then run baseline + PR bench on the same runner. 11/11 jobs green. RESULT: PASS for both modes. - 25900131271 (cache hit, same base.sha, no-op head bump): cache restored, Build-baseline-library step skipped, bench step still runs baseline + PR bench on the same runner via the dispatcher's elif branch. 11/11 jobs green on first try, no spdlog-503 retries. RESULT: PASS for both modes. Cross-runner noise reduction observed on synth/ADDRESS|BYTE|CALLER|EQ|GT|LT| ISZERO benches (PR DTVMStack#493 reported +13-18% all turned into <1% on same-runner). Only 1 micro-benchmark filtered by the baseline<5us gate versus 25 in the cross-runner cache-stale comparison. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the CI performance regression workflow to reduce cross-runner noise by caching a deterministic baseline artifact (the built libdtvmapi.so) rather than caching benchmark result JSON produced on a different runner. This ensures baseline and PR benchmarks are executed on the same runner while still avoiding rebuilding the baseline library on cache hits.
Changes:
- Switch
performance_regression_checkcache from/tmp/perf_baseline_${mode}.jsonto/tmp/baseline_libkeyed bybase.sha. - Fix baseline library staging to copy only the resolved real
.so(avoid globbing/symlink dereference duplicates) and removeBENCHMARK_BASELINE_CACHEso the dispatcher always takes the baseline-lib benchmarking path. - Add a design/change doc describing the motivation, implementation, and validation runs.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
docs/changes/2026-05-15-perf-check-baseline-lib-cache/README.md |
Adds rationale/design notes for switching perf-check baseline caching from JSON results to a baseline shared library. |
.github/workflows/dtvm_evm_test_x86.yml |
Updates perf regression job to cache baseline libdtvmapi.so and adjust the build/copy/env wiring to force same-runner baseline measurements. |
Comments suppressed due to low confidence (1)
docs/changes/2026-05-15-perf-check-baseline-lib-cache/README.md:65
- The “Touched files” bullet says only
.github/workflows/dtvm_evm_test_x86.ymlwas touched, but this change is documented indocs/changes/.../README.mdas well (this file). If the intent is “behavior-affecting files”, consider clarifying the wording to avoid a technically incorrect statement.
## Impact
- **Touched files**: `.github/workflows/dtvm_evm_test_x86.yml` only — `performance_regression_check` job.
- **Behavior change**: per-PR perf-check job wall-clock increases on cache-hit runs by the baseline-bench step (which under the old scheme was skipped); cache-miss runs are unchanged.
- **Cost / quota**: cache-hit runs now do both a baseline bench AND a PR bench on the same runner (cache-miss already does both, plus a baseline build). Both modes (`matrix.mode = {interpreter, multipass}`) run per PR. Empirically verified on fork CI runs `25897744713` (cache miss) and `25900131271` (cache hit) that all 11 non-Lint jobs go green; cache-hit runs spend longer than the old cache-hit scheme (which skipped baseline bench entirely) but shorter than cache-miss (which adds the baseline build on top). Acceptable under the public-repo GitHub Actions allowance for our PR volume.
- **No source-code change**, no test runner change, no `.ci/run_test_suite.sh` change. The `elif [ -n "$BENCHMARK_BASELINE_LIB" ]` path it routes to already exists and is exercised today on cache miss.
- **Downstream**: reviewers stop chasing false-positive interpreter regressions on PRs whose diff is compiler-only (and vice versa). Perf-check signal-to-noise increases without changing the threshold.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
⚡ Performance Regression Check Results✅ Performance Check Passed (interpreter)Performance Benchmark Results (threshold: 25%)
Summary: 194 benchmarks, 0 regressions ✅ Performance Check Passed (multipass)Performance Benchmark Results (threshold: 25%)
Summary: 194 benchmarks, 0 regressions |
Two wording fixes flagged by github-copilot[bot] review: - L16 "The current job..." -> "The pre-PR job (on main):" so the YAML snippet reads correctly after merge/archive. - L65 "Touched files: ... only" -> "Behavior-affecting files: ... only" + note that README.md itself ships in the same commit. No content change beyond wording. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> (cherry picked from commit 8e57176)
Summary
Switch the
Performance Regression Checkjob (.github/workflows/dtvm_evm_test_x86.yml) from caching the baseline benchmark output JSON to caching the built baselinelibdtvmapi.so. Every PR then re-runs baseline benchmarks on the same runner that runs the PR benchmarks, eliminating cross-runner cache-stale noise.Also fixes a separate
cp build/lib/*bug in the same step that dereferenced the SONAME symlink chain and produced 3 full copies (~105 MB) of the.soplus anylibgtest*.amatches. Replaced withcp "$(readlink -f build/lib/libdtvmapi.so)" /tmp/baseline_lib/libdtvmapi.so.Full design doc:
docs/changes/2026-05-15-perf-check-baseline-lib-cache/README.md(added in commit190c0f9).Motivation
PR #493 (
perf-baseline-interpreter-c644fbecb1a49051a62c6fd8bd3f2d7bb807b514cache hit) reported a cluster of synth-bench interpreter regressions:synth/EQ/b1synth/CALLER/a1synth/ISZERO/u0synth/LT/b1synth/GT/b1synth/ADDRESS/a1synth/BYTE/b1The PR's diff was 100% compiler-side (
src/compiler/evm_frontend/*+src/action/evm_bytecode_visitor.h, whereEVMByteCodeVisitoris only instantiated byevm_mir_compiler.cpp:97and the interpreter never reaches it). Local same-machine A-B-A on those exact benches reported deltas inside the baseline self-drift band (≤ 0.6%). The CI deltas were cross-runner cache-stale measurements.Root cause: the
actions/cache@v4step keyed/tmp/perf_baseline_${mode}.jsononpull_request.base.sha, so two PRs with the samebase.shacompared the current PR's freshly-run benchmark against a JSON the first PR's CI runner produced —ubuntu-latestrunners are heterogeneous (different CPU steppings, neighbor load, thermal state), and synth microbenchmarks (1-10 µs scale) are exactly where cross-runner variance is largest.The
.ci/run_test_suite.sh:312-334dispatcher already had the right code path — theelif [ -n "$BENCHMARK_BASELINE_LIB" ]branch builds the baseline lib once and benches it on the current runner. This PR makes the workflow always take that path by:libdtvmapi.soinstead of the bench output JSON.BENCHMARK_BASELINE_CACHEenv var so the bash dispatcher falls through to theelifbranch on both cache-hit and cache-miss.The cache is now content-deterministic for a given
base.sha: two PRs hitting the same key restore byte-identical.sofiles and each runs its own baseline bench locally.Verification
Tested on the personal fork (
abmcar/DTVM) before opening here:Cache-miss run (abmcar/DTVM run
25897744713, after re-running 3 transient spdlog-503 download failures):Performance Regression Check (interpreter)and(multipass)reportedCache not found for input keys: perf-baseline-lib-<mode>-c644fbe...Build baseline librarystep ran;/tmp/baseline_lib/ended with a single SONAME-resolved real fileTotal benchmarks: 194 / Regressions (> 25%): 0 / RESULT: PASSCache-hit run (abmcar/DTVM run
25900131271, commit8189d4dis a comment-only no-op bumpinghead.shawhilebase.shastays atc644fbe):Cache restored from key: perf-baseline-lib-<mode>-c644fbe...Build baseline librarystepskipped(perif: cache-hit != 'true'evaluating false)elifbranch)Total benchmarks: 194 / Regressions (> 25%): 0 / RESULT: PASSCross-runner noise reduction (same synth benches that triggered this investigation, observed on the same-runner cache-miss run):
synth/ADDRESS/a1synth/BYTE/b1synth/CALLER/a1synth/ISZERO/u0synth/LT/b1synth/GT/b1snailtracer/benchmark(macro)blake2b_shifts/8415nulls(macro)Only 1 micro-benchmark was filtered by the
baseline < 5 µsgate, versus 25 in the cross-runner cache-stale comparison.Trade-offs
${matrix.mode}and${base.sha}; the renamed key (perf-baseline-lib-...) orphans oldperf-baseline-...entries, which expire under GitHub's 7-day LRU.dtvmdev1/dtvm-dev-x64:mainis not part of the cache key. If the container's compiler version changes, a cached.socould be from a different code-gen profile. This risk already existed with the JSON cache and is unchanged. Pinning the container by digest or adding it to the cache key is a separate hardening, out of scope here.Test plan
python3 -c "import yaml; yaml.safe_load(...)")tools/format.sh checkcleanBuild baseline librarycorrectly skipped