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

Fix rust_benchmark rule #532

Closed
wants to merge 2 commits into from
Closed

Conversation

djmarcin
Copy link
Contributor

I was just looking into fixing #231, but it doesn't seem like the current setup is well suited to criterion benchmarks (which I think has become the preferred method?). Specifically, when delegating to _rust_test_common, the criterion code doesn't seem to run at all because criterion wants to disable the test harness in favor of its own main.

Since this rule doesn't currently work, it's unlikely anyone depends on it. I propose we repurpose rust_benchmark to simply warn when the compilation mode != "opt" and delegate to a rust_binary with --bench which is more in line with how criterion expects to be invoked.

INFO: Build completed successfully, 60 total actions
Gnuplot not found, using plotters backend
fib 20                  time:   [36.696 us 37.153 us 37.602 us]
Found 2 outliers among 100 measurements (2.00%)
  1 (1.00%) high mild
  1 (1.00%) high severe

This strategy can optionally be made to work for the nightly #[bench] attribute by passing the --test flag to rustc to generate the test harness. This is enabled with the test_harness = True option.

A test was added using criterion to prevent future bit-rot.

@google-cla google-cla bot added the cla: yes label Dec 12, 2020
Base automatically changed from master to main January 28, 2021 20:47
@UebelAndre
Copy link
Collaborator

Hey, sorry about the long delay here. I'd be happy to take a look if you had the time to rebase 😄

@UebelAndre
Copy link
Collaborator

The changes here were merged in #923, thanks again!

@UebelAndre UebelAndre closed this Sep 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants