criterion.rs icon indicating copy to clipboard operation
criterion.rs copied to clipboard

Add optional features

Open bheisler opened this issue 4 years ago • 8 comments

Criterion.rs has a pretty large dependency tree, which can impact compilation time. Mostly this is on clean-compiles, which are relatively rare, but it also increases the amount of code the linker has to process on incremental builds as well. Criterion.rs should provide some Cargo features to cut down the size of the tree. Now is a good time to do this as well, since cargo-criterion will soon have its first alpha release - since that can be compiled and installed once with no impact on the linker, dependencies and code there have a much smaller compile-time cost than they do in Criterion.rs.

Here are some ideas:

  • Now that cargo-criterion is available to handle generating charts, I think the HTML reports should become an optional feature in 0.4, probably disabled by default. All of my new-feature development on the reports will probably happen in cargo-criterion as well, so the HTML reports will probably be deprecated and removed in 0.5.
  • A lot of the dependency tree is needed to do the statistical analysis and save results to disk. Both of these tasks are also handled by cargo-criterion, so this code is only needed when running with cargo bench. We could add a feature that represents cargo bench support. That way, users could disable it if they're using cargo criterion. I think this should be enabled by default though; users would be surprised if they can't use Criterion.rs with cargo bench out of the box.
  • When I added support for regex-based benchmark filtering, I turned off all of the nonessential features of the regex crate. Surprisingly, it still takes up quite a bit of compile time, so allowing regexes as filters could be a feature as well. I think it should be enabled by default though.

Disabling them all would leave Criterion.rs with just the code necessary to perform measurements and send them to cargo-criterion for analysis and reporting.

The last release in the 0.3.* series will expose these features, but it will just use them to enable warnings about any behavior that might change in 0.4.

bheisler avatar Jun 29 '20 15:06 bheisler

While I'm at it, the CSV output should probably also become a non-default feature, slated for deletion. I'm not aware of anybody ever using it, and it's probably superseded for most real use cases by cargo-criterion's --message-format=json option.

bheisler avatar Jul 05 '20 01:07 bheisler

Didn't mean to close this.

bheisler avatar Jan 24 '21 18:01 bheisler

For people that don't want to use this feature you print an annoying big warning for each test. Consider have a opt-out.

Stargateur avatar Mar 23 '21 15:03 Stargateur

Now that cargo-criterion is available to handle generating charts, I think the HTML reports should become an optional feature in 0.4, probably disabled by default. All of my new-feature development on the reports will probably happen in cargo-criterion as well, so the HTML reports will probably be deprecated and removed in 0.5.

Would disabling the HTML reports also disable plot generation as well? I don't use these plots and I would love to be able to prune the dependency tree/remove the need for gnuplot.

Luthaf avatar May 28 '21 09:05 Luthaf

My use case for wanting to disable as much is possible is that I run cargo test --benches in CI just to make sure my benchmarks compile and run successfully. I don't need the plotting, HTML, saving results to disk, etc.

I see other people have contributed PRs to make a lot of this stuff optional already, and they were closed because they are breaking changes. I want to voice my support for moving on from 0.3.x to 0.4.x so that we make the functionality optional, and I would be happy to offer assistance.

briansmith avatar Jul 06 '21 23:07 briansmith

My use case for wanting to disable as much is possible is that I run cargo test --benches in CI just to make sure my benchmarks compile and run successfully. I don't need the plotting, HTML, saving results to disk, etc.

I see other people have contributed PRs to make a lot of this stuff optional already, and they were closed because they are breaking changes. I want to voice my support for moving on from 0.3.x to 0.4.x so that we make the functionality optional, and I would be happy to offer assistance.

I've created a new branch for version-0.4. Assistance and testing would be much appreciated. Discussion about what should be included in 0.4 and what should be pushed to 0.5 would also be a big help.

lemmih avatar Jul 27 '21 07:07 lemmih

We can probably switch out regex for regex-lite and save a bunch of compilation time

emilk avatar Dec 11 '23 20:12 emilk

I would love a smaller dependency tree also. But mostly for a very different reason: Supply chain security. It had not been mentioned in this thread, so I figured I better add this aspect also. From this point of view it becomes more important to look at things like this when selecting dependencies:

  • Are the authors reputable/trustworthy?
  • How do they respond to issues found in their crates?
  • Do they have a good CI?
  • How securely do they seem to handle their git and crates.io access?

faern avatar May 19 '24 08:05 faern