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

Ability to benchmark a top-level binary #8

Closed
epage opened this issue Jul 25, 2023 · 11 comments
Closed

Ability to benchmark a top-level binary #8

epage opened this issue Jul 25, 2023 · 11 comments

Comments

@epage
Copy link

epage commented Jul 25, 2023

I'm looking to do deterministic end-to-end benchmarking. It'd be great if I could have my target binary and flags passed directly to callgrind. First off, I'm unsure if callgrind is recursive for process spawns (I guess it is with #6 though that gets complicated and I don't want it further recursive). Even if it is, I'd still have the overhead of the "bench" function.

@osiewicz
Copy link
Owner

Hey,
Thanks for reaching out. What you're suggesting should be pretty easy to achieve.
We don't even need #6 to land this. In short, I have to reorganize the guts of calliper a bit to have callgrind launching as a part of the public API.

Right now I am on vacation, but I will take a stab at it on Monday. Best case scenario there'll be a release on the same day, we'll see :)

I am curious though, what do you mean by the overhead of a bench function?

@osiewicz
Copy link
Owner

osiewicz commented Jul 31, 2023

I suppose all we need to do is to add a new function to Scenario, akin to new_with_args or something. I'd expect it to take a std::process::Command which it'll overwrite to run it under Callgrind then. I believe that should solve this use case, though feedback is most welcome.
The only issue I have with that is that it complicates spawning logic a bit (we no longer want to unconditionally respawn self), but that should be okay.

@epage
Copy link
Author

epage commented Aug 1, 2023

I am curious though, what do you mean by the overhead of a bench function?

Say I leveraged #6 to implement this, calliper would start capturing from the beginning of my bench, through the fork+exec, the running of my binary, to the end of my bench. By implementing direct support for this, only the running of my binary would be captured

The only issue I have with that is that it complicates spawning logic a bit (we no longer want to unconditionally respawn self), but that should be okay.

At a high level, it seems fine. I can always give a prototype a test drive and iterate from there.

@osiewicz
Copy link
Owner

osiewicz commented Aug 2, 2023

The prototype is now available on main. I've also added a small example in here.

@epage
Copy link
Author

epage commented Aug 2, 2023

Thanks!

Gave it a try but I seem to be getting a panic

❯ RUST_BACKTRACE=1 cargo bench --bench cli
    Finished bench [optimized] target(s) in 0.05s
     Running benches/cli.rs (/home/epage/src/personal/typos/target/release/deps/cli-4c1f9f287d15971c)
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `Some(2)`,
 right: `Some(0)`: ==2411428== Callgrind, a call-graph generating cache profiler
==2411428== Copyright (C) 2002-2017, and GNU GPL'd, by Josef Weidendorfer et al.
==2411428== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==2411428== Command: /home/epage/src/personal/typos/target/release/typos ../typos-dict/assets/words.csv
==2411428==
==2411428== For interactive control, run 'callgrind_control -h'.
==2411428==
==2411428== Events    : Ir Bc Bcm Bi Bim
==2411428== Collected : 2714463562 438992287 20385253 30861209 4447819
==2411428==
==2411428== I   refs:      2,714,463,562
==2411428==
==2411428== Branches:        469,853,496  (438,992,287 cond + 30,861,209 ind)
==2411428== Mispredicts:      24,833,072  ( 20,385,253 cond +  4,447,819 ind)
==2411428== Mispred rate:            5.3% (        4.6%     +       14.4%   )
', /home/epage/.cargo/git/checkouts/calliper-5e1d323732348e59/8761be2/src/callgrind.rs:133:9
stack backtrace:
   0: rust_begin_unwind
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/panicking.rs:593:5
   1: core::panicking::panic_fmt
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/core/src/panicking.rs:67:14
   2: core::panicking::assert_failed_inner
   3: core::panicking::assert_failed
   4: calliper::callgrind::spawn_callgrind
   5: calliper::runner::Runner::run
   6: cli::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
error: bench failed, to rerun pass `--bench cli`

See crate-ci/typos#786

Looks like its expecting the call to always which makes sense for benchmarking function calls but for benchmarking binaries, they can be expected to have a non-zero exit code. In this case, its a spell checker looking for typos and finding them.

@osiewicz
Copy link
Owner

osiewicz commented Aug 2, 2023

Uh, yeah, it looks like this assert is no longer valid (in most cases at least). I'll push a patch today.

@osiewicz
Copy link
Owner

osiewicz commented Aug 2, 2023

Fixed as of a2ff6ab

@epage
Copy link
Author

epage commented Aug 2, 2023

That worked, thanks!

@epage epage closed this as completed Aug 2, 2023
@epage
Copy link
Author

epage commented Aug 2, 2023

btw I saw in bencherdev/bencher#82 talk of integrating with bencher.dev. Any updates on that?

@osiewicz
Copy link
Owner

osiewicz commented Aug 2, 2023

Not really, I didn't get to that just yet - development around calliper has kind of stagnated recently. There should be some more activity in the following months and I may just start with bencher.dev integration though - it feels really good to see people actually use this (especially yourself). :)
If I recall correctly, most of the changes needed to make bencher.dev integration happen were made during iai integration I opened a PR for. I will give it a look soon.

On that note, feedback is most welcome - beyond bencher.dev integration, I feel like run summaries could use some love so that eventually we could make working with benchmark results programatically a bit easier.

@epage
Copy link
Author

epage commented Aug 2, 2023

For the most part, I can limp along with features for now because I want to make it easier to have shared features across different styles of benchmarking tools. I had originally planned to hold off on using something like this until after that work is done but I'm really wanting benchmarking CI support for rust-lang/cargo#12207 to make sure some use cases are fast enough and that we get faster

See also

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