Skip to content

feat: migrate bench tools to criterion harness#488

Merged
moudyellaz merged 6 commits into
mainfrom
moudy/bench-criterion
May 21, 2026
Merged

feat: migrate bench tools to criterion harness#488
moudyellaz merged 6 commits into
mainfrom
moudy/bench-criterion

Conversation

@moudyellaz
Copy link
Copy Markdown
Collaborator

@moudyellaz moudyellaz commented May 19, 2026

🎯 Purpose

Migrate the two microbenchmark tools (crypto_primitives_bench and cycle_bench's verify path) from hand-rolled Instant::now() timing loops to the criterion harness. Gives statistical CIs, outlier flagging, and a richer comparison story for future PR gating.

⚙️ Approach

crypto_primitives_bench: replaced the custom timing loop with a criterion bench under tools/crypto_primitives_bench/benches/primitives.rs. The binary entry point is gone (no src/), the crate is now bench-only with harness = false.

cycle_bench: split into a library + binary so benches can reuse the helpers. The hand-rolled --verify CLI flag, --verify-iters argument, and run_verify(iters) helper are removed. A new criterion target at tools/cycle_bench/benches/verify.rs reuses prove_auth_transfer_in_ppe (extracted from the binary into the new cycle_bench library) to set up one PPE receipt outside the timed loop, then samples Receipt::verify(PRIVACY_PRESERVING_CIRCUIT_ID) under criterion's statistical sampler (sample_size 100, warm_up 2 s, measurement 15 s, noise_threshold 5%). Also aligns the auth_transfer call with the current authenticated_transfer_core::Instruction::Transfer { amount } enum API.

  • Migrate crypto_primitives_bench to criterion (replace bin with bench target).
  • Refactor cycle_bench into lib + binary; drop --verify CLI flag and run_verify helper.
  • Add criterion verify bench for G_verify at tools/cycle_bench/benches/verify.rs.
  • Add Justfile recipe just bench that runs both criterion targets.
  • Update docs/benchmarks/cycle_bench.md and tools/cycle_bench/README.md.

🧪 How to Test

# Run the criterion benches locally:
cargo bench -p crypto_primitives_bench --bench primitives
cargo bench -p cycle_bench --features ppe --bench verify
# Or both via the Justfile recipe:
just bench

Criterion output lands under target/criterion/<group>/<bench>/. The cycle_bench verify bench runs one full PPE prove for setup (minutes) before the timed loop, so expect a long warmup on first run.

🔗 Dependencies

None.

🔜 Future Work

Add a bench-regression GitHub workflow that uses boa-dev/criterion-compare-action to gate crypto_primitives_bench regressions on PRs. The workflow needs the base branch to already have the criterion bench, so it ships as a follow-up after this PR merges.

📋 PR Completion Checklist

  • Complete PR description
  • Implement the core functionality
  • Add/update tests
  • Add/update documentation and inline comments

@moudyellaz moudyellaz force-pushed the moudy/bench-criterion branch from ad0327e to 36a2cd1 Compare May 20, 2026 15:55
@moudyellaz moudyellaz marked this pull request as ready for review May 20, 2026 19:30
@moudyellaz moudyellaz requested review from Arjentix, Pravdyvy and schouhy and removed request for Arjentix May 20, 2026 19:31
Copy link
Copy Markdown
Collaborator

@Pravdyvy Pravdyvy left a comment

Choose a reason for hiding this comment

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

lgtm

@moudyellaz moudyellaz force-pushed the moudy/bench-criterion branch from 9e5d9a0 to 459da07 Compare May 21, 2026 08:30
@moudyellaz moudyellaz changed the title feat: migrate bench tools to criterion harness + add bench-regression workflow feat: migrate bench tools to criterion harness May 21, 2026
Copy link
Copy Markdown
Collaborator

@Arjentix Arjentix left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, I like new approach. My main question is why not everything was moved to criterion? Also I can see that integration benches where not touched

Comment thread Justfile
Comment thread tools/crypto_primitives_bench/Cargo.toml Outdated
Comment thread tools/crypto_primitives_bench/benches/primitives.rs Outdated
Comment thread tools/cycle_bench/README.md
Copy link
Copy Markdown
Collaborator

@Arjentix Arjentix left a comment

Choose a reason for hiding this comment

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

Cool. Maybe also rename tools to benches?

@moudyellaz moudyellaz force-pushed the moudy/bench-criterion branch from 049e08b to fdec527 Compare May 21, 2026 14:45
@moudyellaz moudyellaz merged commit 5543e12 into main May 21, 2026
13 of 18 checks passed
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.

3 participants