#35827 bench: add index benchmarks
| Reviewability | Reviews | Agreement | Size | ||||
|---|---|---|---|---|---|---|---|
| #35827 bench: add index benchmarks | Ready | 0 | Strong | 38 + 412 tests | |||
|
|
|
|
| |||
Reviewability: ReadyStalePaused Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs. Size: lines added or modified outside tests, then in tests; greener = smaller. | |||||||
Goal
Adds sync and lookup microbenchmarks for TxIndex, TxoSpenderIndex, and BlockFilterIndex, backed by shared utilities that generate synthetic blocks with chained spends and distinct output scripts. Also adds documentation to doc/benchmarking.md explaining how to measure I/O syscalls, peak resident memory, and disk usage using -testdatadir.
Problem: Evaluating performance changes to the index subsystem previously required running a full initial block download (IBD). The only existing index benchmark, BlockFilterIndexSync, only processes blocks containing coinbase transactions, leaving no benchmarks for TxIndex or TxoSpenderIndex and no way to measure index scaling against transaction counts or distinct script outputs.
Evidence of importance:
- PR description: The author notes that evaluating index changes previously required a full IBD, whereas this benchmark suite runs in under a minute.
- PR description: The author demonstrates its utility on PR #35531 (showing a 49% smaller index vs lookup regression) and PR #34489 (showing a 91% reduction in write syscalls with unchanged CPU).
Scope: Touches only benchmark harnesses (src/bench/) and developer benchmarking documentation.
Risk of a mistake: low (tests, docs). A flaw in the benchmark code or documentation could yield inaccurate metrics or break the benchmark harness, but cannot affect node runtime or consensus behavior.
Changed since the previous assessment: Patch-id changed with minor adjustments; the overall commit structure, benchmark targets, and documentation additions remain unchanged from the previous version.
Reviewability
Ready
Nothing known that would invalidate a review now.
Agreement
Strong
substantive support, no open objection (l0rinc)
Thread notes: First read of the PR. l0rinc gave Concept ACK and noted concerns around measurement distortion from locking and small benchmark memory caching, both resolved by arejula27.
Objections, suggestions, and questions:
| Reviewer | Type | Harm, request, or question | Status | Must be | Author replied | Quote |
|---|---|---|---|---|---|---|
| l0rinc (member) | objection, correctness | Distorts benchmark timing results by locking inside the measured loop | fixed · fix pushed · v1, not yet re-checked | answered | yes | 2026-07-27: 'locking inside the measured block will likely distort the results. Consider precalculating and maybe using setup block' Status from: 2026-09-06: arejula27: 'The rest of your comments are applied, thank you for the review' |
| l0rinc (member) | objection, correctness | Disk benchmarks might inadvertently remain in memory/page cache, failing to measure disk performance | fixed · fix pushed · v1, not yet re-checked | answered | yes | 2026-07-27: 'these are likely small, so even if they're not in memory, the OS will likely make sure they're still actually in memory...' Status from: 2026-09-06: arejula27: 'With `-testdatadir` on real storage the Disk/Mem split shows up on its own, around 2.7x for txindex. The helper documents that now, since the default hides it.' |
Support:
- l0rinc (member) (Concept ACK, 2026-07-27): Thanks for taking care of these, I will review them later in more detail, just quickly glanced at the structure.
Participants:
- l0rinc (member, support, 10 statements 2026-07-27 to 2026-07-28): Concept ACK; reviewed structure and raised observations on benchmark locking, OS caching, and assertion handling
Files
412 lines under test/bench/ci.
- src/bench/index_sync_util.cpp +147/-0
- src/bench/index_sync_util.h +99/-0
- src/bench/index_txospender.cpp +68/-0
- src/bench/index_txindex.cpp +62/-0
- doc/benchmarking.md +38/-0
- src/bench/index_blockfilter.cpp +33/-0
- src/bench/CMakeLists.txt +3/-0
Card
The short text the ranking pass compares across PRs.
Adds sync and lookup benchmarks for TxIndex, TxoSpenderIndex, and realistic BlockFilterIndex workloads, along with documentation on measuring benchmark I/O and peak memory. Previously, the only index benchmark synced coinbase-only blocks, and evaluating performance changes to indices required a full IBD. The author demonstrated its utility on two recent PRs (#35531 and #34489) to quantify disk usage, lookup latency regressions, and a 91% reduction in write syscalls.
Data
thread read through 2026-09-06 (16 statements), head 2c97a9ad93
Discussion read under schema 1 rules: suggestions and questions may be missing, and an objection may combine several harms (schema history).
labels: Tests
Categories
Test infrastructure
P3 · test coverage (ranking pass: Moved down into the benchmark cluster; no test currently fails and the benefit is profiling convenience.)
The PR adds index microbenchmarks that evaluate performance changes in under a minute instead of requiring a full initial block download. This provides clear value for testing and profiling index improvements, but is reasonably deferrable.
Membership: Adds benchmark harnesses and synthetic block generation utilities in src/bench/ with the Tests label.
Factors: security/stability 0, bug 0, performance 0, user value 0, leverage 1