From 153e6860e84df0a3d52e5a3b2fe9c37b5e0b029a Mon Sep 17 00:00:00 2001
From: Martin Ankerl <martin.ankerl@gmail.com>
Date: Sat, 18 Sep 2021 08:08:46 +0200
Subject: [PATCH] bench: change AddrManGood to AddrManAddThenGood

Moves some of the setup into the benchmark loop so it is possible to run
the loop for an arbitrary number of times. Due to recent optimizations
in #22974 the benchmark now runs much faster, so the inner loop now calls
Good() 32 times as often to get better numbers.

Renamed the benchmark to AddrManAddThenGood because that's now what is
actually tested. To get the the number of just Good(), one needs to
subtract the benchmark result of AddrManAdd.
---
 src/bench/addrman.cpp | 38 +++++++++++++++-----------------------
 1 file changed, 15 insertions(+), 23 deletions(-)

diff --git a/src/bench/addrman.cpp b/src/bench/addrman.cpp
index e5dd571a4ca..bebf86a09d9 100644
--- a/src/bench/addrman.cpp
+++ b/src/bench/addrman.cpp
@@ -103,41 +103,33 @@ static void AddrManGetAddr(benchmark::Bench& bench)
     });
 }
 
-static void AddrManGood(benchmark::Bench& bench)
+static void AddrManAddThenGood(benchmark::Bench& bench)
 {
-    /* Create many CAddrMan objects - one to be modified at each loop iteration.
-     * This is necessary because the CAddrMan::Good() method modifies the
-     * object, affecting the timing of subsequent calls to the same method and
-     * we want to do the same amount of work in every loop iteration. */
-
-    bench.epochs(5).epochIterations(1);
-    const uint64_t addrman_count{bench.epochs() * bench.epochIterations()};
-    Assert(addrman_count == 5U);
-
-    std::vector<std::unique_ptr<CAddrMan>> addrmans(addrman_count);
-    for (size_t i{0}; i < addrman_count; ++i) {
-        addrmans[i] = std::make_unique<CAddrMan>(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 0);
-        FillAddrMan(*addrmans[i]);
-    }
-
     auto markSomeAsGood = [](CAddrMan& addrman) {
         for (size_t source_i = 0; source_i < NUM_SOURCES; ++source_i) {
             for (size_t addr_i = 0; addr_i < NUM_ADDRESSES_PER_SOURCE; ++addr_i) {
-                if (addr_i % 32 == 0) {
-                    addrman.Good(g_addresses[source_i][addr_i]);
-                }
+                addrman.Good(g_addresses[source_i][addr_i]);
             }
         }
     };
 
-    uint64_t i = 0;
+    CreateAddresses();
+
     bench.run([&] {
-        markSomeAsGood(*addrmans.at(i));
-        ++i;
+        // To make the benchmark independent of the number of evaluations, we always prepare a new addrman.
+        // This is necessary because CAddrMan::Good() method modifies the object, affecting the timing of subsequent calls
+        // to the same method and we want to do the same amount of work in every loop iteration.
+        //
+        // This has some overhead (exactly the result of AddrManAdd benchmark), but that overhead is constant so improvements in
+        // CAddrMan::Good() will still be noticeable.
+        CAddrMan addrman(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 0);
+        AddAddressesToAddrMan(addrman);
+
+        markSomeAsGood(addrman);
     });
 }
 
 BENCHMARK(AddrManAdd);
 BENCHMARK(AddrManSelect);
 BENCHMARK(AddrManGetAddr);
-BENCHMARK(AddrManGood);
+BENCHMARK(AddrManAddThenGood);