Conversation
New PrefixIndex type — drop-in replacement for BpsTree lookups.
Per-prefix bucket architecture with adaptive node filling.
- [65536]prefixBucket{firstDI, endDI, nodes} — O(1) prefix lookup
- Adaptive: supplementary scan fills empty buckets with middle key
- narrowWithNodes: per-bucket binary search (max 8 nodes)
- Exact match shortcut: skip disk search on node cache hit
- L1 computed from L2 at build time
Benchmarks (same .kv file, random access):
100K keys: Seek 53% faster, Get 62% faster
1M keys: Seek 46% faster, Get 37% faster
Co-Authored-By: Shuo <shuo@erigon.dev>
PrefixIndex is only built when ERIGON_USE_PREFIX_INDEX=true. Default: false (BpsTree only, current behavior). Usage: ERIGON_USE_PREFIX_INDEX=true ./erigon ... Co-Authored-By: Shuo <shuo@erigon.dev>
Fix F3: Get returns (nil, false) instead of ErrBtIndexLookupBounds for non-existent key in last bucket where endDI==count. Fix F4: addNode copies key bytes via common.Copy to prevent mutation risk from external nodes slice. Add 11 correctness tests: boundary keys, exact match, non-existent, concurrent reads, BpsTree comparison, node key stability. Co-Authored-By: Shuo <shuo@erigon.dev>
…h/erigon into awskii/prefix-index-standalone
fb47e1a to
37a86f1
Compare
Lint fixes:
- Replace bytes.Repeat([]byte{0}, N) with make([]byte, N) (gocritic zeroByteRepeat)
- Replace string comparison with bytes.Equal (gocritic stringXbytes)
- Remove trailing newline (gofmt)
Bench fixes:
- Add missing buildBtreeIndex calls in BenchmarkPrefixIndexGet,
BenchmarkSeekComparison, and BenchmarkGetComparison so the .bt
index file exists before OpenBtreeIndexAndDataFile is called
- Add dbg.UsePrefixIndex=true in BenchmarkPrefixIndexSeek so
bt.search (PrefixIndex) is initialized
Co-authored-by: shuo <shuo@erigon.dev>
37a86f1 to
d6f336b
Compare
…tandalone # Conflicts: # db/datastruct/btindex/bpstree_bench_test.go
…tion - Remove keyCmpFunc callback from PrefixIndex struct and constructors - Add compareKey() method using seg.Reader.MatchCmp directly (zero-copy) - Simplify Seek() binary search: no error return from compare path - Fix comparison direction (cmp < 0 was inverted) - Update tests to match new NewPrefixIndex/NewPrefixIndexWithNodes signatures
|
#21872 could encode nodes from prefix tree to avoid it during startup |
Startup optimization is important. I guess it can be 3.8 headliner. Currently i focusing on RAM usage reduction - because bloatnet did use 90% of RAM on 64G machine (now it using 20GB: 4gb domain existence, 1.3gb bt, 0.5gb compress dicts, 2gb exec. Everything else is: peaks during merge build of rs/bt/kv etc...) FYI: If you have some advise "what we can add into |
…s) (erigontech#21875) ## Problem The only heap-resident structure of an open `.bt` is `BpsTree`'s pivot-key cache. On a mainnet node it was ~1.3 GB (`btindex.decodeListNodes` in heap profiles): ~44M pivots × 32 B/node (`Node{key []byte (24B), di uint64 (8B)}`), dominated by the storage domain. The pivot keys already point into mmap (no heap copy) — the cost was purely the per-node struct. ## Change Replace `mx []Node` with: - `keysBlob []byte` — the `[keyLen:u16][key]` records; mmap-backed for on-disk files (zero heap), heap only in `WarmUp`. - `nodeOfft []uint64` — byte offset of each pivot record (the only per-node heap cost). - `nodeStride uint64` — `di` is derived as `nodeDi(i) = i*nodeStride`, not stored. Net: **32 B → 8 B per node (~4×)**, i.e. ~1.3 GB → ~340 MB. No on-disk format change — `decodeNodes`/`decodeListNodesV0` now return offsets into the existing mmap'd nodes section instead of materializing structs. `uint64` (not `uint32`) offsets: a single `.bt`'s nodes section is bounded by its file size, and the largest storage file is already ~3.5 GB — near the uint32 4 GB ceiling and growing. `di` is uniformly `i*M` for all real files (footer and released-3.4 legacy). For legacy v0 files the stride is recovered from the on-disk `di` so a file opened with a different `M` than it was written with stays correct. ## Notes - Lookup cost stays flat: the old code already read pivot key bytes from mmap; the offset path adds only a 2-byte length read on the same cache line, and the offsets array is more cache-friendly. `bs`/`Get` benchmarks: 0 allocs, timings unchanged. - Structured to be range-friendly for a future PrefixIndex (erigontech#20180) rebase: buckets can hold index ranges into `nodeOfft` with derived `di`. Result: Bloatnet: `1.18G -> 0.028G` --------- Co-authored-by: moskud <sudeepdino008@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces a new PrefixIndex search engine for domain .kv files (as a faster alternative to BpsTree lookups) and wires it into BtIndex behind an environment flag.
Changes:
- Add standalone
PrefixIndeximplementation with per-prefix buckets, cached nodes, and MatchCmp-based comparisons. - Integrate
PrefixIndexintoBtIndex(conditional selection forGet/Seek/stats methods). - Add extensive correctness tests and new benchmarks comparing
PrefixIndexvsBpsTree.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/plans/20260415-prefixindex-matchcmp-integration.md | Implementation plan + benchmark notes for MatchCmp integration. |
| db/datastruct/btindex/testhelpers_test.go | Adds controlled/minimal .kv generators and helpers used by new tests. |
| db/datastruct/btindex/prefix_index.go | New PrefixIndex implementation (bucket ranges, node narrowing, MatchCmp compare). |
| db/datastruct/btindex/prefix_index_test.go | Large correctness/concurrency suite for PrefixIndex + equivalence checks vs BpsTree. |
| db/datastruct/btindex/btree_index.go | Adds optional PrefixIndex search engine to BtIndex and routes calls when enabled. |
| db/datastruct/btindex/bpstree_bench_test.go | Adds benchmarks for PrefixIndex and comparison benchmarks. |
| common/dbg/dbg_env.go | Adds env-backed dbg.UsePrefixIndex toggle. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…tandalone # Conflicts: # db/datastruct/btindex/btree_index.go # db/datastruct/btindex/testhelpers_test.go
…s.Clone, range-over-int, WaitGroup.Go) Claude-Session: https://claude.ai/code/session_01UYCsHj9HYTJnqUCy8a7W91
Resolve btree_index.go: main moved M from the global constant to the per-file footer value (m); keep the PrefixIndex wiring on top. Drop the M argument from OpenBtreeIndexAndDataFile call sites.
… bytes record() dropped every key of length <2, so those keys landed in no bucket and lookup() returned a DI range that starts after them: Get missed them and Seek skipped past them. Commitment domain keys are nibble paths and do get this short. Zero-pad a 1-byte key into the same bucket as the keys it sorts just before, which keeps every bucket a contiguous DI range. Also: drop two redundant full .kv scans from NewPrefixIndexWithNodes (counts now come from the first scan), derive the node-spacing divisor from maxNodesPerBucket, and align the benchmarks' write/read compression flags. Tests: differential test against BpsTree over mixed-length keys, both the scan and the pre-built-nodes constructor.
… and density benchmarks addNode kept the first maxNodesPerBucket pre-built nodes in each bucket. Nodes sit M keys apart, so a busy bucket holds many more than the cap and the kept ones all cluster at its head, leaving the tail with nothing to narrow against. Pick evenly spaced nodes instead: at 781 keys/bucket the narrowed window drops from 180 to 93 on average (max 333 to 127). Adds BenchmarkRealDomainComparison, which A/Bs both engines over a real domain file with keys sampled across the whole DI range rather than the head of the file, and TestNarrowingWindowByDensity, which measures narrowing directly.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
Suppressed comments (5)
db/datastruct/btindex/prefix_index_test.go:1794
- BtIndexWriter keeps a pivot node for di==0 as well (see btree_index.go:264-266). Excluding di==0 here means the test doesn’t cover PrefixIndexWithNodes behavior with the first pivot present.
if di > 0 && di%M == 0 {
db/datastruct/btindex/prefix_index_test.go:751
- BtIndexWriter keeps a pivot node for di==0 as well (see btree_index.go:264-266). Excluding di==0 here makes this test’s node slice diverge from the actual .bt layout.
if di > 0 && di%M == 0 {
db/datastruct/btindex/prefix_index_test.go:100
- BtIndexWriter keeps a pivot node for di==0 as well (see btree_index.go:264-266). Dropping di==0 here means the "with nodes" path isn’t exercising the real on-disk layout and may miss edge cases around the first pivot.
This issue also appears in the following locations of the same file:
- line 751
- line 1794
if di > 0 && di%M == 0 {
db/datastruct/btindex/prefix_index_test.go:1539
- This test uses t.Skip when the chosen gapKey overshoots the next key. That makes the test outcome data-dependent and can silently drop coverage. Prefer selecting a pair that satisfies the precondition (gap < next) and assert if none exists.
mid := len(keys) / 2
gap := gapKey(keys[mid])
if bytes.Compare(gap, keys[mid+1]) >= 0 {
// Gap overshot, skip this variant.
t.Skip("gap key overshot next key")
db/datastruct/btindex/bpstree_bench_test.go:104
- BenchmarkPrefixIndexSeek reads a key from
getterand then passes that slice intoSeekusing the same reader.seg.Reader.Nextreturns slices backed by the reader’s internal buffer, andSeekwill reset/read via the same reader, so the search key can be overwritten mid-call and the probe stream is disrupted. Use a separate probe reader and copy the key into a stable buffer before calling Seek (no per-iteration alloc needed).
key, _ = getter.Next(key[:0])
getter.Skip()
c, err := bt.search.Seek(getter, key)
require.NoError(t, err)
|
closed in favour of #23841 |
New
PrefixIndexsearch engine for domain.kvfiles, offered as an alternative toBpsTreelookups and gated behindERIGON_USE_PREFIX_INDEX(default off).Design
[65536]prefixBucket{firstDI, endDI, nodes}— O(1) 2-byte-prefix lookup replaces the binary search over the pivot listBenchmarks
The earlier "40-62% faster" numbers came from a synthetic 1M-key file, which is the most favourable shape possible: 1M keys over 65536 buckets is ~15 keys per bucket, so the node cache nearly indexes every key individually. Re-measured on an idle 12-core box, warm page cache,
mainmerged in so M=64 and interpolation search are both in the baseline, 3 runs per cell, keys sampled across the whole DI range.Synthetic, 1M keys: Get -46% (734 -> 393 ns), Seek -52% (878 -> 418 ns).
Real mainnet domain files:
So: Seek is consistently faster, Get is roughly a wash, and storage regresses on both.
Why storage regresses. Its keys are
address+slot, so every slot of one contract falls in a single 2-byte bucket. Probes drawn uniformly over keys land in big buckets in proportion to their size, so the effective window is set by the largest contracts, not by the 546-key mean. Accounts hold one key per address, so occupancy is even and the mean is representative — which is why accounts.0-8192 wins despite a higher mean occupancy (5171 keys/bucket) than storage.TestNarrowingWindowByDensitymeasures the mechanism without timing noise. BpsTree's window is bounded by M; PrefixIndex's grows with bucket occupancy:Overlap with the recent
.btworkBpsTree.Getonly, notSeek. PrefixIndex has neither. That is the asymmetry in every row above: Get is a wash, Seek is where PrefixIndex wins. Applying interpolation toSeekwould capture most of that without a second index. Interpolation's stated benefit is cold-read page locality, and all of these numbers are warm..bts; rebuilt at M=64 the gap narrows further.Build cost
NewPrefixIndexWithNodesscans the whole.kvat open — 1-9 s per file warm, a full file read cold — against milliseconds forNewBpsTreeWithNodes, which reads its pivots straight from the.bt. Persisting buckets into the.bt(#21872) is a precondition for this to be usable at all, independent of the search-speed question.Note also that with the flag on,
BtIndexbuilds both engines;bplusis dead weight for lookups but is what makes the A/B benchmark possible in one process.Fixes in this branch
record(), so they landed in no bucket andlookup()returned a DI range starting after them:Getreported not-found for keys that exist andSeekskipped past them. Commitment-domain keys are nibble paths and do get this short.TestPrefixIndexMatchesBpsTreeis a differential test over mixed-length keys covering both constructors.addNodekept the first 8 pre-built nodes per bucket rather than evenly spaced ones, so a busy bucket's tail had nothing to narrow against. Fixing this moved accounts.0-8192 Get from -2.6% to -14.9% and storage from +72% to +46.4%..kvscans removed fromNewPrefixIndexWithNodes..kvuncompressed and read it back through the compressed getter, so the file carried no pattern dictionary and the measured path was decompression-free.What would actually compose
Keep all pivots, bucketed, so the window stays bounded by M like BpsTree, and put the O(1) bucket lookup in front of it — rather than a fixed 8-node cache whose quality degrades with bucket skew. Combined with persisting the buckets in the
.bt, that keeps the Seek win without the build cost or the storage regression.Co-Authored-By: Shuo shuo@erigon.dev