Skip to content

ci: run zebra-consensus benches on zebra-touching commits - #48

Open
aphelionz wants to merge 2 commits into
mainfrom
claude/zebra-bench-ci-2a26d1
Open

ci: run zebra-consensus benches on zebra-touching commits#48
aphelionz wants to merge 2 commits into
mainfrom
claude/zebra-bench-ci-2a26d1

Conversation

@aphelionz

@aphelionz aphelionz commented Aug 25, 2026

Copy link
Copy Markdown
Member

Runs the zebra-consensus criterion benches on every zebra-touching commit (PRs and main pushes), posting the timings to the job's step summary. Informational, not a required check: absolute times on shared runners are noisy, but the ratios the benches watch (cache miss vs hit in #45, serial vs overlapped lookups in #46) are measured within one run on one box.

Follows the z3-regtest conventions: no paths: trigger filter, a Path gate job diffing the merge commit (manual dispatches bypass it), Swatinem rust-cache on the zebra workspace, saved from main only so per-PR saves cannot evict the smoke/regtest caches. Each bench step is guarded on its bench file existing, so this can merge before #45/#46 and activates as they land.

🤖 Generated with Claude Code

Copilot AI lite review requested due to automatic review settings August 25, 2026 13:45

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an informational CI workflow to run Zebra consensus Criterion benchmarks on relevant PRs and main pushes.

Changes:

  • Adds path-gated and manual workflow triggers.
  • Runs available consensus benchmarks with Rust caching.
  • Publishes benchmark results to the step summary.
Suppressed comments (5)

.github/workflows/zebra-bench.yml:67

  • workflow_dispatch is filtered through the same path decision, so manually running this workflow on a commit whose immediate parent changed only documentation silently skips both benches. Since this trigger has no force-run input, special-case manual dispatches as relevant (as the other job-gated workflows do) or remove the trigger.
          if [ "$hits" -gt 0 ]; then relevant=true; else relevant=false; fi

.github/workflows/zebra-bench.yml:63

  • On a push that contains multiple commits, HEAD^1..HEAD only examines the tip commit. For example, a zebra change followed by a docs-only commit makes hits=0 and skips the benches, so a zebra-touching commit is missed. Use the push event's before SHA (fetching it or using full history), while retaining the merge-parent range for pull requests.
          git diff --name-only HEAD^1 HEAD -- "${specs[@]}" > "$RUNNER_TEMP/matched"

.github/workflows/zebra-bench.yml:98

  • This rust-cache setup does not provide a rolling Criterion baseline: for source-only commits with the same toolchain/job/lockfile, the cache key remains the same, and GitHub cache entries are immutable, so the post-run target/criterion data cannot replace the first cached entry. Later runs therefore compare against stale data rather than the immediately preceding run as the workflow comment claims. Store Criterion data with a per-run key plus a ref-scoped restore key, or otherwise provide an updateable baseline separately from the build cache.
        uses: Swatinem/rust-cache@v2
        with:
          prefix-key: v1-bench
          workspaces: |
            zebra -> target

.github/workflows/zebra-bench.yml:108

  • These benchmark commands do not use --locked, so a manifest/lockfile mismatch can make Cargo resolve newer compatible dependencies and rewrite the lockfile during the run. The reported timing can then depend on registry state and represent a dependency graph different from the commit; fail loudly with --locked instead.
          cargo bench -p zebra-consensus --bench script 2>&1 | tee "$RUNNER_TEMP/script.out"

.github/workflows/zebra-bench.yml:124

  • This benchmark command has the same reproducibility problem as the script bench: without --locked, a manifest/lockfile mismatch can resolve newer compatible dependencies during the run, making the timing depend on registry state. Pass --locked so the benchmark either uses the committed dependency graph or fails.
          cargo bench -p zebra-consensus --bench utxo_lookup 2>&1 | tee "$RUNNER_TEMP/utxo.out"

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

':(glob)zebra/zebra-chain/**'
':(glob)zebra/zebra-script/**'
':(glob)zebra/Cargo.toml'
':(glob)zebra/Cargo.lock'
Path-gated informational job (not a required check): runs the script-cache
and UTXO-lookup criterion benches on PRs and main pushes that touch the
benched crates, and posts the timings to the step summary. Bench steps are
guarded on their bench files existing, so this lands before #45/#46 and
activates as they merge. rust-cache persists target/criterion, so criterion
also reports a change estimate against the previous cached run.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 26, 2026 13:29
@aphelionz
aphelionz force-pushed the claude/zebra-bench-ci-2a26d1 branch from 8737529 to 4e5306f Compare August 26, 2026 13:29

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment thread .github/workflows/zebra-bench.yml Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 26, 2026 13:33

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants