-
Notifications
You must be signed in to change notification settings - Fork 14
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
Migrate benches to divan #64
Migrate benches to divan #64
Conversation
Hmm, divan seems to be in conflict with the MSRV. Should I convert the benches to an own crate, similar to |
I tested criterion. It would also be in conflict with MSRV. |
7555dd4
to
c7df6db
Compare
There is some more benchmarks in the |
Not sure if its possible to have different MSRV on dev and release mode. That would be a better solution otherwise. |
We could change the CI like this:
|
Yeah, might be the more practical solution. It should be easy to run benchmarks and test :) |
c7df6db
to
98e8398
Compare
It still doesn't work, even though it should work. There are a bunch of Rust issues about this topic. I'll investigate. |
Okay, as always, it's complicated.
|
Okay, moving it into a separate crate doesn't work either as long as it's part of the same workspace. I think I need to stop for now before I get an existential crisis. |
Hehe, yeah I feel the pain. I can't understand why tooling is always so complicated. |
f4791df
to
5840d56
Compare
5840d56
to
ed7d0e1
Compare
Ok, another attempt. Now there are two completely unrelated folders:
So if you want to test everything, you can do: cargo test
cd integration_tests
cargo test If you want to run all benchmarks, you can do: cd integration_tests
cargo bench Because we are using a separate workspace that is not related to the main lib, everything works. From now on tests and benchmarks can use new and shiny features without compromises. Is this a feasible solution for you? Can you also check if you're happy with the stability of the divan benchmarks? They run really fast, but are slightly less reliable (I think this is by design). |
The consistency seems better then before. I could some really strange measurements where one benchmark doubled in time between runs. It might be possible to increase the iteration count later if we feel the need. It does feel nice to have all the benchmarks and integration test separate. You can add packages without having to consider the main package. Great job! |
This PR migrates the current benches from unstable Rust to divan. Nothing else. If you are happy with this, then I'll create another PR that adds more benchmarks using divan's
args feature
.You can run the benchmarks with
cargo bench
.Before:
After:
Closes #62