Skip to content
This repository has been archived by the owner on Mar 26, 2024. It is now read-only.

Add criterion integration for benchmarks #145

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

elasticdog
Copy link
Member

There is a bit of ugliness to allow passing arguments through to criterion without issues, in that we must explicitly put bench = false in all of the Cargo.toml files for the crates in the workspace. It's something that is being looked at upstream.

See:

Checklist

  • Ran cargo xtask fixup for formatting and linting
  • Modified all relevant documentation
  • Updated CHANGELOG.md per "Keep a Changelog" format
  • Added tests covering any code changes

There is a bit of ugliness to allow passing arguments through to
criterion without issues, in that we must explicitly put `bench = false`
in all of the Cargo.toml files for the crates in the workspace. It's
something that is being looked at upstream.

See:
- https://github.com/bheisler/criterion.rs
- https://bheisler.github.io/criterion.rs/book/faq.html#cargo-bench-gives-unrecognized-option-errors-for-valid-command-line-options
- rust-lang/rust#47241
@elasticdog elasticdog enabled auto-merge May 22, 2023 23:46
@elasticdog
Copy link
Member Author

Looks like we're hitting a known error when Nextest uses --all-targets and tries to run the criterion-based benchmarks. The issue has been fixed upstream, but not yet released. Maybe I'll sit on this PR for a bit, as it seems like things are close.

bheisler/criterion.rs#602

@elasticdog
Copy link
Member Author

What do you know, they released 0.5.0 the next day. I'll circle back to this in a bit, but with how many things criterion pulls in, it should be an optional dependency hidden behind a feature flag.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant