perf: optimize B-tree traversal and freelist merging - #1240
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: mrueg The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Eliminate closure allocation overhead from sort.Search calls and reduce heap allocations during cursor creation. 1. Implement custom binary search routines to avoid sort.Search closures: - Added Inodes.Search and Inodes.SearchExact to internal/common/inode.go. - Added Page.SearchBranchPageElements and Page.SearchLeafPageElements to internal/common/page.go. - Replaced sort.Search in node.go (childIndex, put, del) and cursor.go (searchNode, searchPage, nsearch). - Inlined binary search inside Mergepgids in internal/common/page.go to optimize sorted union merging of page IDs. 2. Pre-allocate Cursor search stack: - Added a pre-allocated array initialStack [16]elemRef inside the Cursor struct in cursor.go. - Modified Bucket.Cursor in bucket.go to initialize the stack using this array slice backing, avoiding heap allocations during B-tree traversal for depth <= 16. Synthetic read/write benchmark results: BenchmarkRead: 15.30 ns/op -> 13.69 ns/op (~10.5% improvement in read latency). Signed-off-by: Manuel Rüger <manuel@rueg.eu>
|
@mrueg Thanks! I will take a look on it. |
There was a problem hiding this comment.
Pull request overview
This PR optimizes B+tree traversal and freelist merging hot paths by replacing sort.Search closure-based binary searches with custom non-closure implementations and by reducing allocations during cursor creation/traversal.
Changes:
- Added custom binary search helpers for
common.Inodesandcommon.Pageelement searches, and updated cursor/node search paths to use them. - Inlined binary search in
Mergepgidsto avoidsort.Searchclosure overhead. - Preallocated a small cursor stack buffer (
[16]elemRef) and wiredBucket.Cursor()to reuse it for typical tree depths.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| node.go | Switches child/key lookups from sort.Search to Inodes.Search. |
| internal/common/page.go | Inlines binary search in Mergepgids and adds page-element search helpers. |
| internal/common/inode.go | Adds Inodes.Search / Inodes.SearchExact custom binary searches. |
| cursor.go | Uses new binary searches and adds initialStack backing storage for the cursor stack. |
| bucket.go | Initializes cursor stack using Cursor.initialStack to avoid heap allocations for shallow traversals. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| type Cursor struct { | ||
| bucket *Bucket | ||
| stack []elemRef | ||
| bucket *Bucket | ||
| stack []elemRef | ||
| initialStack [16]elemRef | ||
| } |
Eliminate closure allocation overhead from
sort.Searchcalls and reduce heap allocations during cursor creation.Implement custom binary search routines to avoid
sort.Searchclosures:Inodes.SearchandInodes.SearchExacttointernal/common/inode.go.Page.SearchBranchPageElementsandPage.SearchLeafPageElementstointernal/common/page.go.sort.Searchinnode.go(childIndex,put,del) andcursor.go(searchNode,searchPage,nsearch).Mergepgidsininternal/common/page.goto optimize sorted union merging of page IDs.SearchExactbreaks early on a zero comparison instead of running the search to completion with a side-effecting closure. This is equivalent: keys are unique within a node, and the oldsort.Searchalways probed the index it returned (when< n), so theexactflag was only ever set at that index.Pre-allocate the
Cursorsearch stack:initialStack [16]elemRefarray inside theCursorstruct incursor.go.Cursor.stackis re-bound toinitialStack[:0]at the start of each traversal (first,last,seek), avoiding a separate heap allocation for the stack at tree depth <= 16.Benchmarks
Measured against
3fb8889with ad-hoc benchmarks over a 100k-key bucket (8-byte keys, 32-byte values,NoSync), 15 interleaved rounds per binary, compared withbenchstat:The gain is on point lookups, where cursor setup is a meaningful share of the work. A full sequential scan (
CursorScan) is unchanged, as expected: the cursor is allocated once and amortized over 100kNext()calls.Note on
cmd/bbolt bench: I originally quoted a ~10.5%BenchmarkReadimprovement from it. That number does not reproduce reliably — across 15 interleaved rounds it reports ±100%+ variance because each run creates and writes a ~150MB database, so filesystem state dominates a change measured in nanoseconds. The Go benchmarks above are the meaningful measurement.Notes for reviewers
Copying a
Cursor.stackis backed by the cursor's owninitialStack, soc2 := *c1would give the copy a slice header pointing into the original's array. This hazard is not new: before this change a copy's slice header aliased the original's heap-allocated backing array in exactly the same way. Binding at the start of every traversal rather than once at construction means a copied cursor re-binds to its own array on the nextFirst/Last/Seek, which makes copies strictly safer than before. The type is documented as non-copyable after first use. Nothing in bbolt copies aCursor— only*Cursoris ever handed out byBucket.Cursor()/Tx.Cursor().Memory trade-off.
elemRefis 24 bytes, soCursorgrows from 32 to 416 bytes and the cursor-allocating paths go from 168 B/op in 3 allocations to 416 B/op in 1. Fewer allocations, more bytes. Happy to drop the array to 8 entries if reviewers prefer, since bbolt trees are rarely deeper than ~5 levels.