Convert to Criterion Benching - #180
michaelciraci wants to merge 17 commits into
Conversation
| pub fn fast() -> Criterion { | ||
| Criterion::default() | ||
| .warm_up_time(Duration::from_millis(100)) | ||
| .measurement_time(Duration::from_millis(100)) |
There was a problem hiding this comment.
I know you want quick benchmarks. 100 ms is fairly small to me but I think we could go higher for slightly more accurate benchmarks
There was a problem hiding this comment.
The old ones I think were on the order of 1s per benchmark, so we definitely have room to increase these.
There was a problem hiding this comment.
I ended up committing the Cargo.lock file because without it, the MSRV tests keep giving errors that criterion dependencies are too old for 1.77. Alternatively we could probably add a couple lines to the pipeline to force a specific version for 1.77, but I think just using a Cargo.lock file to pick exact versions would be more maintainable than a list of commands for specific versions in the dependency tree.
There was a problem hiding this comment.
Another option is to pin those problematic crates to working versions in the cargo.toml file - that's what we did before raising the msrv. It lets us lock versions of just the crates that are a problem, while leaving the rest untouched.
There was a problem hiding this comment.
Ah, I see you already did that with getrandom. If there are too many problems with criterion on 1.77, we could also just increase the msrv. What's the earliest rust version that would work?
There was a problem hiding this comment.
I feel like we shouldn't increase the MSRV just for a test dependency. I think the worst-case scenario is we make a separate benchmarking crate in this repo and just cd into that to run benchmarks.
There was a problem hiding this comment.
Agree with just bumping the MSRV here. A low floor is nice when it's free to maintain, but its value is limited, and once a dev only dependency like criterion starts forcing pins or extra structure to keep it, that value doesn't justify the cost anymore.
| 3, 9, 27, 81, 243, 729, 2187, 6561, 19683, 59049, 177147, 531441, 1594323, 4782969, | ||
| ]; | ||
| for &len in POWERS_OF_THREE { | ||
| c.bench_function(&format!("bench_power3_planned_scalar_f32_{len:07}"), |b| { |
There was a problem hiding this comment.
Is there a way with this system of isolating individual benchmarks, or groups? For example, right now I could run cargo bench bench_power3_planned_scalar_f64 and it would isolate just the power3 f64 benchmarks. No need to run the f32 ones, or the power2 ones etc, if i'm specifically working on a power3 feature.
There was a problem hiding this comment.
You can still apply a filter. For that for example, you could do:
cargo bench --bench bench_check_scalar_2to1024 -- bench_power3_planned_scalar_f64
|
When anyway working on this, should we also update the names? For example |
Done |
3 participants
Closes #159