diff --git a/internal/manifest/annotator.go b/internal/manifest/annotator.go index edab24db9a..729fc41dbd 100644 --- a/internal/manifest/annotator.go +++ b/internal/manifest/annotator.go @@ -4,6 +4,12 @@ package manifest +import ( + "sort" + + "github.com/cockroachdb/pebble/internal/base" +) + // The Annotator type defined below is used by other packages to lazily // compute a value over a B-Tree. Each node of the B-Tree stores one // `annotation` per annotator, containing the result of the computation over @@ -59,6 +65,11 @@ type annotation struct { // AnnotationAggregator.Accumulate or AnnotationAggregator.Merge. // NB: This is untyped for the same reason as annotator above. v interface{} + + // scratch is used to hold the aggregated annotation value when computing + // range annotations in order to avoid overwriting an already cached + // annotation, and to also avoid additional allocations. + scratch interface{} // valid indicates whether future reads of the annotation may use the // value as-is. If false, v will be zeroed and recalculated. valid bool @@ -81,39 +92,114 @@ func (a *Annotator[T]) findAnnotation(n *node) *annotation { return &n.annot[len(n.annot)-1] } -// nodeAnnotation computes this annotator's annotation of this node across all -// files in the node's subtree. The second return value indicates whether the -// annotation is stable and thus cacheable. -func (a *Annotator[T]) nodeAnnotation(n *node) (_ *T, cacheOK bool) { +// nodeRangeAnnotation computes this annotator's annotation of a node across +// all files in the range defined by lowerBound and upperBound. The second +// return value indicates whether the annotation is stable and thus cacheable. +func (a *Annotator[T]) nodeRangeAnnotation( + n *node, + cmp base.Compare, + // lowerBound and upperBound may be nil to indicate no lower or upper bound. + lowerBound []byte, + // upperBound is a UserKeyBoundary that may be inclusive or exclusive. + upperBound *base.UserKeyBoundary, +) (v *T, cacheOK bool) { annot := a.findAnnotation(n) - t := annot.v.(*T) - // If the annotation is already marked as valid, we can return it without + // If the annotation is already marked as valid and this node's + // subtree is fully within the bounds, we can return it without // recomputing anything. - if annot.valid { - return t, true + if lowerBound == nil && upperBound == nil && annot.valid { + return annot.v.(*T), true + } + + // We will accumulate annotations from each item in the end-exclusive + // range [leftItem, rightItem). + leftItem, rightItem := 0, int(n.count) + if lowerBound != nil { + // leftItem is the index of the first item that overlaps the lower bound. + leftItem = sort.Search(int(n.count), func(i int) bool { + return cmp(lowerBound, n.items[i].Largest.UserKey) <= 0 + }) + } + if upperBound != nil { + // rightItem is the index of the first item that does not overlap the + // upper bound. + rightItem = sort.Search(int(n.count), func(i int) bool { + return !upperBound.IsUpperBoundFor(cmp, n.items[i].Smallest.UserKey) + }) + } + + var result *T + switch { + // If there is no cached annotation, we can directly write to the node's + // annotation value. + case !annot.valid: + result = a.Aggregator.Zero(annot.v.(*T)) + // Otherwise, use annot.scratch as scratch space to avoid allocations. + // The allocation for annot.scratch is performed lazily here instead of + // within findAnnotation to avoid an allocation when range annotations + // are not used. + case annot.scratch == nil: + annot.scratch = a.Aggregator.Zero(nil) + result = annot.scratch.(*T) + default: + result = a.Aggregator.Zero(annot.scratch.(*T)) } - t = a.Aggregator.Zero(t) valid := true + // Accumulate annotations from every item that overlaps the bounds. + for i := leftItem; i < rightItem; i++ { + v, ok := a.Aggregator.Accumulate(n.items[i], result) + result = v + valid = valid && ok + } - for i := int16(0); i <= n.count; i++ { - if !n.leaf { - v, ok := a.nodeAnnotation(n.children[i]) - t = a.Aggregator.Merge(v, t) - valid = valid && ok + if !n.leaf { + // We will accumulate annotations from each child in the end-inclusive + // range [leftChild, rightChild]. + leftChild, rightChild := leftItem, rightItem + // If the lower bound overlaps with the child at leftItem, there is no + // need to accumulate annotations from the child to its left. + if leftItem < int(n.count) && cmp(lowerBound, n.items[leftItem].Smallest.UserKey) >= 0 { + leftChild++ } + // If the upper bound spans beyond the child at rightItem, we must also + // accumulate annotations from the child to its right. + if rightItem < int(n.count) && upperBound.IsUpperBoundFor(cmp, n.items[rightItem].Largest.UserKey) { + rightChild++ + } + + for i := leftChild; i <= rightChild; i++ { + newLowerBound, newUpperBound := lowerBound, upperBound + // If this child is to the right of leftItem, then its entire + // subtree is within the lower bound. + if i > leftItem { + newLowerBound = nil + } + // If this child is to the left of rightItem, then its entire + // subtree is within the upper bound. + if i < rightItem { + newUpperBound = nil + } - if i < n.count { - var ok bool - t, ok = a.Aggregator.Accumulate(n.items[i], t) + v, ok := a.nodeRangeAnnotation( + n.children[i], + cmp, + newLowerBound, + newUpperBound, + ) + result = a.Aggregator.Merge(v, result) valid = valid && ok } } - annot.v = t - annot.valid = valid + // Update this node's cached annotation only if we accumulated from its + // entire subtree. + if lowerBound == nil && upperBound == nil { + annot.v = result + annot.valid = valid + } - return t, annot.valid + return result, valid } // InvalidateAnnotation removes any existing cached annotations from this @@ -138,7 +224,7 @@ func (a *Annotator[T]) LevelAnnotation(lm LevelMetadata) *T { return a.Aggregator.Zero(nil) } - v, _ := a.nodeAnnotation(lm.tree.root) + v, _ := a.nodeRangeAnnotation(lm.tree.root, lm.tree.cmp, nil, nil) return v } @@ -158,6 +244,21 @@ func (a *Annotator[T]) MultiLevelAnnotation(lms []LevelMetadata) *T { return aggregated } +// LevelRangeAnnotation calculates the annotation defined by this Annotator for +// the files within LevelMetadata which are within the range +// [lowerBound, upperBound). A pointer to the Annotator is used as the key for +// pre-calculated values, so the same Annotator must be used to avoid duplicate +// computation. Annotation must not be called concurrently, and in practice this +// is achieved by requiring callers to hold DB.mu. +func (a *Annotator[T]) LevelRangeAnnotation(lm LevelMetadata, bounds *base.UserKeyBounds) *T { + if lm.Empty() { + return a.Aggregator.Zero(nil) + } + + v, _ := a.nodeRangeAnnotation(lm.tree.root, lm.tree.cmp, bounds.Start, &bounds.End) + return v +} + // InvalidateAnnotation clears any cached annotations defined by Annotator. A // pointer to the Annotator is used as the key for pre-calculated values, so // the same Annotator must be used to clear the appropriate cached annotation. @@ -206,6 +307,14 @@ func SumAnnotator(accumulate func(f *FileMetadata) (v uint64, cacheOK bool)) *An } } +// NumFilesAnnotator is an Annotator which computes an annotation value +// equal to the number of files included in the annotation. Particularly, it +// can be used to efficiently calculate the number of files in a given key +// range using range annotations. +var NumFilesAnnotator = SumAnnotator(func(f *FileMetadata) (uint64, bool) { + return 1, true +}) + // PickFileAggregator implements the AnnotationAggregator interface. It defines // an aggregator that picks a single file from a set of eligible files. type PickFileAggregator struct { diff --git a/internal/manifest/annotator_test.go b/internal/manifest/annotator_test.go index 07badf6a38..52db4f0175 100644 --- a/internal/manifest/annotator_test.go +++ b/internal/manifest/annotator_test.go @@ -5,54 +5,47 @@ package manifest import ( + "math/rand" "testing" "github.com/cockroachdb/pebble/internal/base" "github.com/stretchr/testify/require" ) -func makeTestLevelMetadata(count int) (LevelMetadata, []*FileMetadata) { - files := make([]*FileMetadata, count) - for i := 0; i < count; i++ { - files[i] = newItem(key(i)) +// Creates a version with numFiles files in level 6. +func makeTestVersion(numFiles int) (*Version, []*FileMetadata) { + files := make([]*FileMetadata, numFiles) + for i := 0; i < numFiles; i++ { + // Each file spans 10 keys, e.g. [0->9], [10->19], etc. + files[i] = (&FileMetadata{}).ExtendPointKeyBounds( + base.DefaultComparer.Compare, key(i*10), key(i*10+9), + ) + files[i].InitPhysicalBacking() } - lm := MakeLevelMetadata(base.DefaultComparer.Compare, 6, files) - return lm, files -} + var levelFiles [7][]*FileMetadata + levelFiles[6] = files -// NumFilesAnnotator is an Annotator which computes an annotation value -// equal to the number of files included in the annotation. -var NumFilesAnnotator = SumAnnotator(func(f *FileMetadata) (uint64, bool) { - return 1, true -}) + v := NewVersion(base.DefaultComparer, 0, levelFiles) + return v, files +} func TestNumFilesAnnotator(t *testing.T) { const count = 1000 - lm, _ := makeTestLevelMetadata(0) + v, _ := makeTestVersion(0) for i := 1; i <= count; i++ { - lm.tree.Insert(newItem(key(i))) - numFiles := *NumFilesAnnotator.LevelAnnotation(lm) + v.Levels[6].tree.Insert(newItem(key(i))) + numFiles := *NumFilesAnnotator.LevelAnnotation(v.Levels[6]) require.EqualValues(t, i, numFiles) } - - numFiles := *NumFilesAnnotator.LevelAnnotation(lm) - require.EqualValues(t, count, numFiles) - - numFiles = *NumFilesAnnotator.LevelAnnotation(lm) - require.EqualValues(t, count, numFiles) - - lm.tree.Delete(newItem(key(count / 2))) - numFiles = *NumFilesAnnotator.LevelAnnotation(lm) - require.EqualValues(t, count-1, numFiles) } func BenchmarkNumFilesAnnotator(b *testing.B) { - lm, _ := makeTestLevelMetadata(0) + v, _ := makeTestVersion(0) for i := 1; i <= b.N; i++ { - lm.tree.Insert(newItem(key(i))) - numFiles := *NumFilesAnnotator.LevelAnnotation(lm) + v.Levels[6].tree.Insert(newItem(key(i))) + numFiles := *NumFilesAnnotator.LevelAnnotation(v.Levels[6]) require.EqualValues(b, uint64(i), numFiles) } } @@ -70,12 +63,115 @@ func TestPickFileAggregator(t *testing.T) { }, } - lm, files := makeTestLevelMetadata(1) + v, files := makeTestVersion(1) for i := 1; i <= count; i++ { - lm.tree.Insert(newItem(key(i))) - pickedFile := a.LevelAnnotation(lm) + v.Levels[6].tree.Insert(newItem(key(i))) + pickedFile := a.LevelAnnotation(v.Levels[6]) // The picked file should always be the one with the smallest key. require.Same(t, files[0], pickedFile) } } + +func bounds(i int, j int, exclusive bool) *base.UserKeyBounds { + b := base.UserKeyBoundsEndExclusiveIf(key(i).UserKey, key(j).UserKey, exclusive) + return &b +} + +func randomBounds(rng *rand.Rand, count int) *base.UserKeyBounds { + first := rng.Intn(count) + second := rng.Intn(count) + exclusive := rng.Intn(2) == 0 + return bounds(min(first, second), max(first, second), exclusive) +} + +func requireMatchOverlaps(t *testing.T, v *Version, bounds *base.UserKeyBounds) { + overlaps := v.Overlaps(6, *bounds) + numFiles := *NumFilesAnnotator.LevelRangeAnnotation(v.Levels[6], bounds) + require.EqualValues(t, overlaps.length, numFiles) +} + +func TestNumFilesRangeAnnotationEmptyRanges(t *testing.T) { + const count = 5_000 + v, files := makeTestVersion(count) + + // Delete files containing key ranges [0, 999] and [24_000, 25_999]. + for i := 0; i < 100; i++ { + v.Levels[6].tree.Delete(files[i]) + } + for i := 2400; i < 2600; i++ { + v.Levels[6].tree.Delete(files[i]) + } + + // Ranges that are completely empty. + requireMatchOverlaps(t, v, bounds(1, 999, false)) + requireMatchOverlaps(t, v, bounds(0, 1000, true)) + requireMatchOverlaps(t, v, bounds(50_000, 60_000, false)) + requireMatchOverlaps(t, v, bounds(24_500, 25_500, false)) + requireMatchOverlaps(t, v, bounds(24_000, 26_000, true)) + + // Partial overlaps with empty ranges. + requireMatchOverlaps(t, v, bounds(0, 1000, false)) + requireMatchOverlaps(t, v, bounds(20, 1001, true)) + requireMatchOverlaps(t, v, bounds(20, 1010, true)) + requireMatchOverlaps(t, v, bounds(23_000, 27_000, true)) + requireMatchOverlaps(t, v, bounds(25_000, 40_000, false)) + requireMatchOverlaps(t, v, bounds(25_500, 26_001, true)) + + // Ranges which only spans a single table. + requireMatchOverlaps(t, v, bounds(45_000, 45_000, true)) + requireMatchOverlaps(t, v, bounds(30_000, 30_001, true)) + requireMatchOverlaps(t, v, bounds(23_000, 23_000, false)) +} + +func TestNumFilesRangeAnnotationRandomized(t *testing.T) { + const count = 10_000 + const numIterations = 10_000 + + v, _ := makeTestVersion(count) + + rng := rand.New(rand.NewSource(int64(0))) + for i := 0; i < numIterations; i++ { + requireMatchOverlaps(t, v, randomBounds(rng, count*11)) + } +} + +func BenchmarkNumFilesRangeAnnotation(b *testing.B) { + const count = 100_000 + v, files := makeTestVersion(count) + + rng := rand.New(rand.NewSource(int64(0))) + b.Run("annotator", func(b *testing.B) { + for i := 0; i < b.N; i++ { + b := randomBounds(rng, count*11) + // Randomly delete and reinsert a file to verify that range + // annotations are still fast despite small mutations. + toDelete := rng.Intn(count) + v.Levels[6].tree.Delete(files[toDelete]) + + NumFilesAnnotator.LevelRangeAnnotation(v.Levels[6], b) + + v.Levels[6].tree.Insert(files[toDelete]) + } + }) + + // Also benchmark an equivalent aggregation using version.Overlaps to show + // the difference in performance. + b.Run("overlaps", func(b *testing.B) { + for i := 0; i < b.N; i++ { + b := randomBounds(rng, count*11) + toDelete := rng.Intn(count) + v.Levels[6].tree.Delete(files[toDelete]) + + overlaps := v.Overlaps(6, *b) + iter := overlaps.Iter() + numFiles := 0 + for f := iter.First(); f != nil; f = iter.Next() { + numFiles++ + } + + v.Levels[6].tree.Insert(files[toDelete]) + } + }) + +} diff --git a/internal/manifest/btree.go b/internal/manifest/btree.go index 350200b612..02d2f22cfb 100644 --- a/internal/manifest/btree.go +++ b/internal/manifest/btree.go @@ -13,6 +13,7 @@ import ( "unsafe" "github.com/cockroachdb/errors" + "github.com/cockroachdb/pebble/internal/base" "github.com/cockroachdb/pebble/internal/invariants" ) @@ -300,14 +301,14 @@ func (n *node) popFront() (*FileMetadata, *node) { // // This function is for use only as a helper function for internal B-Tree code. // Clients should not invoke it directly. -func (n *node) find(cmp btreeCmp, item *FileMetadata) (index int, found bool) { +func (n *node) find(bcmp btreeCmp, item *FileMetadata) (index int, found bool) { // Logic copied from sort.Search. Inlining this gave // an 11% speedup on BenchmarkBTreeDeleteInsert. i, j := 0, int(n.count) for i < j { h := int(uint(i+j) >> 1) // avoid overflow when computing h // i ≤ h < j - v := cmp(item, n.items[h]) + v := bcmp(item, n.items[h]) if v == 0 { return h, true } else if v > 0 { @@ -386,8 +387,8 @@ func (n *node) split(i int) (*FileMetadata, *node) { // Insert inserts a item into the subtree rooted at this node, making sure no // nodes in the subtree exceed maxItems items. -func (n *node) Insert(cmp btreeCmp, item *FileMetadata) error { - i, found := n.find(cmp, item) +func (n *node) Insert(bcmp btreeCmp, item *FileMetadata) error { + i, found := n.find(bcmp, item) if found { // cmp provides a total ordering of the files within a level. // If we're inserting a metadata that's equal to an existing item @@ -404,7 +405,7 @@ func (n *node) Insert(cmp btreeCmp, item *FileMetadata) error { splitLa, splitNode := mut(&n.children[i]).split(maxItems / 2) n.insertAt(i, splitLa, splitNode) - switch cmp := cmp(item, n.items[i]); { + switch cmp := bcmp(item, n.items[i]); { case cmp < 0: // no change, we want first split node case cmp > 0: @@ -418,7 +419,7 @@ func (n *node) Insert(cmp btreeCmp, item *FileMetadata) error { } } - err := mut(&n.children[i]).Insert(cmp, item) + err := mut(&n.children[i]).Insert(bcmp, item) if err == nil { n.subtreeCount++ } @@ -447,8 +448,8 @@ func (n *node) removeMax() *FileMetadata { // Remove removes a item from the subtree rooted at this node. Returns // the item that was removed or nil if no matching item was found. -func (n *node) Remove(cmp btreeCmp, item *FileMetadata) (out *FileMetadata) { - i, found := n.find(cmp, item) +func (n *node) Remove(bcmp btreeCmp, item *FileMetadata) (out *FileMetadata) { + i, found := n.find(bcmp, item) if n.leaf { if found { out, _ = n.removeAt(i) @@ -460,7 +461,7 @@ func (n *node) Remove(cmp btreeCmp, item *FileMetadata) (out *FileMetadata) { if n.children[i].count <= minItems { // Child not large enough to remove from. n.rebalanceOrMerge(i) - return n.Remove(cmp, item) + return n.Remove(bcmp, item) } child := mut(&n.children[i]) if found { @@ -471,7 +472,7 @@ func (n *node) Remove(cmp btreeCmp, item *FileMetadata) (out *FileMetadata) { return out } // File is not in this node and child is large enough to remove from. - out = child.Remove(cmp, item) + out = child.Remove(bcmp, item) if out != nil { n.subtreeCount-- } @@ -635,7 +636,8 @@ func (n *node) verifyInvariants() { // goroutines, but Read operations are. type btree struct { root *node - cmp btreeCmp + cmp base.Compare + bcmp btreeCmp } // Release dereferences and clears the root node of the btree, removing all @@ -678,7 +680,7 @@ func (t *btree) Delete(item *FileMetadata) (obsolete bool) { if t.root == nil || t.root.count == 0 { return false } - if out := mut(&t.root).Remove(t.cmp, item); out != nil { + if out := mut(&t.root).Remove(t.bcmp, item); out != nil { obsolete = out.FileBacking.Unref() == 0 } if invariants.Enabled { @@ -712,7 +714,7 @@ func (t *btree) Insert(item *FileMetadata) error { t.root = newRoot } item.FileBacking.Ref() - err := mut(&t.root).Insert(t.cmp, item) + err := mut(&t.root).Insert(t.bcmp, item) if invariants.Enabled { t.root.verifyInvariants() } @@ -723,7 +725,7 @@ func (t *btree) Insert(item *FileMetadata) error { // iterator after modifications are made to the tree. If modifications are made, // create a new iterator. func (t *btree) Iter() iterator { - return iterator{r: t.root, pos: -1, cmp: t.cmp} + return iterator{r: t.root, pos: -1, cmp: t.bcmp} } // Count returns the number of files contained within the B-Tree. diff --git a/internal/manifest/btree_test.go b/internal/manifest/btree_test.go index 1c4b2ed2cd..c75f33004e 100644 --- a/internal/manifest/btree_test.go +++ b/internal/manifest/btree_test.go @@ -111,7 +111,7 @@ func (n *node) verifyCountAllowed(t *testing.T, root bool) { } func (t *btree) isSorted(tt *testing.T) { - t.root.isSorted(tt, t.cmp) + t.root.isSorted(tt, t.bcmp) } func (n *node) isSorted(t *testing.T, cmp func(*FileMetadata, *FileMetadata) int) { @@ -209,7 +209,7 @@ func checkIter(t *testing.T, it iterator, start, end int, keyMemo map[int]Intern // TestBTree tests basic btree operations. func TestBTree(t *testing.T) { var tr btree - tr.cmp = cmp + tr.bcmp = cmp keyMemo := make(map[int]InternalKey) // With degree == 16 (max-items/node == 31) we need 513 items in order for @@ -269,7 +269,7 @@ func TestIterClone(t *testing.T) { const count = 65536 var tr btree - tr.cmp = cmp + tr.bcmp = cmp keyMemo := make(map[int]InternalKey) for i := 0; i < count; i++ { @@ -295,7 +295,7 @@ func TestIterClone(t *testing.T) { func TestIterCmpEdgeCases(t *testing.T) { var tr btree - tr.cmp = cmp + tr.bcmp = cmp t.Run("empty", func(t *testing.T) { a := tr.Iter() b := tr.Iter() @@ -330,7 +330,7 @@ func TestIterCmpRand(t *testing.T) { const iterCount = 1000 var tr btree - tr.cmp = cmp + tr.bcmp = cmp for i := 0; i < itemCount; i++ { require.NoError(t, tr.Insert(newItem(key(i)))) } @@ -365,7 +365,7 @@ func TestBTreeSeek(t *testing.T) { const count = 513 var tr btree - tr.cmp = cmp + tr.bcmp = cmp for i := 0; i < count; i++ { require.NoError(t, tr.Insert(newItem(key(i*2)))) } @@ -404,7 +404,7 @@ func TestBTreeSeek(t *testing.T) { func TestBTreeInsertDuplicateError(t *testing.T) { var tr btree - tr.cmp = cmp + tr.bcmp = cmp require.NoError(t, tr.Insert(newItem(key(1)))) require.NoError(t, tr.Insert(newItem(key(2)))) require.NoError(t, tr.Insert(newItem(key(3)))) @@ -447,7 +447,7 @@ func TestBTreeCloneConcurrentOperations(t *testing.T) { wg.Add(1) var tr btree - tr.cmp = cmp + tr.bcmp = cmp go populate(&tr, 0) wg.Wait() close(treeC) @@ -516,7 +516,7 @@ func TestIterStack(t *testing.T) { func TestIterEndSentinel(t *testing.T) { var tr btree - tr.cmp = cmp + tr.bcmp = cmp require.NoError(t, tr.Insert(newItem(key(1)))) require.NoError(t, tr.Insert(newItem(key(2)))) require.NoError(t, tr.Insert(newItem(key(3)))) @@ -560,7 +560,7 @@ func TestRandomizedBTree(t *testing.T) { // Use a btree comparator that sorts by file number to make it easier to // prevent duplicates or overlaps. tree := btree{ - cmp: func(a *FileMetadata, b *FileMetadata) int { + bcmp: func(a *FileMetadata, b *FileMetadata) int { return stdcmp.Compare(a.FileNum, b.FileNum) }, } @@ -684,7 +684,7 @@ func BenchmarkBTreeInsert(b *testing.B) { b.ResetTimer() for i := 0; i < b.N; { var tr btree - tr.cmp = cmp + tr.bcmp = cmp for _, item := range insertP { if err := tr.Insert(item); err != nil { b.Fatal(err) @@ -706,7 +706,7 @@ func BenchmarkBTreeDelete(b *testing.B) { for i := 0; i < b.N; { b.StopTimer() var tr btree - tr.cmp = cmp + tr.bcmp = cmp for _, item := range insertP { if err := tr.Insert(item); err != nil { b.Fatal(err) @@ -732,7 +732,7 @@ func BenchmarkBTreeDeleteInsert(b *testing.B) { forBenchmarkSizes(b, func(b *testing.B, count int) { insertP := perm(count) var tr btree - tr.cmp = cmp + tr.bcmp = cmp for _, item := range insertP { if err := tr.Insert(item); err != nil { b.Fatal(err) @@ -755,7 +755,7 @@ func BenchmarkBTreeDeleteInsertCloneOnce(b *testing.B) { forBenchmarkSizes(b, func(b *testing.B, count int) { insertP := perm(count) var tr btree - tr.cmp = cmp + tr.bcmp = cmp for _, item := range insertP { if err := tr.Insert(item); err != nil { b.Fatal(err) @@ -781,8 +781,8 @@ func BenchmarkBTreeDeleteInsertCloneEachTime(b *testing.B) { forBenchmarkSizes(b, func(b *testing.B, count int) { insertP := perm(count) var tr, trRelease btree - tr.cmp = cmp - trRelease.cmp = cmp + tr.bcmp = cmp + trRelease.bcmp = cmp for _, item := range insertP { if err := tr.Insert(item); err != nil { b.Fatal(err) @@ -809,7 +809,7 @@ func BenchmarkBTreeDeleteInsertCloneEachTime(b *testing.B) { // BenchmarkBTreeIter measures the cost of creating a btree iterator. func BenchmarkBTreeIter(b *testing.B) { var tr btree - tr.cmp = cmp + tr.bcmp = cmp for i := 0; i < b.N; i++ { it := tr.Iter() it.first() @@ -823,7 +823,7 @@ func BenchmarkBTreeIterSeekGE(b *testing.B) { forBenchmarkSizes(b, func(b *testing.B, count int) { var keys []InternalKey var tr btree - tr.cmp = cmp + tr.bcmp = cmp for i := 0; i < count; i++ { s := key(i) @@ -857,7 +857,7 @@ func BenchmarkBTreeIterSeekLT(b *testing.B) { forBenchmarkSizes(b, func(b *testing.B, count int) { var keys []InternalKey var tr btree - tr.cmp = cmp + tr.bcmp = cmp for i := 0; i < count; i++ { k := key(i) @@ -896,7 +896,7 @@ func BenchmarkBTreeIterSeekLT(b *testing.B) { // next item in the tree. func BenchmarkBTreeIterNext(b *testing.B) { var tr btree - tr.cmp = cmp + tr.bcmp = cmp const count = 8 << 10 for i := 0; i < count; i++ { @@ -920,7 +920,7 @@ func BenchmarkBTreeIterNext(b *testing.B) { // previous item in the tree. func BenchmarkBTreeIterPrev(b *testing.B) { var tr btree - tr.cmp = cmp + tr.bcmp = cmp const count = 8 << 10 for i := 0; i < count; i++ { diff --git a/internal/manifest/l0_sublevels.go b/internal/manifest/l0_sublevels.go index 04cef07ff2..3185b95b3d 100644 --- a/internal/manifest/l0_sublevels.go +++ b/internal/manifest/l0_sublevels.go @@ -337,7 +337,7 @@ func NewL0Sublevels( // Construct a parallel slice of sublevel B-Trees. // TODO(jackson): Consolidate and only use the B-Trees. for _, sublevelFiles := range s.levelFiles { - tr, ls := makeBTree(btreeCmpSmallestKey(cmp), sublevelFiles) + tr, ls := makeBTree(cmp, btreeCmpSmallestKey(cmp), sublevelFiles) s.Levels = append(s.Levels, ls) tr.Release() } @@ -630,7 +630,7 @@ func (s *L0Sublevels) AddL0Files( // Construct a parallel slice of sublevel B-Trees. // TODO(jackson): Consolidate and only use the B-Trees. for _, sublevel := range updatedSublevels { - tr, ls := makeBTree(btreeCmpSmallestKey(newVal.cmp), newVal.levelFiles[sublevel]) + tr, ls := makeBTree(newVal.cmp, btreeCmpSmallestKey(newVal.cmp), newVal.levelFiles[sublevel]) if sublevel == len(newVal.Levels) { newVal.Levels = append(newVal.Levels, ls) } else { diff --git a/internal/manifest/level_metadata.go b/internal/manifest/level_metadata.go index 7cba73e14a..abe4a13cb3 100644 --- a/internal/manifest/level_metadata.go +++ b/internal/manifest/level_metadata.go @@ -49,7 +49,7 @@ func MakeLevelMetadata(cmp Compare, level int, files []*FileMetadata) LevelMetad } var lm LevelMetadata lm.level = level - lm.tree, _ = makeBTree(bcmp, files) + lm.tree, _ = makeBTree(cmp, bcmp, files) for _, f := range files { lm.totalSize += f.Size if f.Virtual { @@ -60,9 +60,10 @@ func MakeLevelMetadata(cmp Compare, level int, files []*FileMetadata) LevelMetad return lm } -func makeBTree(cmp btreeCmp, files []*FileMetadata) (btree, LevelSlice) { +func makeBTree(cmp base.Compare, bcmp btreeCmp, files []*FileMetadata) (btree, LevelSlice) { var t btree t.cmp = cmp + t.bcmp = bcmp for _, f := range files { t.Insert(f) } @@ -157,7 +158,7 @@ func (lf LevelFile) Slice() LevelSlice { // TODO(jackson): Can we improve this interface or avoid needing to export // a slice constructor like this? func NewLevelSliceSeqSorted(files []*FileMetadata) LevelSlice { - tr, slice := makeBTree(btreeCmpSeqNum, files) + tr, slice := makeBTree(nil, btreeCmpSeqNum, files) tr.Release() slice.verifyInvariants() return slice @@ -168,7 +169,7 @@ func NewLevelSliceSeqSorted(files []*FileMetadata) LevelSlice { // TODO(jackson): Can we improve this interface or avoid needing to export // a slice constructor like this? func NewLevelSliceKeySorted(cmp base.Compare, files []*FileMetadata) LevelSlice { - tr, slice := makeBTree(btreeCmpSmallestKey(cmp), files) + tr, slice := makeBTree(cmp, btreeCmpSmallestKey(cmp), files) tr.Release() slice.verifyInvariants() return slice @@ -179,7 +180,7 @@ func NewLevelSliceKeySorted(cmp base.Compare, files []*FileMetadata) LevelSlice // tests. // TODO(jackson): Update tests to avoid requiring this and remove it. func NewLevelSliceSpecificOrder(files []*FileMetadata) LevelSlice { - tr, slice := makeBTree(btreeCmpSpecificOrder(files), files) + tr, slice := makeBTree(nil, btreeCmpSpecificOrder(files), files) tr.Release() slice.verifyInvariants() return slice diff --git a/internal/manifest/version.go b/internal/manifest/version.go index a179d0382a..1babb573e3 100644 --- a/internal/manifest/version.go +++ b/internal/manifest/version.go @@ -1068,12 +1068,12 @@ func NewVersion( // order to test consistency checking, etc. Once we've constructed the // initial B-Tree, we swap out the btreeCmp for the correct one. // TODO(jackson): Adjust or remove the tests and remove this. - v.Levels[l].tree, _ = makeBTree(btreeCmpSpecificOrder(files[l]), files[l]) + v.Levels[l].tree, _ = makeBTree(comparer.Compare, btreeCmpSpecificOrder(files[l]), files[l]) v.Levels[l].level = l if l == 0 { - v.Levels[l].tree.cmp = btreeCmpSeqNum + v.Levels[l].tree.bcmp = btreeCmpSeqNum } else { - v.Levels[l].tree.cmp = btreeCmpSmallestKey(comparer.Compare) + v.Levels[l].tree.bcmp = btreeCmpSmallestKey(comparer.Compare) } for _, f := range files[l] { v.Levels[l].totalSize += f.Size @@ -1501,7 +1501,7 @@ func (v *Version) Overlaps(level int, bounds base.UserKeyBounds) LevelSlice { if !restart { // Construct a B-Tree containing only the matching items. var tr btree - tr.cmp = v.Levels[level].tree.cmp + tr.bcmp = v.Levels[level].tree.bcmp for i, meta := 0, l0Iter.First(); meta != nil; i, meta = i+1, l0Iter.Next() { if selectedIndices[i] { err := tr.Insert(meta)