bheisler / iai

Experimental one-shot benchmarking/profiling harness for Rust
Apache License 2.0
586 stars 24 forks source link

Panics with `no entry found for key` since valgrind 3.21.0 #34

Open niklasf opened 1 year ago

niklasf commented 1 year ago
❯ RUST_BACKTRACE=1 cargo bench
    Finished bench [optimized] target(s) in 0.01s
     Running benches/benches.rs (target/release/deps/benches-0f90e9e90a9ae8e1)
thread 'main' panicked at 'no entry found for key', /home/niklas/.cargo/registry/src/index.crates.io-6f17d22bba15001f/iai-0.1.1/src/lib.rs:162:40
stack backtrace:
   0: rust_begin_unwind
             at /rustc/8bdcc62cb0362869f0e7b43a6ae4f96b953d3cbc/library/std/src/panicking.rs:578:5
   1: core::panicking::panic_fmt
             at /rustc/8bdcc62cb0362869f0e7b43a6ae4f96b953d3cbc/library/core/src/panicking.rs:67:14
   2: core::panicking::panic_display
             at /rustc/8bdcc62cb0362869f0e7b43a6ae4f96b953d3cbc/library/core/src/panicking.rs:150:5
   3: core::panicking::panic_str
             at /rustc/8bdcc62cb0362869f0e7b43a6ae4f96b953d3cbc/library/core/src/panicking.rs:134:5
   4: core::option::expect_failed
             at /rustc/8bdcc62cb0362869f0e7b43a6ae4f96b953d3cbc/library/core/src/option.rs:1952:5
   5: iai::parse_cachegrind_output
   6: iai::run_bench
   7: iai::runner
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
error: bench failed, to rerun pass `--bench benches`

Works again after downgrading to valgrind 3.19.0.

fornwall commented 1 year ago

I think the issue starts with valgrind 3.21.0, starting with this change in that version (from https://valgrind.org/docs/manual/dist.news.html):

--cache-sim=no is now the default. The cache simulation is old and unlikely to match any real modern machine. This means only the Ir event are gathered by default, but that is by far the most useful event.

So either iai should add--cache-sim=yes, or only handle the Ir (instructions executed) event as the other ones may not be that useful.

niklasf commented 1 year ago

Oops, 3.21.0 is the first "bad" version, indeed. Looks like I messed up the downgrade on my first attempt.

sigaloid commented 1 year ago

Here's a PR that fixes it by adding that argument. Not sure the ramifications of still using the cache simulation, but I figured this would be better than simply removing all of the other statistics.

If you want to use my fork that re-enables the outdated cache simulation:

iai = { git = "https://github.com/sigaloid/iai", rev = "d56a597" }

If you want to use my fork that disables the outdated cache simulation (as mentioned, valgrind has deemed it outdated and not realistic):

iai = { git = "https://github.com/sigaloid/iai", rev = "6c83e942" }

This will reduce the stats you get to one number: Instruction count.

joonazan commented 1 year ago

I can confirm that the cache simulation is very inaccurate. Reducing memory use has a much higher impact in actual benchmarks than in iai. I wonder if there is another tool that is actually meant for simulating performance.