Fixes #27

I added this behind a command line flag, but if you think it would be appropriate, I could make it enabled by default and make the flag disable it. I left the warning in, but only printed it if the user did not enable missing benchmarks.

Below is how it looks. I fed in some of my own benchmark results. I couldn't get the integration tests to pass on my machine, so I'm going to wait for the Travis output and see what adjustments I need to make.

Let me know if you have any feedback!

$ cat old
running 8 tests
test b01_compile_trivial   ... bench:       2,132 ns/iter (+/- 673)
test b02_compile_large     ... bench:   1,594,916 ns/iter (+/- 78,139)
test b03_compile_huge      ... bench:  51,480,362 ns/iter (+/- 1,144,024)
test b04_compile_simple    ... bench:      31,399 ns/iter (+/- 868)
test b05_compile_slow      ... bench:     225,000 ns/iter (+/- 5,625)
test b06_interpret_trivial ... bench:      10,325 ns/iter (+/- 221)
test b07_interpret_simple  ... bench:   6,740,466 ns/iter (+/- 81,711)
test b08_interpret_slow    ... ignored

test result: ok. 0 passed; 0 failed; 1 ignored; 7 measured

     Running target/release/deps/brainfuck-ca6ac36e58b54315

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured

     Running target/release/deps/brainfuck-184a47abc46cc402

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured

$ cat new
running 16 tests
test b01_compile_trivial       ... bench:       2,829 ns/iter (+/- 279)
test b01_compile_trivial_opt   ... bench:       1,746 ns/iter (+/- 55)
test b02_compile_large         ... bench:   2,516,662 ns/iter (+/- 392,982)
test b02_compile_large_opt     ... bench:   1,039,463 ns/iter (+/- 71,707)
test b03_compile_huge          ... bench:  86,575,722 ns/iter (+/- 3,605,019)
test b03_compile_huge_opt      ... bench:  15,571,740 ns/iter (+/- 1,062,383)
test b04_compile_simple        ... bench:      44,247 ns/iter (+/- 490)
test b04_compile_simple_opt    ... bench:      24,441 ns/iter (+/- 255)
test b05_compile_slow          ... bench:     289,571 ns/iter (+/- 3,150)
test b05_compile_slow_opt      ... bench:     158,586 ns/iter (+/- 2,301)
test b06_interpret_trivial     ... bench:      10,442 ns/iter (+/- 46)
test b06_interpret_trivial_opt ... bench:       5,634 ns/iter (+/- 19)
test b07_interpret_simple      ... bench:   7,388,517 ns/iter (+/- 34,084)
test b07_interpret_simple_opt  ... bench:   4,114,281 ns/iter (+/- 18,730)
test b08_interpret_slow        ... ignored
test b08_interpret_slow_opt    ... ignored

test result: ok. 0 passed; 0 failed; 2 ignored; 14 measured

     Running target/release/deps/brainfuck-ca6ac36e58b54315

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured

     Running target/release/deps/brainfuck-184a47abc46cc402

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured

$ cargo run -- benchcmp old new --include-missing
    Finished dev [unoptimized + debuginfo] target(s) in 0.0 secs
     Running `target/debug/cargo-benchcmp benchcmp old new --include-missing`
 name                       old ns/iter  new ns/iter  diff ns/iter  diff % 
 b01_compile_trivial        2,132        2,829                 697  32.69% 
 b02_compile_large          1,594,916    2,516,662         921,746  57.79% 
 b03_compile_huge           51,480,362   86,575,722     35,095,360  68.17% 
 b04_compile_simple         31,399       44,247             12,848  40.92% 
 b05_compile_slow           225,000      289,571            64,571  28.70% 
 b06_interpret_trivial      10,325       10,442                117   1.13% 
 b07_interpret_simple       6,740,466    7,388,517         648,051   9.61% 
 b01_compile_trivial_opt    n/a          1,746                 n/a     n/a 
 b02_compile_large_opt      n/a          1,039,463             n/a     n/a 
 b03_compile_huge_opt       n/a          15,571,740            n/a     n/a 
 b04_compile_simple_opt     n/a          24,441                n/a     n/a 
 b05_compile_slow_opt       n/a          158,586               n/a     n/a 
 b06_interpret_trivial_opt  n/a          5,634                 n/a     n/a 
 b07_interpret_simple_opt   n/a          4,114,281             n/a     n/a

