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

Include complete example in documentation #17

Closed
fitzgen opened this issue Oct 2, 2016 · 6 comments
Closed

Include complete example in documentation #17

fitzgen opened this issue Oct 2, 2016 · 6 comments

Comments

@fitzgen
Copy link
Contributor

fitzgen commented Oct 2, 2016

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.

@BurntSushi
Copy link
Owner

Your first example should work:

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

Indeed, this works just fine for me on current master. Is there anything else I might be missing to reproduce your problem?

(To your broader point, yes, we should have a more complete example in the README.)

@fitzgen
Copy link
Contributor Author

fitzgen commented Oct 2, 2016

Here is everything I'm doing to reproduce. I'm using a local cargo benchcmp that has the prettytable update in #16. Any ideas?

fitzgen@nfitzgerald-22150 :: (master *) :: ~/src/hydra
    $ cargo bench > old
    Finished release [optimized] target(s) in 0.0 secs
     Running target/release/bench-f9747807159bcfdb
     Running target/release/deps/hydra-2f1061b3ba48e5e5

fitzgen@nfitzgerald-22150 :: (master *) :: ~/src/hydra
    $ cargo bench > new
    Finished release [optimized] target(s) in 0.0 secs
     Running target/release/bench-f9747807159bcfdb
     Running target/release/deps/hydra-2f1061b3ba48e5e5

fitzgen@nfitzgerald-22150 :: (master *) :: ~/src/hydra
    $ cat old

running 3 tests
test benches::trace_ring_buffer_in_mutex       ... bench:          51 ns/iter (+/- 16)
test benches::trace_ring_buffer_large_capacity ... bench:          12 ns/iter (+/- 1)
test benches::trace_ring_buffer_small_capacity ... bench:          20 ns/iter (+/- 4)

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


running 4 tests
test trace_ring_buffer::tests::trace_entry_has_right_size ... ignored
test trace_ring_buffer::tests::trace_ring_buffer_no_roll_over ... ignored
test trace_ring_buffer::tests::trace_ring_buffer_with_roll_over ... ignored
test trace_ring_buffer::tests::trace_ring_buffer_with_roll_over_and_does_not_divide_evenly ... ignored

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


fitzgen@nfitzgerald-22150 :: (master *) :: ~/src/hydra
    $ cat new

running 3 tests
test benches::trace_ring_buffer_in_mutex       ... bench:          52 ns/iter (+/- 3)
test benches::trace_ring_buffer_large_capacity ... bench:          14 ns/iter (+/- 1)
test benches::trace_ring_buffer_small_capacity ... bench:          19 ns/iter (+/- 3)

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


running 4 tests
test trace_ring_buffer::tests::trace_entry_has_right_size ... ignored
test trace_ring_buffer::tests::trace_ring_buffer_no_roll_over ... ignored
test trace_ring_buffer::tests::trace_ring_buffer_with_roll_over ... ignored
test trace_ring_buffer::tests::trace_ring_buffer_with_roll_over_and_does_not_divide_evenly ... ignored

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


fitzgen@nfitzgerald-22150 :: (master *) :: ~/src/hydra
    $ ../cargo-benchcmp/target/debug/cargo-benchcmp old new
Invalid arguments.

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

@BurntSushi
Copy link
Owner

You need to actually use cargo benchcmp, not cargo-benchcmp.

Yes, this is annoying and yes this should be in the README. It looks like this is Docopt's fault, and the error message is awful. :-(

@BurntSushi
Copy link
Owner

And to use cargo benchcmp, that requires cargo-benchcmp to be in your PATH. Blech.

@fitzgen
Copy link
Contributor Author

fitzgen commented Oct 2, 2016

Yay?

fitzgen@nfitzgerald-22150 :: (master *) :: ~/src/hydra
    $ cp ../cargo-benchcmp/target/debug/cargo-benchcmp ~/.cargo/bin/cargo-benchcmp 
../cargo-benchcmp/target/debug/cargo-benchcmp -> /Users/fitzgen/.cargo/bin/cargo-benchcmp

fitzgen@nfitzgerald-22150 :: (master *) :: ~/src/hydra
    $ cargo benchcmp old new
 name                                       old ns/iter  new ns/iter  diff ns/iter  diff % 
 benches::trace_ring_buffer_in_mutex        51           52                      1   1.96% 
 benches::trace_ring_buffer_large_capacity  12           14                      2  16.67% 
 benches::trace_ring_buffer_small_capacity  20           19                     -1  -5.00% 

It looks like this is Docopt's fault

Ah because when run as cargo benchcmp you get two args and when run as cargo-benchcmp you get one? Would it be possible to detect which situation we're in before passing args to docopt and then do the Right Thing after that?

@BurntSushi
Copy link
Owner

@fitzgen I'd wholeheartedly support that, yes. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants