-
Notifications
You must be signed in to change notification settings - Fork 99
ci(bench): WASM + native perf-regression tracking with sticky PR comments #1002
base: next
Are you sure you want to change the base?
Changes from 7 commits
73b54cd
5c7c32f
f742e76
5fc04e0
120c50b
214b0ac
ae75f95
4718b59
da13e80
6c223b7
08ad7c6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,276 @@ | ||
| # Performance regression tracking for miden-crypto. | ||
| # | ||
| # Two jobs run on every PR + every push to `next`: | ||
| # | ||
| # 1. bench-wasm: headless-Chromium + wasm-pack harness in miden-bench-wasm/ | ||
| # tracks RPO/RPX/Poseidon2/Blake3/Keccak hash perf as | ||
| # compiled to wasm32 + run in V8 — the runtime real users | ||
| # ship in. | ||
| # | ||
| # 2. bench-native: cargo bench --bench {hash,word,transpose} on the GHA | ||
| # runner. Tracks the same primitives at native speed for | ||
| # comparison + regression detection. | ||
| # | ||
| # Each bench job is paired with a trusted `publish-*` job that downloads | ||
| # its artifact and calls `benchmark-action/github-action-benchmark` with | ||
| # the write-capable token. The split is deliberate: the bench jobs run | ||
| # PR-controlled code (Rust, npm install, Node) and therefore MUST NOT | ||
| # hold credentials, so they run with `contents: read` and | ||
| # `persist-credentials: false`. The publish jobs check out the default | ||
| # branch (trusted ref) before running the action, so the PR's tree never | ||
| # reaches a step that has write scope. This is the same split miden-vm's | ||
| # non-regression workflows use. | ||
| # | ||
| # `benchmark-action/github-action-benchmark`: | ||
| # - Stores each metric over time on the `gh-pages` branch under | ||
| # `bench/` (separate from the existing `docs/` subdir used by docs.yml, | ||
| # so the two don't collide). | ||
| # - Posts a sticky PR comment showing the diff vs the latest `next` | ||
| # baseline. Header per job, so wasm and native each get their own | ||
| # comment that updates in place. | ||
| # - Alerts (writes a comment header) on regression > 10 %. Doesn't fail | ||
| # the workflow — GHA Linux runners share CPUs and the noise floor on | ||
| # hash benches is real. False-positive alerts are cheap to ignore; | ||
| # false-negative reverts are expensive. | ||
|
|
||
| name: bench | ||
|
|
||
| on: | ||
| pull_request: | ||
| push: | ||
| branches: [next] | ||
| workflow_dispatch: | ||
|
|
||
| # Default to read-only at workflow scope. Privileged scopes are granted | ||
| # per-job and only on the trusted publish jobs that do NOT execute any | ||
| # PR-controlled code. | ||
| permissions: | ||
| contents: read | ||
|
|
||
| # Skip duplicate runs on PR sync events. | ||
| concurrency: | ||
| group: bench-${{ github.workflow }}-${{ github.head_ref || github.ref }} | ||
| cancel-in-progress: true | ||
|
|
||
| jobs: | ||
| bench-wasm: | ||
| name: WASM perf (Chromium / V8) | ||
| runs-on: ubuntu-latest | ||
| # Local Apple M5 wall-clock for the full bench is ~40s; GHA ubuntu- | ||
| # latest x86_64 is ~2.7× slower across these benches (measured), | ||
| # putting the bench step at ~2 min. Plus wasm-pack build (~3 min) | ||
| # + Chromium install (~30s) + setup overhead = ~6-7 min typical. | ||
| # 20 min cap leaves comfortable headroom for the worst-case run. | ||
| timeout-minutes: 20 | ||
| permissions: | ||
| contents: read | ||
| steps: | ||
| - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # pin@v4 | ||
| with: | ||
| # The default checkout writes the GHA token to `.git/config`, | ||
| # which PR-controlled build scripts (build.rs, npm postinstall, | ||
| # etc.) would then have access to. Strip it — this job has no | ||
| # business holding any credential. | ||
| persist-credentials: false | ||
|
|
||
| - name: Install Rust + wasm32 target | ||
| run: | | ||
| rustup update --no-self-update | ||
| rustup target add wasm32-unknown-unknown | ||
|
|
||
| - name: Install wasm-pack | ||
| # Pinned to v0.4.0 SHA. The action installs wasm-pack v0.13.1. | ||
| uses: jetli/wasm-pack-action@0d096b08b4e5a7de8c28de67e11e945404e9eefa # pin@v0.4.0 | ||
| with: | ||
| version: "v0.13.1" | ||
|
|
||
| - name: Build miden-bench-wasm | ||
| # `--target web` produces an ESM JS file that imports/instantiates | ||
| # the .wasm via `fetch()` — what the static page expects. The | ||
| # `--release` profile is required: without it the per-op cost | ||
| # is dominated by debug-mode overflow checks and the numbers are | ||
| # meaningless as a perf-tracking signal. | ||
| run: | | ||
| wasm-pack build --release --target web \ | ||
| --out-dir static/pkg miden-bench-wasm | ||
|
|
||
| - name: Install Node deps | ||
| working-directory: miden-bench-wasm | ||
| run: npm install --no-audit --no-fund | ||
|
|
||
| - name: Install Chromium | ||
| working-directory: miden-bench-wasm | ||
| # Chromium-only — the action's matrix support is via npx, but we | ||
| # only target one browser for now (V8 fidelity to the wallet). | ||
| run: npx playwright install --with-deps chromium | ||
|
|
||
| - name: Run benches | ||
| id: run | ||
| working-directory: miden-bench-wasm | ||
| run: | | ||
| node driver.mjs > results.json | ||
| echo "results=$(wc -l < results.json) entries written" | ||
| # Surface the per-bench medians in the workflow summary so a | ||
| # human can eyeball them without clicking into the artifact. | ||
| { | ||
| echo '## WASM bench results' | ||
| echo '' | ||
| echo '| bench | median (ns/iter) |' | ||
| echo '|---|---|' | ||
| jq -r '.[] | "| \(.name) | \(.value | tostring) |"' results.json | ||
| } >> "$GITHUB_STEP_SUMMARY" | ||
|
|
||
| - name: Upload raw results as artifact | ||
| # Picked up by the trusted `publish-wasm` job, which runs the | ||
| # benchmark-action against this file with write-scope token. Also | ||
| # keeps the full per-batch sample distribution for post-hoc | ||
| # analysis when an alert fires. | ||
| uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # pin@v4 | ||
| with: | ||
| name: bench-wasm-results | ||
| path: miden-bench-wasm/results.json | ||
| retention-days: 30 | ||
|
|
||
| bench-native: | ||
| name: Native perf (Linux x86_64) | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 20 | ||
| permissions: | ||
| contents: read | ||
| steps: | ||
| - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # pin@v4 | ||
| with: | ||
| persist-credentials: false | ||
|
|
||
| - name: Install Rust | ||
| run: rustup update --no-self-update | ||
|
|
||
| - name: Run benches | ||
| id: run | ||
| # Initial bench triage: | ||
| # - hash: RPO/RPX/Poseidon2/Blake3/Keccak merge + sequential. | ||
| # The headline. Fast (sub-second per group). | ||
| # - word: Felt arithmetic primitives. Foundational, fast. | ||
| # - transpose: Matrix transpose. Foundational, fast. | ||
| # | ||
| # Deliberately excluded for now (revisit after first PR lands): | ||
| # - smt / merkle / partial_mt / store: moderate runtime, not the | ||
| # headline; defer until we know the wasm + tracked-native | ||
| # setup is stable. | ||
| # - large_smt / large_smt_forest / sparse_path: multi-minute | ||
| # runtimes. Wrong fit for per-PR alerts; ideal for a separate | ||
| # nightly bench job. | ||
| # - encryption / dsa: defer pending review of which sub-benches | ||
| # are stable. | ||
| run: | | ||
| cargo bench --bench hash --bench word --bench transpose \ | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we add |
||
| -p miden-crypto -- --output-format bencher 2>&1 \ | ||
| | tee bench-native.txt | ||
|
|
||
| - name: Upload raw results as artifact | ||
|
github-advanced-security[bot] marked this conversation as resolved.
Fixed
|
||
| uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # pin@v4 | ||
| with: | ||
| name: bench-native-results | ||
| path: bench-native.txt | ||
| retention-days: 30 | ||
|
|
||
| publish-wasm: | ||
| name: Publish WASM bench | ||
| needs: bench-wasm | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 10 | ||
| permissions: | ||
| contents: write # github-action-benchmark pushes to gh-pages on `push to next` | ||
| pull-requests: write # for sticky PR comment on pull_request runs | ||
| steps: | ||
| # Explicitly check out the default branch (trusted ref). On a | ||
| # `pull_request` event, the default `actions/checkout` resolves to | ||
| # the PR's merge ref — that's PR-controlled code, which has no | ||
| # business being on disk in a job that holds the write token. | ||
| - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # pin@v4 | ||
| with: | ||
| ref: ${{ github.event.repository.default_branch }} | ||
| # `benchmark-action/github-action-benchmark` reads its credential | ||
| # from the `github-token` input rather than `.git/config`, so the | ||
| # persisted token isn't needed. Strip it to satisfy zizmor's | ||
| # `artipacked` audit. | ||
| persist-credentials: false | ||
|
|
||
| - name: Download bench results | ||
| uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # pin@v4 | ||
| with: | ||
| name: bench-wasm-results | ||
| path: results | ||
|
|
||
| - name: Track + alert on regression | ||
| uses: benchmark-action/github-action-benchmark@52576c92bccf6ac60c8223ec7eb2565637cae9ba # pin@v1.22.1 | ||
| with: | ||
| tool: customSmallerIsBetter | ||
| output-file-path: results/results.json | ||
| # Separate gh-pages subdir so this never collides with docs.yml's | ||
| # `destination_dir: docs` deploy (which lives at /docs/). | ||
| benchmark-data-dir-path: bench/wasm | ||
| gh-pages-branch: gh-pages | ||
| # Push baseline updates ONLY on push to next (not on PR runs — | ||
| # those compare against the existing baseline, they don't update it). | ||
| auto-push: ${{ github.event_name == 'push' }} | ||
| comment-on-alert: true | ||
| # Regression alert at 10 %. Run-to-run noise on GHA shared | ||
| # runners is real but accepting 15-20 % headroom would defeat | ||
| # the purpose: a 15 % regression IS a regression worth | ||
| # investigating, not the noise floor. Drive variance down | ||
| # (longer batches, more samples) to fit under this threshold | ||
| # rather than widening the threshold to fit measured noise. | ||
| # See README "Noise reduction" for the roadmap to tighten further. | ||
| alert-threshold: "110%" | ||
| fail-on-alert: false # warn via comment, don't fail the PR | ||
| summary-always: true # keep the diff comment fresh on each run | ||
| comment-always: ${{ github.event_name == 'pull_request' }} | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could this be gated to same-repo PRs? Fork PRs get a read-only |
||
| alert-comment-cc-users: '@WiktorStarczewski' | ||
| github-token: ${{ secrets.GITHUB_TOKEN }} | ||
|
|
||
| publish-native: | ||
| name: Publish native bench | ||
| needs: bench-native | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could these two publish jobs be serialized before they write |
||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 10 | ||
| permissions: | ||
| contents: write | ||
| pull-requests: write | ||
| steps: | ||
| - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # pin@v4 | ||
| with: | ||
| ref: ${{ github.event.repository.default_branch }} | ||
| # `benchmark-action/github-action-benchmark` reads its credential | ||
| # from the `github-token` input rather than `.git/config`, so the | ||
| # persisted token isn't needed. Strip it to satisfy zizmor's | ||
| # `artipacked` audit. | ||
| persist-credentials: false | ||
|
|
||
| - name: Download bench results | ||
| uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # pin@v4 | ||
| with: | ||
| name: bench-native-results | ||
| path: results | ||
|
|
||
| - name: Track + alert on regression | ||
| uses: benchmark-action/github-action-benchmark@52576c92bccf6ac60c8223ec7eb2565637cae9ba # pin@v1.22.1 | ||
| with: | ||
| tool: cargo | ||
| output-file-path: results/bench-native.txt | ||
| benchmark-data-dir-path: bench/native | ||
| gh-pages-branch: gh-pages | ||
| auto-push: ${{ github.event_name == 'push' }} | ||
| comment-on-alert: true | ||
| # Same 10 % threshold as wasm — native benches are ns-scale and | ||
| # timer noise is proportionally larger, but criterion's own | ||
| # statistical pruning already handles much of the per-iteration | ||
| # variance. If 10 % is too tight in practice, the path forward | ||
| # is more iterations / `iai-callgrind` instruction-count | ||
| # benchmarking, NOT a wider threshold. | ||
| alert-threshold: "110%" | ||
| fail-on-alert: false | ||
| summary-always: true | ||
| comment-always: ${{ github.event_name == 'pull_request' }} | ||
| alert-comment-cc-users: '@WiktorStarczewski' | ||
| github-token: ${{ secrets.GITHUB_TOKEN }} | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Uh oh!
There was an error while loading. Please reload this page.