$ cargo run -- benchcmp new old --include-missing
   Compiling cargo-benchcmp v0.1.5 (file:///home/sunjay/Documents/projects/cargo-benchcmp)
    Finished dev [unoptimized + debuginfo] target(s) in 3.9 secs
     Running `target/debug/cargo-benchcmp benchcmp new old --include-missing`
 name                       new ns/iter  old ns/iter  diff ns/iter   diff % 
 b01_compile_trivial        2,829        2,132                -697  -24.64% 
 b02_compile_large          2,516,662    1,594,916        -921,746  -36.63% 
 b03_compile_huge           86,575,722   51,480,362    -35,095,360  -40.54% 
 b04_compile_simple         44,247       31,399            -12,848  -29.04% 
 b05_compile_slow           289,571      225,000           -64,571  -22.30% 
 b06_interpret_trivial      10,442       10,325               -117   -1.12% 
 b07_interpret_simple       7,388,517    6,740,466        -648,051   -8.77% 
 b01_compile_trivial_opt    1,746        n/a                   n/a      n/a 
 b02_compile_large_opt      1,039,463    n/a                   n/a      n/a 
 b03_compile_huge_opt       15,571,740   n/a                   n/a      n/a 
 b04_compile_simple_opt     24,441       n/a                   n/a      n/a 
 b05_compile_slow_opt       158,586      n/a                   n/a      n/a 
 b06_interpret_trivial_opt  5,634        n/a                   n/a      n/a 
 b07_interpret_simple_opt   4,114,281    n/a                   n/a      n/a 

I wasn't getting any color with TERM=tmux-256color, neither automatic nor forced always, so I think it didn't understand that terminal. I do get automatic color with prettytable-rs 0.8.

Writing tests as described in #3. Going straight for the bonus points: so far I've written quickcheck tests for a number of unit test situations.

TODO list

Unit tests

• [X] Test the benchmark parser, including the fact that throughput is optional.
• [X] Test that find overlap finds the correct pairs.
• [X] Test that find overlap reports correct missing benchmarks (for both old and new).
• [x] Test that the name determination for the two columns works correctly when given two file paths. (This may require refactoring the names method off of the Args struct.)
• [X] Test commafy.
• [x] Test that the name prefix feature works. (Refactor code to get a unit test.)

Integration tests

• [x] Test some basic example inputs and check that it produces the expected output.
• [x] Test that passing input on stdin works.

I forked your project and ported it to Rust as an exercise. I don't consider that an improvement, since you now need to compile the tool before you can use it. So I didn't open a PR. Besides, it's in an orphan branch because it doesn't really relate strongly to the Python stuff.

But then I had an idea to add to the tool and I did it to the Rust version because I'm more familiar with that code. Are you interested in pulling that code back into this repo? I prefer contributing/collaborating, even though it's a small tool.

Currently, comparisons are always color coded: any improvement is shown in green and any regression is shown in red. It doesn't matter if the change is 0.78% or if it's 7.8%, the output looks the same.

This change introduces a 3% cut-off so that comparison where the change is smaller than 3% are shown without color. The value of 3% is pretty arbitrary and should perhaps be set lower.

The cut-off is not connected with the --threshold option, so it's possible to ask for a threshold of 1% and still see uncolored lines in the output. That may or may not be a feature :-)

As a side-effect of switching from tabwriter to prettytable-rs, we can now do coloured terminal output. It's a small change to get red lines for regressions and green lines for improvements. It almost makes the --regressions/--improvements flags superfluous :)

I can implement this feature quite easily (I did it before but I can't find the code just now), I may be able to squeeze it into a break or do it right after work. My modem is broken so I'm currently relying on internet access at work 😐

It would be great if there were a complete example that shows how to generate the "old" and "new" files that get passed to cargo benchcmp old new.

I feel like a dummy because none of

$ cargo bench > old
$ cargo bench > new
$ cargo benchcmp old new

nor

$ cargo bench &> old
$ cargo bench &> new
$ cargo benchcmp old new

nor

$ cargo benchcmp ./target/release/bench-12345 ./target/release/bench-67890

are working for me. (Obviously, in a non-toy example, I'd make changes between generating old and new benchmark results).

They all spit back:

Invalid arguments.

Usage:
    cargo benchcmp [options] <old> <new>
    cargo benchcmp [options] <old> <new> <file>
    cargo benchcmp -h | --help
    cargo benchcmp --version

What am I doing wrong here? I'd be happy to submit a PR updating the docs with a complete example once I can figure out how to run cargo benchcmp properly.

Hi there,

Thanks for writing this cool tool :) I'm new to Rust, so please excuse any stupid questions.

I'm trying to use benchcmp with my lib, Frunk, using the following:

$ rustup run nightly cargo bench > benches-control && rustup run nightly cargo bench > benches-variable
#.. then 
$ cargo benchcmp benches-control benches-variable

I understand that benchmarking on the same branch will yield zero (or close to) differences, and my intention is to use benchcmp to compare performance before and after applying a PR, so I'll switch branches in between during actual usage.

Unfortunately the second command yields no output and I'm not sure why ? My benches are here, located in a separate directory.

Thanks in advance for your help,

Lloyd

I just started with implementing #7. Right now it only does automatic detection and colouring, and I made the header row bold to make it more distinctive.

Still needs the flag, so don't merge yet.

Ok, I've rewritten this issue because the original had petty complaints about things I should have expected anyway.

Are you interested in sharing project ownership with me? I have more things I'd love to contribute to the tool to make it even more useful. And I think I can learn a lot from you about Rust. I'm also willing to look into the other issues that are open right now. But I'd like to have a clear sense of the roles and the way we work together.

I implemented --junit <path>. This will create junit-format xml from benchmark comparisons.

JUnit XML Format: https://www.ibm.com/support/knowledgecenter/en/SSUFAU_1.0.0/com.ibm.rsar.analysis.codereview.cobol.doc/topics/cac_useresults_junit.html

Thanks for this utility!

I think it would be nice to indicate differences inside the control error somehow. Perhaps colour them orange/warning-colour instead of red when a subsequent bench is slower than the control but inside the error.

For example a control

test benchmark_a  ... bench:  24,000,000 ns/iter (+/- 500,000)
test benchmark_b  ... bench:  24,000,000 ns/iter (+/- 5,000)

...and the latest bench

test benchmark_a  ... bench:  24,150,000 ns/iter (+/- 480,000)
test benchmark_b  ... bench:  24,150,000 ns/iter (+/- 5,500)

Will produce this output:

 name         control ns/iter   latest ns/iter     diff ns/iter  diff %  speedup 
 benchmark_a  24,000,000        24,150,000             150,000   0.62%   x 0.99 // colour orange
 benchmark_b  24,000,000        24,150,000             150,000   0.62%   x 0.99 // colour red

benchmark_a will be a warning as the diff is inside the control error. benchmark_b will be red as before as the diff is outside the error.

Perhaps you could also allow an option, similar to --threshold, to ignore warnings/ diffs inside errors.

One windows, piping the benchmarks like cargo bench > old will save the results in UTF-16. Running cargo benchcmp will then fail with "stream did not contain valid UTF-8".

One relatively simple way to paper over variability of benchmarks (for example cpu warmup-related things) is to pick the best time of multiple runs. The tool could allow having multiple input files for both before and after.

I saw another crate use a badge for test coverage using a service called Coveralls. I have no experience with them, but it looks nice. Might be a good idea when #3 is resolved?

This is the big feature I've been working on. I would link you to the branch, but my modem is broken and I forgot to bring the code/laptop to work. Which also means that I cannot currently show you pretty pictures to help make the case for this feature.

What I can do is give a quick overview of the design decisions and effects of this feature.

Effects

• Generate images (a bar chart with error bars) 📊
• Extra code to maintain 🐞
• Dependency on gnuplot for the plotting feature 😕 (couldn't find a pure Rust lib, so I'm piping to whatever is called gnuplot on the PATH)

Design decisions

• Currently produces one plot per test name since different benches can have very different values. But maybe a big plot is also nice? Do we give that as an option then, or do we always generate it?
• Where to put the plots. Right now it defaults to target/benchcmp. Should it be configurable? What about the issue where you call benchcmp from a subdirectory of your project? AFAIK we can't get the project root from an environment variable.. Do we go dirty and poll the directory structure to find the Cargo.toml file?
• Command line interface: my original plan was to put the original stuff under a subcommand table and this new stuff under subcommand plot, but now I'm thinking perhaps it's better to provide a separate cargo-benchplot executable?