#33637 refactor: optimize block index comparisons (1.4-6.8x faster)
https://github.com/bitcoin/bitcoin/pull/33637 · · +247/-44 in 12 files, 7 commits · labels: Refactoring
Goal
- Speed up block index comparisons performed during block tracking and validation
- Reduces CPU overhead on frequent comparison paths during node operation
This pull request optimizes CBlockIndexWorkComparator and arith_uint256 comparison operations by moving comparators to headers to enable inlining and rewriting them using std::tie and C++20 operator<=>. It also introduces dedicated microbenchmarks, unit tests, and two differential fuzz targets to verify equivalence against the old comparison logic.
Problem: Block index comparisons are called frequently during candidate block tracking in setBlockIndexCandidates and methods like CheckBlockIndex. Previously, out-of-line function calls and multi-branch comparisons introduced measurable overhead on these comparison paths.
Category: Validation (#24 of 48)
P3 · speedup
- P3 because micro-optimizing comparator operations provides isolated speedups on specific checks
- The overall speedup yields minimal perceptible impact on node performance or initial block download
P3 because while CBlockIndexWorkComparator sees an isolated 1.4x to 6.8x speedup and CheckBlockIndex shows measurable gains, it is an internal micro-optimization that produces minimal perceptible impact on overall node operation or IBD.
Membership: Modifies CBlockIndexWorkComparator in src/node/blockstorage.h used directly across chainstate validation and block candidate selection.
Factors: security/stability 0, bug 0, performance 1, user value 0, leverage 1
Reviewability: Ready
- Ready for review
- CI is passing and previous reviewer feedback has been addressed
The branch is rebased, CI is green, and all previous reviewer requests have been integrated.
Author status: active; rebased cleanly and addressed reviewer suggestions.
Resolved concerns:
- mzumsande and sipa suggested adding differential fuzz tests rather than relying solely on randomized unit tests; the author added dedicated fuzz targets.
- maflcko noted that moving comparator logic to headers accounted for much of the gain and asked to structure the commits accordingly, which the author accommodated with updated benchmarks.
Agreement: Strong
- Strong support for optimizing and modernizing comparison hot paths
- Approved the simplified comparison approach (laanwj)
- Approved the localized hot-path micro-optimization (optout21)
- Concept support for the performance improvements (mzumsande, Raimo33)
Strong: ACKs from laanwj and optout21, concept support from mzumsande and Raimo33, with all testing feedback resolved.
Reviewers broadly agree on the approach and performance value; suggested additions for fuzz testing and benchmark formatting were implemented by the author.
- laanwj approved noting that '<=>' makes sense.
- optout21 ACKed the localized hot-path micro-optimization after verifying rebases.
- mzumsande and Raimo33 provided concept and approach ACKs.
- sipa advised adding fuzz equivalence tests, which the author implemented.
Review verdicts (DrahtBot): 1 (+1)
Files
221 lines under test/bench/ci.
- src/test/fuzz/block_index_work_comparator.cpp +61/-0
- src/test/fuzz/arith_uint256_comparison_equivalence.cpp +56/-0
- src/bench/blockstorage.cpp +53/-0
- src/test/blockchain_tests.cpp +29/-0
- src/node/blockstorage.cpp +0/-25
- src/node/blockstorage.h +16/-3
- src/test/arith_uint256_tests.cpp +19/-0
- src/arith_uint256.cpp +0/-12
- src/arith_uint256.h +9/-3
- src/test/fuzz/CMakeLists.txt +2/-0
- src/uint256.h +1/-1
- src/bench/CMakeLists.txt +1/-0
Card
PR 33637 optimizes CBlockIndexWorkComparator and arith_uint256 comparisons by inlining them into headers and adopting C++20 spaceship operators and std::tie. Microbenchmarks show a 1.4x to 6.8x speedup for comparator operations and a 1.4x to 1.6x improvement in CheckBlockIndex. The changes are accompanied by equivalence unit tests, microbenchmarks, and two differential fuzz targets. Reviewers have supported the approach with multiple ACKs, and the PR is ready for final review.