Skip to content

Commit d2d78bb

Browse files
committed
db: remove levelIter.{cmp,split}
``` goos: darwin goarch: arm64 pkg: github.com/cockroachdb/pebble cpu: Apple M1 Pro │ prev.txt │ head.txt │ │ sec/op │ sec/op vs base │ NewIterClose/1-10 1.226µ ± 1% 1.212µ ± 1% -1.14% (p=0.041 n=10) NewIterClose/10-10 6.407µ ± 0% 6.403µ ± 0% ~ (p=0.092 n=10) NewIterClose/100-10 91.84µ ± 1% 91.64µ ± 1% ~ (p=0.971 n=10) geomean 8.968µ 8.926µ -0.47% ```
1 parent 17b8cef commit d2d78bb

File tree

2 files changed

+22
-26
lines changed

2 files changed

+22
-26
lines changed

level_iter.go

Lines changed: 21 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,6 @@ type levelIter struct {
3636
ctx context.Context
3737
logger Logger
3838
comparer *Comparer
39-
cmp Compare
40-
split Split
4139
// The lower/upper bounds for iteration as specified at creation or the most
4240
// recent call to SetBounds.
4341
lower []byte
@@ -161,8 +159,6 @@ func (l *levelIter) init(
161159
l.tableOpts.layer = l.layer
162160
l.tableOpts.snapshotForHideObsoletePoints = opts.snapshotForHideObsoletePoints
163161
l.comparer = comparer
164-
l.cmp = comparer.Compare
165-
l.split = comparer.Split
166162
l.iterFile = nil
167163
l.newIters = newIters
168164
l.files = files
@@ -204,11 +200,11 @@ func (l *levelIter) maybeTriggerCombinedIteration(file *manifest.TableMetadata,
204200
return
205201
}
206202

207-
if l.upper != nil && l.cmp(file.RangeKeyBounds.SmallestUserKey(), l.upper) >= 0 {
203+
if l.upper != nil && l.comparer.Compare(file.RangeKeyBounds.SmallestUserKey(), l.upper) >= 0 {
208204
// Range key bounds are above the upper iteration bound.
209205
return
210206
}
211-
if l.lower != nil && l.cmp(file.RangeKeyBounds.LargestUserKey(), l.lower) <= 0 {
207+
if l.lower != nil && l.comparer.Compare(file.RangeKeyBounds.LargestUserKey(), l.lower) <= 0 {
212208
// Range key bounds are below the lower iteration bound.
213209
return
214210
}
@@ -238,14 +234,14 @@ func (l *levelIter) maybeTriggerCombinedIteration(file *manifest.TableMetadata,
238234
if !l.combinedIterState.triggered {
239235
l.combinedIterState.triggered = true
240236
l.combinedIterState.key = file.RangeKeyBounds.SmallestUserKey()
241-
} else if l.cmp(l.combinedIterState.key, file.RangeKeyBounds.SmallestUserKey()) > 0 {
237+
} else if l.comparer.Compare(l.combinedIterState.key, file.RangeKeyBounds.SmallestUserKey()) > 0 {
242238
l.combinedIterState.key = file.RangeKeyBounds.SmallestUserKey()
243239
}
244240
case -1:
245241
if !l.combinedIterState.triggered {
246242
l.combinedIterState.triggered = true
247243
l.combinedIterState.key = file.RangeKeyBounds.LargestUserKey()
248-
} else if l.cmp(l.combinedIterState.key, file.RangeKeyBounds.LargestUserKey()) < 0 {
244+
} else if l.comparer.Compare(l.combinedIterState.key, file.RangeKeyBounds.LargestUserKey()) < 0 {
249245
l.combinedIterState.key = file.RangeKeyBounds.LargestUserKey()
250246
}
251247
}
@@ -315,7 +311,7 @@ func (l *levelIter) findFileGE(key []byte, flags base.SeekGEFlags) *manifest.Tab
315311
if nextInsteadOfSeek {
316312
m = l.iterFile
317313
} else {
318-
m = l.files.SeekGE(l.cmp, key)
314+
m = l.files.SeekGE(l.comparer.Compare, key)
319315
}
320316
// The below loop has a bit of an unusual organization. There are several
321317
// conditions under which we need to Next to a later file. If none of those
@@ -348,14 +344,14 @@ func (l *levelIter) findFileGE(key []byte, flags base.SeekGEFlags) *manifest.Tab
348344
//
349345
// If the file does not contain point keys ≥ `key`, next to continue
350346
// looking for a file that does.
351-
if (m.HasRangeKeys || nextInsteadOfSeek) && l.cmp(m.PointKeyBounds.LargestUserKey(), key) < 0 {
347+
if (m.HasRangeKeys || nextInsteadOfSeek) && l.comparer.Compare(m.PointKeyBounds.LargestUserKey(), key) < 0 {
352348
// If nextInsteadOfSeek is set and nextsUntilSeek is non-negative,
353349
// the iterator has been nexting hoping to discover the relevant
354350
// file without seeking. It's exhausted the allotted nextsUntilSeek
355351
// and should seek to the sought key.
356352
if nextInsteadOfSeek && nextsUntilSeek == 0 {
357353
nextInsteadOfSeek = false
358-
m = l.files.SeekGE(l.cmp, key)
354+
m = l.files.SeekGE(l.comparer.Compare, key)
359355
continue
360356
} else if nextsUntilSeek > 0 {
361357
nextsUntilSeek--
@@ -374,7 +370,7 @@ func (l *levelIter) findFileGE(key []byte, flags base.SeekGEFlags) *manifest.Tab
374370
// a table which can't possibly contain the target key and is required
375371
// for correctness by mergingIter.SeekGE (see the comment in that
376372
// function).
377-
if m.PointKeyBounds.Largest().IsExclusiveSentinel() && l.cmp(m.PointKeyBounds.LargestUserKey(), key) == 0 {
373+
if m.PointKeyBounds.Largest().IsExclusiveSentinel() && l.comparer.Compare(m.PointKeyBounds.LargestUserKey(), key) == 0 {
378374
m = l.files.Next()
379375
continue
380376
}
@@ -406,7 +402,7 @@ func (l *levelIter) findFileLT(key []byte, flags base.SeekLTFlags) *manifest.Tab
406402
if prevInsteadOfSeek {
407403
m = l.iterFile
408404
} else {
409-
m = l.files.SeekLT(l.cmp, key)
405+
m = l.files.SeekLT(l.comparer.Compare, key)
410406
}
411407
// The below loop has a bit of an unusual organization. There are several
412408
// conditions under which we need to Prev to a previous file. If none of
@@ -439,7 +435,7 @@ func (l *levelIter) findFileLT(key []byte, flags base.SeekLTFlags) *manifest.Tab
439435
//
440436
// If the file does not contain point keys < `key`, prev to continue
441437
// looking for a file that does.
442-
if (m.HasRangeKeys || prevInsteadOfSeek) && l.cmp(m.PointKeyBounds.SmallestUserKey(), key) >= 0 {
438+
if (m.HasRangeKeys || prevInsteadOfSeek) && l.comparer.Compare(m.PointKeyBounds.SmallestUserKey(), key) >= 0 {
443439
m = l.files.Prev()
444440
continue
445441
}
@@ -456,11 +452,11 @@ func (l *levelIter) findFileLT(key []byte, flags base.SeekLTFlags) *manifest.Tab
456452
func (l *levelIter) initTableBounds(f *manifest.TableMetadata) int {
457453
l.tableOpts.LowerBound = l.lower
458454
if l.tableOpts.LowerBound != nil {
459-
if l.cmp(f.PointKeyBounds.LargestUserKey(), l.tableOpts.LowerBound) < 0 {
455+
if l.comparer.Compare(f.PointKeyBounds.LargestUserKey(), l.tableOpts.LowerBound) < 0 {
460456
// The largest key in the sstable is smaller than the lower bound.
461457
return -1
462458
}
463-
if l.cmp(l.tableOpts.LowerBound, f.PointKeyBounds.SmallestUserKey()) <= 0 {
459+
if l.comparer.Compare(l.tableOpts.LowerBound, f.PointKeyBounds.SmallestUserKey()) <= 0 {
464460
// The lower bound is smaller or equal to the smallest key in the
465461
// table. Iteration within the table does not need to check the lower
466462
// bound.
@@ -469,12 +465,12 @@ func (l *levelIter) initTableBounds(f *manifest.TableMetadata) int {
469465
}
470466
l.tableOpts.UpperBound = l.upper
471467
if l.tableOpts.UpperBound != nil {
472-
if l.cmp(f.PointKeyBounds.SmallestUserKey(), l.tableOpts.UpperBound) >= 0 {
468+
if l.comparer.Compare(f.PointKeyBounds.SmallestUserKey(), l.tableOpts.UpperBound) >= 0 {
473469
// The smallest key in the sstable is greater than or equal to the upper
474470
// bound.
475471
return 1
476472
}
477-
if l.cmp(l.tableOpts.UpperBound, f.PointKeyBounds.LargestUserKey()) > 0 {
473+
if l.comparer.Compare(l.tableOpts.UpperBound, f.PointKeyBounds.LargestUserKey()) > 0 {
478474
// The upper bound is greater than the largest key in the
479475
// table. Iteration within the table does not need to check the upper
480476
// bound. NB: tableOpts.UpperBound is exclusive and f.PointKeyBounds.Largest() is
@@ -564,7 +560,7 @@ func (l *levelIter) loadFile(file *manifest.TableMetadata, dir int) loadFileRetu
564560
// any keys within the iteration prefix. Loading the next file is
565561
// unnecessary. This has been observed in practice on slow shared
566562
// storage. See #3575.
567-
if l.prefix != nil && l.cmp(l.split.Prefix(file.PointKeyBounds.SmallestUserKey()), l.prefix) > 0 {
563+
if l.prefix != nil && l.comparer.Compare(l.comparer.Split.Prefix(file.PointKeyBounds.SmallestUserKey()), l.prefix) > 0 {
568564
// Note that because l.iter is nil, a subsequent call to
569565
// SeekPrefixGE with TrySeekUsingNext()=true will load the file
570566
// (returning newFileLoaded) and disable TrySeekUsingNext before
@@ -625,18 +621,18 @@ func (l *levelIter) verify(kv *base.InternalKV) *base.InternalKV {
625621
// We allow returning a boundary key that is outside of the lower/upper
626622
// bounds as such keys are always range tombstones which will be skipped
627623
// by the Iterator.
628-
if l.lower != nil && kv != nil && !kv.K.IsExclusiveSentinel() && l.cmp(kv.K.UserKey, l.lower) < 0 {
624+
if l.lower != nil && kv != nil && !kv.K.IsExclusiveSentinel() && l.comparer.Compare(kv.K.UserKey, l.lower) < 0 {
629625
l.logger.Fatalf("levelIter %s: lower bound violation: %s < %s\n%s", l.layer, kv, l.lower, debug.Stack())
630626
}
631-
if l.upper != nil && kv != nil && !kv.K.IsExclusiveSentinel() && l.cmp(kv.K.UserKey, l.upper) > 0 {
627+
if l.upper != nil && kv != nil && !kv.K.IsExclusiveSentinel() && l.comparer.Compare(kv.K.UserKey, l.upper) > 0 {
632628
l.logger.Fatalf("levelIter %s: upper bound violation: %s > %s\n%s", l.layer, kv, l.upper, debug.Stack())
633629
}
634630
}
635631
return kv
636632
}
637633

638634
func (l *levelIter) SeekGE(key []byte, flags base.SeekGEFlags) *base.InternalKV {
639-
if invariants.Enabled && l.lower != nil && l.cmp(key, l.lower) < 0 {
635+
if invariants.Enabled && l.lower != nil && l.comparer.Compare(key, l.lower) < 0 {
640636
panic(errors.AssertionFailedf("levelIter SeekGE to key %q violates lower bound %q", key, l.lower))
641637
}
642638

@@ -662,7 +658,7 @@ func (l *levelIter) SeekGE(key []byte, flags base.SeekGEFlags) *base.InternalKV
662658
}
663659

664660
func (l *levelIter) SeekPrefixGE(prefix, key []byte, flags base.SeekGEFlags) *base.InternalKV {
665-
if invariants.Enabled && l.lower != nil && l.cmp(key, l.lower) < 0 {
661+
if invariants.Enabled && l.lower != nil && l.comparer.Compare(key, l.lower) < 0 {
666662
panic(errors.AssertionFailedf("levelIter SeekGE to key %q violates lower bound %q", key, l.lower))
667663
}
668664
l.err = nil // clear cached iteration error
@@ -691,7 +687,7 @@ func (l *levelIter) SeekPrefixGE(prefix, key []byte, flags base.SeekGEFlags) *ba
691687
}
692688

693689
func (l *levelIter) SeekLT(key []byte, flags base.SeekLTFlags) *base.InternalKV {
694-
if invariants.Enabled && l.upper != nil && l.cmp(key, l.upper) > 0 {
690+
if invariants.Enabled && l.upper != nil && l.comparer.Compare(key, l.upper) > 0 {
695691
panic(errors.AssertionFailedf("levelIter SeekLT to key %q violates upper bound %q", key, l.upper))
696692
}
697693

@@ -853,7 +849,7 @@ func (l *levelIter) skipEmptyFileForward() *base.InternalKV {
853849
// subsequent SeekPrefixGE with TrySeekUsingNext could mistakenly skip
854850
// the file's relevant keys.
855851
if l.prefix != nil {
856-
if l.cmp(l.split.Prefix(l.iterFile.PointKeyBounds.LargestUserKey()), l.prefix) > 0 {
852+
if l.comparer.Compare(l.comparer.Split.Prefix(l.iterFile.PointKeyBounds.LargestUserKey()), l.prefix) > 0 {
857853
l.exhaustedForward()
858854
return nil
859855
}

level_iter_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -422,7 +422,7 @@ func (i *levelIterTestIter) rangeDelSeek(
422422
var t *keyspan.Span
423423
var err error
424424
if dir < 0 {
425-
t, err = keyspan.SeekLE(i.levelIter.cmp, i.rangeDelIter, key)
425+
t, err = keyspan.SeekLE(i.levelIter.comparer.Compare, i.rangeDelIter, key)
426426
} else {
427427
t, err = i.rangeDelIter.SeekGE(key)
428428
}

0 commit comments

Comments
 (0)