Skip to content

Commit

Permalink
*: pass keyspan.Spans by pointer not value
Browse files Browse the repository at this point in the history 
Previously the keyspan.Span type used to represent range deletions and range
keys was passed primarily by value, including through the FragmentIterator
interface. This was convenient when transforming spans (eg, truncating bounds),
but passing them by pointer is enough of a performance win to be worthwhile.

Informs cockroachdb/cockroach#82559.

```
name                                    old time/op    new time/op    delta
IteratorSeekNoRangeKeys/batch=false-10    4.28µs ± 1%    2.90µs ± 4%  -32.27%  (p=0.000 n=19+20)
IteratorSeekNoRangeKeys/batch=true-10     8.78µs ± 1%    5.37µs ± 1%  -38.86%  (p=0.000 n=20+20)

name                                    old alloc/op   new alloc/op   delta
IteratorSeekNoRangeKeys/batch=false-10     16.0B ± 0%     16.0B ± 0%     ~     (all equal)
IteratorSeekNoRangeKeys/batch=true-10       192B ± 0%      128B ± 0%  -33.33%  (p=0.000 n=20+20)
```
  • Loading branch information
@jbowens
jbowens committed Jun 9, 2022
1 parent 8a5e6e6 commit b576609
Show file tree
Hide file tree
Showing 50 changed files with 435 additions and 381 deletions.
4 changes: 2 additions & 2 deletions batch_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -795,7 +795,7 @@ func TestBatchRangeOps(t *testing.T) {

var buf bytes.Buffer
if fragmentIter != nil {
for s := fragmentIter.First(); s.Valid(); s = fragmentIter.Next() {
for s := fragmentIter.First(); s != nil; s = fragmentIter.Next() {
for i := range s.Keys {
s.Keys[i].Trailer = base.MakeTrailer(
s.Keys[i].SeqNum()&^base.InternalKeySeqNumBatch,
Expand Down Expand Up @@ -998,7 +998,7 @@ func scanInternalIterator(w io.Writer, ii internalIterator) {
}

func scanKeyspanIterator(w io.Writer, ki keyspan.FragmentIterator) {
for s := ki.First(); s.Valid(); s = ki.Next() {
for s := ki.First(); s != nil; s = ki.Next() {
fmt.Fprintln(w, s)
}
}
Expand Down
6 changes: 3 additions & 3 deletions compaction.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -603,14 +603,14 @@ func newFlush(
}

updateRangeBounds := func(iter keyspan.FragmentIterator) {
if s := iter.First(); s.Valid() {
if s := iter.First(); s != nil {
if key := s.SmallestKey(); !smallestSet ||
base.InternalCompare(c.cmp, c.smallest, key) > 0 {
smallestSet = true
c.smallest = key.Clone()
}
}
if s := iter.Last(); s.Valid() {
if s := iter.Last(); s != nil {
if key := s.LargestKey(); !largestSet ||
base.InternalCompare(c.cmp, c.largest, key) < 0 {
largestSet = true
Expand Down Expand Up @@ -2194,7 +2194,7 @@ func (d *DB) runCompaction(
// added to the writer, eliding out-of-file range tombstones based
// on sequence number at this stage is difficult, and necessitates
// read-time logic to ignore range tombstones outside file bounds.
if rangedel.Encode(v, tw.Add); err != nil {
if rangedel.Encode(&v, tw.Add); err != nil {
return err
}
}
Expand Down
4 changes: 2 additions & 2 deletions data_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,7 @@ func runBuildCmd(td *datadriven.TestData, d *DB, fs vfs.FS) error {
}

if rdi := b.newRangeDelIter(nil, math.MaxUint64); rdi != nil {
for s := rdi.First(); s.Valid(); s = rdi.Next() {
for s := rdi.First(); s != nil; s = rdi.Next() {
err := rangedel.Encode(s, func(k base.InternalKey, v []byte) error {
k.SetSeqNum(0)
return w.Add(k, v)
Expand All @@ -531,7 +531,7 @@ func runBuildCmd(td *datadriven.TestData, d *DB, fs vfs.FS) error {
}

if rki := b.newRangeKeyIter(nil, math.MaxUint64); rki != nil {
for s := rki.First(); s.Valid(); s = rki.Next() {
for s := rki.First(); s != nil; s = rki.Next() {
for _, k := range s.Keys {
var err error
switch k.Kind() {
Expand Down
18 changes: 9 additions & 9 deletions error_iter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,12 @@ func newErrorKeyspanIter(err error) *errorKeyspanIter {
return &errorKeyspanIter{err: err}
}

func (*errorKeyspanIter) SeekGE(key []byte) keyspan.Span { return keyspan.Span{} }
func (*errorKeyspanIter) SeekLT(key []byte) keyspan.Span { return keyspan.Span{} }
func (*errorKeyspanIter) First() keyspan.Span { return keyspan.Span{} }
func (*errorKeyspanIter) Last() keyspan.Span { return keyspan.Span{} }
func (*errorKeyspanIter) Next() keyspan.Span { return keyspan.Span{} }
func (*errorKeyspanIter) Prev() keyspan.Span { return keyspan.Span{} }
func (i *errorKeyspanIter) Error() error { return i.err }
func (i *errorKeyspanIter) Close() error { return i.err }
func (*errorKeyspanIter) String() string { return "error" }
func (*errorKeyspanIter) SeekGE(key []byte) *keyspan.Span { return nil }
func (*errorKeyspanIter) SeekLT(key []byte) *keyspan.Span { return nil }
func (*errorKeyspanIter) First() *keyspan.Span { return nil }
func (*errorKeyspanIter) Last() *keyspan.Span { return nil }
func (*errorKeyspanIter) Next() *keyspan.Span { return nil }
func (*errorKeyspanIter) Prev() *keyspan.Span { return nil }
func (i *errorKeyspanIter) Error() error { return i.err }
func (i *errorKeyspanIter) Close() error { return i.err }
func (*errorKeyspanIter) String() string { return "error" }
6 changes: 3 additions & 3 deletions get_iter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ type getIter struct {
key []byte
iter internalIterator
rangeDelIter keyspan.FragmentIterator
tombstone keyspan.Span
tombstone *keyspan.Span
levelIter levelIter
level int
batch *Batch
Expand Down Expand Up @@ -91,7 +91,7 @@ func (g *getIter) Next() (*InternalKey, []byte) {

if g.iterKey != nil {
key := g.iterKey
if g.tombstone.CoversAt(g.snapshot, key.SeqNum()) {
if g.tombstone != nil && g.tombstone.CoversAt(g.snapshot, key.SeqNum()) {
// We have a range tombstone covering this key. Rather than return a
// point or range deletion here, we return false and close our
// internal iterator which will make Valid() return false,
Expand Down Expand Up @@ -133,7 +133,7 @@ func (g *getIter) Next() (*InternalKey, []byte) {

// If we have a tombstone from a previous level it is guaranteed to delete
// keys in lower levels.
if g.tombstone.VisibleAt(g.snapshot) {
if g.tombstone != nil && g.tombstone.VisibleAt(g.snapshot) {
return nil, nil
}

Expand Down
12 changes: 6 additions & 6 deletions ingest.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func ingestLoad1(
if iter != nil {
defer iter.Close()
var smallest InternalKey
if s := iter.First(); s.Valid() {
if s := iter.First(); s != nil {
key := s.SmallestKey()
if err := ingestValidateKey(opts, &key); err != nil {
return nil, err
Expand All @@ -136,7 +136,7 @@ func ingestLoad1(
if err := iter.Error(); err != nil {
return nil, err
}
if s := iter.Last(); s.Valid() {
if s := iter.Last(); s != nil {
k := s.SmallestKey()
if err := ingestValidateKey(opts, &k); err != nil {
return nil, err
Expand All @@ -155,7 +155,7 @@ func ingestLoad1(
if iter != nil {
defer iter.Close()
var smallest InternalKey
if s := iter.First(); s.Valid() {
if s := iter.First(); s != nil {
key := s.SmallestKey()
if err := ingestValidateKey(opts, &key); err != nil {
return nil, err
Expand All @@ -165,7 +165,7 @@ func ingestLoad1(
if err := iter.Error(); err != nil {
return nil, err
}
if s := iter.Last(); s.Valid() {
if s := iter.Last(); s != nil {
k := s.SmallestKey()
if err := ingestValidateKey(opts, &k); err != nil {
return nil, err
Expand Down Expand Up @@ -412,10 +412,10 @@ func overlapWithIterator(
}
rangeDelItr := *rangeDelIter
rangeDel := rangeDelItr.SeekLT(meta.Smallest.UserKey)
if !rangeDel.Valid() {
if rangeDel == nil {
rangeDel = rangeDelItr.Next()
}
for ; rangeDel.Valid(); rangeDel = rangeDelItr.Next() {
for ; rangeDel != nil; rangeDel = rangeDelItr.Next() {
key := rangeDel.SmallestKey()
c := sstableKeyCompare(cmp, key, meta.Largest)
if c > 0 {
Expand Down
6 changes: 4 additions & 2 deletions ingest_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ func TestIngestLoad(t *testing.T) {
for _, data := range strings.Split(td.Input, "\n") {
if strings.HasPrefix(data, "rangekey: ") {
data = strings.TrimPrefix(data, "rangekey: ")
err := rangekey.Encode(keyspan.ParseSpan(data), w.AddRangeKey)
s := keyspan.ParseSpan(data)
err := rangekey.Encode(&s, w.AddRangeKey)
if err != nil {
return err.Error()
}
Expand Down Expand Up @@ -1275,7 +1276,8 @@ func TestIngest_UpdateSequenceNumber(t *testing.T) {
for _, data := range strings.Split(input, "\n") {
if strings.HasPrefix(data, "rangekey: ") {
data = strings.TrimPrefix(data, "rangekey: ")
err := rangekey.Encode(keyspan.ParseSpan(data), w.AddRangeKey)
s := keyspan.ParseSpan(data)
err := rangekey.Encode(&s, w.AddRangeKey)
if err != nil {
return nil, err
}
Expand Down
Loading

0 comments on commit b576609

Please sign in to comment.