refactor: improve benchmark setup and execution for various tests
What changed, and why it matters
This commit is a code cleanup that changes how several performance benchmarks are set up and run. It does not alter the behavior of the actual Bitcoin Core software that users run, only the internal testing/benchmarking code. There is no security issue here.
No security action needed. This is a benign benchmark refactoring commit.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The commit refactors benchmark functions in src/bench/ to use nanobench’s epochIterations(1) and .setup() callbacks instead of manually resetting state inside bench.run(). It also renames DeserializeAndCheckBlockTest to CheckBlockTest and removes the stream rewind logic. These changes are confined to benchmark code and do not affect production consensus, networking, or wallet logic.
Changed components
src/bench/addrman.cppsrc/bench/checkblock.cppsrc/bench/coin_selection.cppsrc/bench/load_external.cppsrc/bench/streams_findbyte.cppInspect captured patch +46 / −58
diff --git a/src/bench/addrman.cpp b/src/bench/addrman.cpp
index d28030e4..fc081d9f 100644
--- a/src/bench/addrman.cpp
+++ b/src/bench/addrman.cpp
@@ -161,18 +161,13 @@ static void AddrManAddThenGood(benchmark::Bench& bench)
CreateAddresses();
- bench.run([&] {
- // To make the benchmark independent of the number of evaluations, we always prepare a new addrman.
- // This is necessary because AddrMan::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
- // AddrMan::Good() will still be noticeable.
- AddrMan addrman{EMPTY_NETGROUPMAN, /*deterministic=*/false, ADDRMAN_CONSISTENCY_CHECK_RATIO};
- AddAddressesToAddrMan(addrman);
-
- markSomeAsGood(addrman);
- });
+ std::optional<AddrMan> addrman;
+ bench.epochIterations(1)
+ .setup([&] {
+ addrman.emplace(EMPTY_NETGROUPMAN, /*deterministic=*/false, ADDRMAN_CONSISTENCY_CHECK_RATIO);
+ AddAddressesToAddrMan(*addrman);
+ })
+ .run([&] { markSomeAsGood(*addrman); });
}
BENCHMARK(AddrManAdd);
diff --git a/src/bench/checkblock.cpp b/src/bench/checkblock.cpp
index 765b8b0d..cbba543f 100644
--- a/src/bench/checkblock.cpp
+++ b/src/bench/checkblock.cpp
@@ -27,38 +27,30 @@
static void DeserializeBlockTest(benchmark::Bench& bench)
{
- DataStream stream(benchmark::data::block413567);
- std::byte a{0};
- stream.write({&a, 1}); // Prevent compaction
-
- bench.unit("block").run([&] {
- CBlock block;
- stream >> TX_WITH_WITNESS(block);
- bool rewound = stream.Rewind(benchmark::data::block413567.size());
- assert(rewound);
- });
+ DataStream stream;
+ bench.unit("block").epochIterations(1)
+ .setup([&] { stream = DataStream{benchmark::data::block413567}; })
+ .run([&] { CBlock block; stream >> TX_WITH_WITNESS(block); });
}
-static void DeserializeAndCheckBlockTest(benchmark::Bench& bench)
+static void CheckBlockTest(benchmark::Bench& bench)
{
- DataStream stream(benchmark::data::block413567);
- std::byte a{0};
- stream.write({&a, 1}); // Prevent compaction
-
ArgsManager bench_args;
const auto chainParams = CreateChainParams(bench_args, ChainType::MAIN);
- bench.unit("block").run([&] {
- CBlock block; // Note that CBlock caches its checked state, so we need to recreate it here
- stream >> TX_WITH_WITNESS(block);
- bool rewound = stream.Rewind(benchmark::data::block413567.size());
- assert(rewound);
-
- BlockValidationState validationState;
- bool checked = CheckBlock(block, validationState, chainParams->GetConsensus());
- assert(checked);
- });
+ CBlock block;
+ bench.unit("block").epochIterations(1)
+ .setup([&] {
+ block = CBlock{};
+ DataStream stream{benchmark::data::block413567};
+ stream >> TX_WITH_WITNESS(block);
+ })
+ .run([&] {
+ BlockValidationState validationState;
+ bool checked = CheckBlock(block, validationState, chainParams->GetConsensus());
+ assert(checked);
+ });
}
BENCHMARK(DeserializeBlockTest);
-BENCHMARK(DeserializeAndCheckBlockTest);
+BENCHMARK(CheckBlockTest);
diff --git a/src/bench/coin_selection.cpp b/src/bench/coin_selection.cpp
index 72b3d70e..801aee2c 100644
--- a/src/bench/coin_selection.cpp
+++ b/src/bench/coin_selection.cpp
@@ -124,17 +124,14 @@ static CAmount make_hard_case(int utxos, std::vector<OutputGroup>& utxo_pool)
static void BnBExhaustion(benchmark::Bench& bench)
{
- // Setup
std::vector<OutputGroup> utxo_pool;
-
- bench.run([&] {
- // Benchmark
- CAmount target = make_hard_case(17, utxo_pool);
- (void)SelectCoinsBnB(utxo_pool, target, /*cost_of_change=*/0, MAX_STANDARD_TX_WEIGHT); // Should exhaust
-
- // Cleanup
- utxo_pool.clear();
- });
+ CAmount target;
+ bench.epochIterations(1)
+ .setup([&] { target = make_hard_case(17, utxo_pool); })
+ .run([&] {
+ auto res{SelectCoinsBnB(utxo_pool, target, /*cost_of_change=*/0, MAX_STANDARD_TX_WEIGHT)}; // Should exhaust
+ ankerl::nanobench::doNotOptimizeAway(res);
+ });
}
BENCHMARK(CoinSelection);
diff --git a/src/bench/load_external.cpp b/src/bench/load_external.cpp
index 3350d16a..128d531f 100644
--- a/src/bench/load_external.cpp
+++ b/src/bench/load_external.cpp
@@ -62,12 +62,17 @@ static void LoadExternalBlockFile(benchmark::Bench& bench)
std::multimap<uint256, FlatFilePos> blocks_with_unknown_parent;
FlatFilePos pos;
- bench.run([&] {
- // "rb" is "binary, O_RDONLY", positioned to the start of the file.
- // The file will be closed by LoadExternalBlockFile().
- AutoFile file{fsbridge::fopen(blkfile, "rb")};
- testing_setup->m_node.chainman->LoadExternalBlockFile(file, &pos, &blocks_with_unknown_parent);
- });
+ bench.epochIterations(1)
+ .setup([&] {
+ blocks_with_unknown_parent.clear();
+ pos = FlatFilePos{};
+ })
+ .run([&] {
+ // "rb" is "binary, O_RDONLY", positioned to the start of the file.
+ // The file will be closed by LoadExternalBlockFile().
+ AutoFile file{fsbridge::fopen(blkfile, "rb")};
+ testing_setup->m_node.chainman->LoadExternalBlockFile(file, &pos, &blocks_with_unknown_parent);
+ });
fs::remove(blkfile);
}
diff --git a/src/bench/streams_findbyte.cpp b/src/bench/streams_findbyte.cpp
index 45e93d77..47b2ad74 100644
--- a/src/bench/streams_findbyte.cpp
+++ b/src/bench/streams_findbyte.cpp
@@ -22,10 +22,9 @@ static void FindByte(benchmark::Bench& bench)
file.seek(0, SEEK_SET);
BufferedFile bf{file, /*nBufSize=*/file_size + 1, /*nRewindIn=*/file_size};
- bench.run([&] {
- bf.SetPos(0);
- bf.FindByte(std::byte(1));
- });
+ bench.epochIterations(1)
+ .setup([&] { bf.SetPos(0); })
+ .run([&] { bf.FindByte(std::byte(1)); });
assert(file.fclose() == 0);
}
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.