Skip to content

Commit

Permalink
Merge bitcoin/bitcoin#30349: benchmark: Improve SipHash_32b accuracy …
Browse files Browse the repository at this point in the history
…to avoid potential optimization issues

42066f4 Refactor SipHash_32b benchmark to improve accuracy and avoid optimization issues (Lőrinc)

Pull request description:

  This PR stems from the discussions in bitcoin/bitcoin#30317 (comment)

  The previous benchmark for `SipHash` was slightly less accurate in representing real-world usage and allowed for potential compiler optimizations that could invalidate the benchmark.
  This change aims to ensure the benchmark produces more realistic results.

  By modifying the initial values and only incrementing the bytes of `val`, the benchmark should reflects a more typical usage patterns - and prevent the compiler from optimizing away the calculations.

  -------

  On my M1 processor the benchmark's speed changed significantly (but the CI seems to produce the same result as before):

  > cmake -B build -DCMAKE_BUILD_TYPE=Release -DBUILD_BENCH=ON && cmake --build build -j10 &&
  ./build/src/bench/bench_bitcoin --filter=SipHash_32b --min-time=1000

  Before:
  |               ns/op |                op/s |    err% |     total | benchmark
  |--------------------:|--------------------:|--------:|----------:|:----------
  |               35.15 |       28,445,856.66 |    0.2% |      1.10 | `SipHash_32b`

  After (note that only the benchmark changed):
  |               ns/op |                op/s |    err% |     total | benchmark
  |--------------------:|--------------------:|--------:|----------:|:----------
  |               22.05 |       45,350,886.64 |    0.3% |      1.10 | `SipHash_32b`

ACKs for top commit:
  maflcko:
    review ACK 42066f4
  achow101:
    ACK 42066f4
  hodlinator:
    ACK 42066f4

Tree-SHA512: 6bbe9d725d4c3396642e55ce48c31baa5339e56838d6d5fb377fb1069daa9292375e7020ceff7da0d78befffc1e984f717b5232217fe911989613480adaa937e
  • Loading branch information
achow101 committed Nov 14, 2024
2 parents 1a8f51e + 42066f4 commit 380e1f4
Showing 1 changed file with 9 additions and 3 deletions.
12 changes: 9 additions & 3 deletions src/bench/crypto_hash.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -192,10 +192,16 @@ static void SHA512(benchmark::Bench& bench)

static void SipHash_32b(benchmark::Bench& bench)
{
uint256 x;
uint64_t k1 = 0;
FastRandomContext rng{/*fDeterministic=*/true};
auto k0{rng.rand64()}, k1{rng.rand64()};
auto val{rng.rand256()};
auto i{0U};
bench.run([&] {
*((uint64_t*)x.begin()) = SipHashUint256(0, ++k1, x);
ankerl::nanobench::doNotOptimizeAway(SipHashUint256(k0, k1, val));
++k0;
++k1;
++i;
val.data()[i % uint256::size()] ^= i & 0xFF;
});
}

Expand Down

0 comments on commit 380e1f4

Please sign in to comment.