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

Migrate benches to divan #64

Merged
merged 2 commits into from
Sep 7, 2024

Conversation

jakoschiko
Copy link
Contributor

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:

running 11 tests
test tests::u64_get_built_in                     ... bench:      80,418.25 ns/iter (+/- 1,215.98)
test tests::u64_get_indexmap                     ... bench:     104,553.57 ns/iter (+/- 716.26)
test tests::u64_get_intmap                       ... bench:      12,021.77 ns/iter (+/- 246.29)
test tests::u64_insert_built_in                  ... bench:     115,302.30 ns/iter (+/- 2,222.09)
test tests::u64_insert_built_in_without_capacity ... bench:     283,263.70 ns/iter (+/- 957.75)
test tests::u64_insert_indexmap                  ... bench:     131,009.76 ns/iter (+/- 1,660.13)
test tests::u64_insert_intmap                    ... bench:      35,768.73 ns/iter (+/- 894.52)
test tests::u64_insert_intmap_checked            ... bench:      42,419.53 ns/iter (+/- 727.90)
test tests::u64_insert_intmap_entry              ... bench:      43,489.37 ns/iter (+/- 161.42)
test tests::u64_insert_intmap_without_capacity   ... bench:     487,727.00 ns/iter (+/- 5,910.03)
test tests::u64_resize_intmap                    ... bench:      22,185.56 ns/iter (+/- 97.57)

After:

Timer precision: 10 ns
basic_bench                              fastest       │ slowest       │ median        │ mean          │ samples │ iters
├─ u64_get_built_in                      99.89 µs      │ 108.8 µs      │ 102 µs        │ 102.4 µs      │ 100     │ 100
├─ u64_get_indexmap                      106.5 µs      │ 114 µs        │ 107.3 µs      │ 107.7 µs      │ 100     │ 100
├─ u64_get_intmap                        12.2 µs       │ 37.05 µs      │ 12.31 µs      │ 12.99 µs      │ 100     │ 100
├─ u64_insert_built_in                   109.6 µs      │ 149.1 µs      │ 110.4 µs      │ 111.2 µs      │ 100     │ 100
├─ u64_insert_built_in_without_capacity  271 µs        │ 283.4 µs      │ 273.9 µs      │ 274.2 µs      │ 100     │ 100
├─ u64_insert_indexmap                   143.6 µs      │ 150.4 µs      │ 146.5 µs      │ 146.8 µs      │ 100     │ 100
├─ u64_insert_intmap                     27.17 µs      │ 151.6 µs      │ 27.77 µs      │ 29.7 µs       │ 100     │ 100
├─ u64_insert_intmap_checked             30.49 µs      │ 144.7 µs      │ 42.72 µs      │ 43.54 µs      │ 100     │ 100
├─ u64_insert_intmap_entry               42.44 µs      │ 164.1 µs      │ 43.67 µs      │ 45.6 µs       │ 100     │ 100
├─ u64_insert_intmap_without_capacity    482.3 µs      │ 612.3 µs      │ 500.5 µs      │ 523.8 µs      │ 100     │ 100
╰─ u64_resize_intmap                     31.01 µs      │ 36.46 µs      │ 31.06 µs      │ 31.31 µs      │ 100     │ 100

Closes #62

@jakoschiko
Copy link
Contributor Author

Hmm, divan seems to be in conflict with the MSRV. Should I convert the benches to an own crate, similar to integration_tests?

@jakoschiko
Copy link
Contributor Author

I tested criterion. It would also be in conflict with MSRV.

@JesperAxelsson
Copy link
Owner

There is some more benchmarks in the benchmark branch. Getting it in a different crate might be the best options. It's not great having to switch branch just to test changes, but was an easy solution to not raise MSRV.

@JesperAxelsson
Copy link
Owner

Not sure if its possible to have different MSRV on dev and release mode. That would be a better solution otherwise.

@jakoschiko
Copy link
Contributor Author

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:

  • It uses the MSRV toolchain for compiling the lib
  • It uses a newer Rust toolchain for compiling the lib and testing

@JesperAxelsson
Copy link
Owner

Yeah, might be the more practical solution. It should be easy to run benchmarks and test :)

@jakoschiko
Copy link
Contributor Author

It still doesn't work, even though it should work. There are a bunch of Rust issues about this topic. I'll investigate.

@jakoschiko
Copy link
Contributor Author

jakoschiko commented Sep 2, 2024

Okay, as always, it's complicated.

@jakoschiko
Copy link
Contributor Author

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.

@JesperAxelsson
Copy link
Owner

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.

@jakoschiko jakoschiko force-pushed the stable-benches branch 2 times, most recently from f4791df to 5840d56 Compare September 4, 2024 22:37
@jakoschiko
Copy link
Contributor Author

Ok, another attempt.

Now there are two completely unrelated folders:

  1. The root of the repository
    • Contains the lib
    • Contains basic tests
    • Not a workspace anymore
  2. The folder integration_tests
    • Contains more complicated tests (as before)
    • Contains the benchmarks (this is new)
    • Is now a workspace (this is new)

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).

@JesperAxelsson
Copy link
Owner

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!

@JesperAxelsson JesperAxelsson merged commit 4f4af91 into JesperAxelsson:master Sep 7, 2024
2 checks passed
@jakoschiko jakoschiko deleted the stable-benches branch September 17, 2024 20:57
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.

Use benchmark lib that doesn't require nightly Rust
2 participants