Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can't save baselines when benchmarking #17200

Closed
hymm opened this issue Jan 6, 2025 · 1 comment · Fixed by #17222
Closed

Can't save baselines when benchmarking #17200

hymm opened this issue Jan 6, 2025 · 1 comment · Fixed by #17222
Labels
C-Benchmarks Stress tests and benchmarks used to measure how fast things are C-Bug An unexpected or incorrect behavior D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!

Comments

@hymm
Copy link
Contributor

hymm commented Jan 6, 2025

Bevy version

main branch 6f68776

What you did

Trying to save baselines to use with critcmp when doing benchmarking.

cargo bench -- --save-baseline before

What went wrong

So initially this errors with "Unrecognized Option". But per https://bheisler.github.io/criterion.rs/book/faq.html#cargo-bench-gives-unrecognized-option-errors-for-valid-command-line-options I added

[lib]
bench = false

to the Cargo.toml, which allows the benches to run, but it still doesn't actually save the baseline and my target/criterion folder is empty.

Additional information

bisected to c03e494 @BD103

@hymm hymm added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Jan 6, 2025
@alice-i-cecile alice-i-cecile added C-Benchmarks Stress tests and benchmarks used to measure how fast things are and removed S-Needs-Triage This issue needs to be labelled labels Jan 6, 2025
@BD103
Copy link
Member

BD103 commented Jan 7, 2025

This is definitely because I added lib.rs in #16986. I'll open a fix today!

@BenjaminBrienen BenjaminBrienen added S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! D-Straightforward Simple bug fixes and API improvements, docs, test and examples labels Jan 7, 2025
github-merge-queue bot pushed a commit that referenced this issue Jan 8, 2025
…ts in benchmarks (#17222)

# Objective

- Commands like `cargo bench -- --save-baseline before` do not work
because the default `libtest` is intercepting Criterion-specific CLI
arguments.
- Fixes #17200.

## Solution

- Disable the default `libtest` benchmark harness for the library crate,
as per [the Criterion
book](https://bheisler.github.io/criterion.rs/book/faq.html#cargo-bench-gives-unrecognized-option-errors-for-valid-command-line-options).

## Testing

- `cargo bench -p benches -- --save-baseline before`
- You don't need to run the entire benchmarks, just make sure that they
start without any errors. :)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Benchmarks Stress tests and benchmarks used to measure how fast things are C-Bug An unexpected or incorrect behavior D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants