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

Idea: Cargo Plugin #266

Closed
bheisler opened this issue Feb 2, 2019 · 3 comments
Closed

Idea: Cargo Plugin #266

bheisler opened this issue Feb 2, 2019 · 3 comments
Labels
Bigger Project A large project, probably involving major internal changes as well as new API surface Enhancement Request For Comments Input from users is requested

Comments

@bheisler
Copy link
Owner

bheisler commented Feb 2, 2019

Having the benchmark executable generate the HTML reports is awkward for a number of reasons:

  • All benchmarks need to pull in the code to generate HTML and Gnuplot reports, compile it, etc.
  • Since benchmarks can be in multiple different executables and can be optionally filtered down with command-line parameters, the HTML reports have to be generated piecemeal, which creates redundant work (eg. each benchmark executable overwrites the report index), introduces bugs and makes it harder to add certain features.

It might work better if the criterion crate was trimmed down to just the measurement and analysis code and the CLI output. The benchmarks would write their measurements to disk, which could be read by a cargo criterion command and turned into HTML reports. The core benchmarks would work fine with cargo bench (in fact, I expect that cargo-criterion would just shell out to cargo-bench to execute them), but wouldn't produce reports.

This redesign would also make it easier to add things like historical-performance timelines and the like, and it would allow the report-generation code to evolve independently of the measurement code.

Some downsides:

  • The user would have to cargo install cargo-criterion.
  • Adding custom reports to a cargo subcommand would require a plugin or external executable of some sort, where adding custom reports in-process is easy.

I've had this idea in the back of my mind for a long time, but I'm not sure which direction to go, here.

@bheisler bheisler added Enhancement Bigger Project A large project, probably involving major internal changes as well as new API surface Request For Comments Input from users is requested labels Feb 2, 2019
@ghost ghost mentioned this issue Apr 2, 2019
@anp
Copy link

anp commented Sep 7, 2019

I'd definitely be in favor of this change! Independent evolution of the features sounds great, as do the technical advantages of a smaller core crate with fewer potential side effects in a binary.

The downsides you cite are valid. It seems to me that they are similar in that they increase the "activation energy" of using the library and require buying into larger amounts of complexity for a project's tooling. Does this seem accurate to you?

It sounds like the case where the current setup shines is for projects that use criterion to run benchmarks which generate HTML reports for benchmarks that run in a single crate. The end-developer complexity of multi-binary report generation is low (they just run a single command and cargo takes care of it, serializing side effects in the target directory, right?), although the actual complexity is quite high in terms of the bugs and slow iteration you cite.

Could the single-crate report generation case be satisfied with a cargo feature that enables a dependency on the report generator crate? This would more or less preserve the existing developer experience with the small papercut of having to read about it, add a feature to Cargo.toml, etc.

How important do you think the multi-binary experience is? If multi-binary benchmark suites used the above opt-in feature to write to disjoint HTML reports that are still individually useful, would it be feasible to offer a cargo criterion merge or similar kind of command to produce a single report?

@bheisler
Copy link
Owner Author

The multi-binary case is moderately common, I'd think. It's not obvious, but every different [[bench]] section in the Cargo.toml file becomes a separate binary. This isn't just about supporting people with multiple crates in one workspace. Criterion.rs' example benchmarks were structured in that way for a long time.

It's been a while since I've felt the need for something like this, though, so it's sort of fallen down the priority list. The technical benefits aren't as compelling anymore; the code to generate the reports isn't a major contributor to compile times or binary size, and redundantly generating the summary reports isn't likely to matter compared to the time taken to perform the measurements in the first place.

@bheisler
Copy link
Owner Author

This Cargo plugin now has its own repository: https://github.com/bheisler/cargo-criterion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bigger Project A large project, probably involving major internal changes as well as new API surface Enhancement Request For Comments Input from users is requested
Projects
None yet
Development

No branches or pull requests

2 participants