-
Notifications
You must be signed in to change notification settings - Fork 159
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
[Rust] Will this work with Criterion? #8
Comments
I'm currently not a user of criterion (since I could not run it on my local macOS). However, as far as seeing your output example, we can extract the benchmark result and put remaining outputs to |
Thanks @rhysd I suppose the range would be the high-value - low-value CC @bheisler |
Hey, Criterion.rs maintainer here. The command-line output isn't really intended for programmatic access so the format isn't guaranteed to stay stable. I don't currently plan to change it, so this is more of a heads-up that you may need to fix it if it changes in the future. Criterion.rs generates a file that is designed for programs rather than humans (see here for more on that) but it contains the raw measurements, not the statistics that Criterion.rs calculates from them, so if you wanted to use that you would need to decide on how you want to summarize those numbers. If you go that direction, I would recommend also reading up on the measurement and analysis process to clarify what some of those numbers mean. Unrelated: @rhysd - if it's not too much trouble, would you mind explaining your difficulties with Criterion.rs on macOS, or perhaps opening an issue on the Criterion.rs repository? I don't have a Mac to test with but it ought to work. |
@bheisler Thank you for the comment. I did not know that it supports machine-readable format. The format would be preferable.
I'm sorry that I cannot remember clearly, but when I tried Criterion example at that time, the benchmark program crashed with SEGV. Since it was about 6 months ago and I used older macOS (10.12) at that time, now it might work fine. |
OK, thanks. If you decide to try it again and run into the same problem, please feel free to file an issue. |
I'm now trying to take over @jasonwilliams's work. I'll notify you if I face an issue. Thank you for taking care. |
I tried to implement simple calculation based on the analysis process document: const { linear } = require('regression');
const fs = require('fs');
const lines = fs.readFileSync('target/criterion/Bench Fib 10/new/raw.csv', 'utf8').split('\n');
const measurements = lines
.slice(1)
.filter(l => l !== '')
.map(l => {
const data = l.split(',');
const measured = parseFloat(data[5]);
const iter = parseInt(data[7], 10);
return [iter, measured];
});
let max = measurements[0][1] / measurements[0][0];
let min = max;
for (const m of measurements.slice(1)) {
const v = m[1] / m[0];
if (v > max) {
max = v;
}
if (v < min) {
min = v;
}
}
console.log('min:', min);
console.log('estimate:', linear(measurements).equation[0]);
console.log('max:', max); and output was:
Criterion.rs's output was
so I think there is a difference in calculation of estimation. And min/max seems wrong. Hmm, I'm not confident to maintain the logic. After all I'm feeling using CLI stdout might be better. Though it may change without major version up of Criterion.rs. CSV format document says
so we might adopt some machine readable output of summary in the future. |
Yeah, a good idea would be to have |
Or criterion could add the calculated info to the machine accessable file. @bheisler What do you think? Or maybe it could support the normal |
Hey folks. I've been thinking about adding better machine-readable output to this I'm thinking it would be broadly similar to Cargo's existing The messages themselves are still TBD, but I do want to keep my options open for the future so I'm thinking that the only field that is guaranteed to be there would be some estimate of a typical value. All of the other stats (mean, variance, MAD, confidence bounds, etc.) would be officially documented as optional so that I still have the option to not report them. For example, I'm imagining an alternate sampling method for long-running benchmarks where we just run the benchmark say 100 times and record each individual time. Those measurements would produce a different set of statistics and I wouldn't want to break downstream code by only sometimes reporting, say, the slope measurements. I'm also thinking I might set it up to parse None of this is a promise, mind - I'm still designing a lot of this stuff and figuring out what makes sense. Would that work for what you folks need? Do you have any comments or concerns with this idea so far? Is there anything in particular you want to have in the message format? |
@bheisler This is already solved with the bencher compat output criterion provides. It is the reason I asked you to add it originally. |
If anyone else is blocked by this/#145, Bencher supports Criterion: https://github.com/bencherdev/bencher#supported-benchmark-harnesses |
It seems that `github-action-benchmark` does not support criterion for the moment, and the suggested alternative Bencher requires payment for private projects and does not seem to work on Windows. See: benchmark-action/github-action-benchmark#8
A lot of rust projects use Criterion which overrides
cargo bench
, that may output something different, would this project work with that?What does the output need to look like?
For e.g im using
https://github.com/jasonwilliams/criterion-compare-action
which is a wrapper around https://github.com/bheisler/criterion.rs
This is the output from criterion on https://github.com/jasonwilliams/boa:
https://bheisler.github.io/criterion.rs/book/user_guide/command_line_output.html
The text was updated successfully, but these errors were encountered: