Skip to content

Commit

Permalink
sstable: fix value block closure allocation
Browse files Browse the repository at this point in the history 
Fix an allocation introduced when intializing a rowblk.Iter over a table that
contains value blocks.

```
goos: darwin
goarch: arm64
pkg: github.com/cockroachdb/pebble/sstable
                                                                       │   old.txt   │              new.txt               │
                                                                       │   sec/op    │   sec/op     vs base               │
IteratorScanManyVersions/format=(Pebble,v4)/cache-size=20MB/NewIter-10   681.8n ± 2%   659.0n ± 1%  -3.35% (p=0.000 n=20)

                                                                       │  old.txt   │              new.txt              │
                                                                       │    B/op    │    B/op     vs base               │
IteratorScanManyVersions/format=(Pebble,v4)/cache-size=20MB/NewIter-10   304.0 ± 0%   288.0 ± 0%  -5.26% (p=0.000 n=20)

                                                                       │  old.txt   │              new.txt               │
                                                                       │ allocs/op  │ allocs/op   vs base                │
IteratorScanManyVersions/format=(Pebble,v4)/cache-size=20MB/NewIter-10   2.000 ± 0%   1.000 ± 0%  -50.00% (p=0.000 n=20)
```
  • Loading branch information
@jbowens
jbowens committed Aug 12, 2024
1 parent 791b374 commit 9a4f0c8
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 26 deletions.
6 changes: 6 additions & 0 deletions sstable/block/kv.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,9 @@ func InPlaceValuePrefix(setHasSameKeyPrefix bool) ValuePrefix {
}
return prefix
}

// GetLazyValueForPrefixAndValueHandler is an interface for getting a LazyValue
// from a value prefix and value.
type GetLazyValueForPrefixAndValueHandler interface {
GetLazyValueForPrefixAndValueHandle(handle []byte) base.LazyValue
}
2 changes: 1 addition & 1 deletion sstable/reader_iter_single_lvl.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ func (i *singleLevelIterator) init(
vbih: r.valueBIH,
stats: stats,
}
i.data.SetGetLazyValue(i.vbReader.getLazyValueForPrefixAndValueHandle)
i.data.SetGetLazyValuer(i.vbReader)
i.vbRH = objstorageprovider.UsePreallocatedReadHandle(r.readable, objstorage.NoReadBefore, &i.vbRHPrealloc)
}
i.data.SetHasValuePrefix(true)
Expand Down
9 changes: 8 additions & 1 deletion sstable/reader_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2153,7 +2153,7 @@ func BenchmarkIteratorScanManyVersions(b *testing.B) {
require.NoError(b, err)
return r
}
for _, format := range []TableFormat{TableFormatPebblev2, TableFormatPebblev3} {
for _, format := range []TableFormat{TableFormatPebblev2, TableFormatPebblev3, TableFormatPebblev4} {
b.Run(fmt.Sprintf("format=%s", format.String()), func(b *testing.B) {
// 150MiB results in a high cache hit rate for both formats. 20MiB
// results in a high cache hit rate for the data blocks in
Expand All @@ -2165,6 +2165,13 @@ func BenchmarkIteratorScanManyVersions(b *testing.B) {
defer func() {
require.NoError(b, r.Close())
}()
b.Run("NewIter", func(b *testing.B) {
for i := 0; i < b.N; i++ {
iter, err := r.NewIter(NoTransforms, nil, nil)
require.NoError(b, err)
require.NoError(b, iter.Close())
}
})
for _, readValue := range []bool{false, true} {
b.Run(fmt.Sprintf("read-value=%t", readValue), func(b *testing.B) {
iter, err := r.NewIter(NoTransforms, nil, nil)
Expand Down
45 changes: 23 additions & 22 deletions sstable/rowblk/rowblk_iter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ type Iter struct {
// for block iteration for already loaded blocks.
firstUserKey []byte
lazyValueHandling struct {
getLazyValue func([]byte) base.LazyValue
getValue block.GetLazyValueForPrefixAndValueHandler
hasValuePrefix bool
}
synthSuffixBuf []byte
Expand Down Expand Up @@ -271,10 +271,11 @@ func (i *Iter) SetHasValuePrefix(hasValuePrefix bool) {
i.lazyValueHandling.hasValuePrefix = hasValuePrefix
}

// SetGetLazyValue sets the function the iterator should use to get lazy values
// when the value encodes a value prefix.
func (i *Iter) SetGetLazyValue(fn func([]byte) base.LazyValue) {
i.lazyValueHandling.getLazyValue = fn
// SetGetLazyValuer sets the value block reader the iterator should use to get
// lazy values when the value encodes a value prefix.
func (i *Iter) SetGetLazyValuer(g block.GetLazyValueForPrefixAndValueHandler) {
i.lazyValueHandling.getValue = g

}

// Handle returns the underlying block buffer handle, if the iterator was
Expand Down Expand Up @@ -683,10 +684,10 @@ func (i *Iter) SeekGE(key []byte, flags base.SeekGEFlags) *base.InternalKV {
if !i.lazyValueHandling.hasValuePrefix ||
i.ikv.K.Kind() != base.InternalKeyKindSet {
i.ikv.V = base.MakeInPlaceValue(i.val)
} else if i.lazyValueHandling.getLazyValue == nil || !block.ValuePrefix(i.val[0]).IsValueHandle() {
} else if i.lazyValueHandling.getValue == nil || !block.ValuePrefix(i.val[0]).IsValueHandle() {
i.ikv.V = base.MakeInPlaceValue(i.val[1:])
} else {
i.ikv.V = i.lazyValueHandling.getLazyValue(i.val)
i.ikv.V = i.lazyValueHandling.getValue.GetLazyValueForPrefixAndValueHandle(i.val)
}
return &i.ikv
}
Expand Down Expand Up @@ -967,10 +968,10 @@ func (i *Iter) SeekLT(key []byte, flags base.SeekLTFlags) *base.InternalKV {
if !i.lazyValueHandling.hasValuePrefix ||
i.ikv.K.Kind() != base.InternalKeyKindSet {
i.ikv.V = base.MakeInPlaceValue(i.val)
} else if i.lazyValueHandling.getLazyValue == nil || !block.ValuePrefix(i.val[0]).IsValueHandle() {
} else if i.lazyValueHandling.getValue == nil || !block.ValuePrefix(i.val[0]).IsValueHandle() {
i.ikv.V = base.MakeInPlaceValue(i.val[1:])
} else {
i.ikv.V = i.lazyValueHandling.getLazyValue(i.val)
i.ikv.V = i.lazyValueHandling.getValue.GetLazyValueForPrefixAndValueHandle(i.val)
}
return &i.ikv
}
Expand All @@ -996,10 +997,10 @@ func (i *Iter) First() *base.InternalKV {
if !i.lazyValueHandling.hasValuePrefix ||
i.ikv.K.Kind() != base.InternalKeyKindSet {
i.ikv.V = base.MakeInPlaceValue(i.val)
} else if i.lazyValueHandling.getLazyValue == nil || !block.ValuePrefix(i.val[0]).IsValueHandle() {
} else if i.lazyValueHandling.getValue == nil || !block.ValuePrefix(i.val[0]).IsValueHandle() {
i.ikv.V = base.MakeInPlaceValue(i.val[1:])
} else {
i.ikv.V = i.lazyValueHandling.getLazyValue(i.val)
i.ikv.V = i.lazyValueHandling.getValue.GetLazyValueForPrefixAndValueHandle(i.val)
}
return &i.ikv
}
Expand Down Expand Up @@ -1042,10 +1043,10 @@ func (i *Iter) Last() *base.InternalKV {
if !i.lazyValueHandling.hasValuePrefix ||
i.ikv.K.Kind() != base.InternalKeyKindSet {
i.ikv.V = base.MakeInPlaceValue(i.val)
} else if i.lazyValueHandling.getLazyValue == nil || !block.ValuePrefix(i.val[0]).IsValueHandle() {
} else if i.lazyValueHandling.getValue == nil || !block.ValuePrefix(i.val[0]).IsValueHandle() {
i.ikv.V = base.MakeInPlaceValue(i.val[1:])
} else {
i.ikv.V = i.lazyValueHandling.getLazyValue(i.val)
i.ikv.V = i.lazyValueHandling.getValue.GetLazyValueForPrefixAndValueHandle(i.val)
}
return &i.ikv
}
Expand Down Expand Up @@ -1101,10 +1102,10 @@ start:
if !i.lazyValueHandling.hasValuePrefix ||
i.ikv.K.Kind() != base.InternalKeyKindSet {
i.ikv.V = base.MakeInPlaceValue(i.val)
} else if i.lazyValueHandling.getLazyValue == nil || !block.ValuePrefix(i.val[0]).IsValueHandle() {
} else if i.lazyValueHandling.getValue == nil || !block.ValuePrefix(i.val[0]).IsValueHandle() {
i.ikv.V = base.MakeInPlaceValue(i.val[1:])
} else {
i.ikv.V = i.lazyValueHandling.getLazyValue(i.val)
i.ikv.V = i.lazyValueHandling.getValue.GetLazyValueForPrefixAndValueHandle(i.val)
}
return &i.ikv
}
Expand Down Expand Up @@ -1392,10 +1393,10 @@ func (i *Iter) nextPrefixV3(succKey []byte) *base.InternalKV {
}
if i.ikv.K.Kind() != base.InternalKeyKindSet {
i.ikv.V = base.MakeInPlaceValue(i.val)
} else if i.lazyValueHandling.getLazyValue == nil || !block.ValuePrefix(i.val[0]).IsValueHandle() {
} else if i.lazyValueHandling.getValue == nil || !block.ValuePrefix(i.val[0]).IsValueHandle() {
i.ikv.V = base.MakeInPlaceValue(i.val[1:])
} else {
i.ikv.V = i.lazyValueHandling.getLazyValue(i.val)
i.ikv.V = i.lazyValueHandling.getValue.GetLazyValueForPrefixAndValueHandle(i.val)
}
return &i.ikv
}
Expand Down Expand Up @@ -1450,10 +1451,10 @@ start:
if !i.lazyValueHandling.hasValuePrefix ||
i.ikv.K.Kind() != base.InternalKeyKindSet {
i.ikv.V = base.MakeInPlaceValue(i.val)
} else if i.lazyValueHandling.getLazyValue == nil || !block.ValuePrefix(i.val[0]).IsValueHandle() {
} else if i.lazyValueHandling.getValue == nil || !block.ValuePrefix(i.val[0]).IsValueHandle() {
i.ikv.V = base.MakeInPlaceValue(i.val[1:])
} else {
i.ikv.V = i.lazyValueHandling.getLazyValue(i.val)
i.ikv.V = i.lazyValueHandling.getValue.GetLazyValueForPrefixAndValueHandle(i.val)
}
return &i.ikv
}
Expand Down Expand Up @@ -1531,10 +1532,10 @@ start:
if !i.lazyValueHandling.hasValuePrefix ||
i.ikv.K.Kind() != base.InternalKeyKindSet {
i.ikv.V = base.MakeInPlaceValue(i.val)
} else if i.lazyValueHandling.getLazyValue == nil || !block.ValuePrefix(i.val[0]).IsValueHandle() {
} else if i.lazyValueHandling.getValue == nil || !block.ValuePrefix(i.val[0]).IsValueHandle() {
i.ikv.V = base.MakeInPlaceValue(i.val[1:])
} else {
i.ikv.V = i.lazyValueHandling.getLazyValue(i.val)
i.ikv.V = i.lazyValueHandling.getValue.GetLazyValueForPrefixAndValueHandle(i.val)
}
return &i.ikv
}
Expand Down Expand Up @@ -1567,7 +1568,7 @@ func (i *Iter) Close() error {
i.handle = block.BufferHandle{}
i.val = nil
i.ikv = base.InternalKV{}
i.lazyValueHandling.getLazyValue = nil
i.lazyValueHandling.getValue = nil
return nil
}

Expand Down
2 changes: 1 addition & 1 deletion sstable/value_block.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -751,7 +751,7 @@ type valueBlockReader struct {
bufToMangle []byte
}

func (r *valueBlockReader) getLazyValueForPrefixAndValueHandle(handle []byte) base.LazyValue {
func (r *valueBlockReader) GetLazyValueForPrefixAndValueHandle(handle []byte) base.LazyValue {
fetcher := &r.lazyFetcher
valLen, h := decodeLenFromValueHandle(handle[1:])
*fetcher = base.LazyFetcher{
Expand Down
2 changes: 1 addition & 1 deletion sstable/writer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ func TestWriterWithValueBlocks(t *testing.T) {
}
forceIgnoreValueBlocks := func(i *singleLevelIterator) {
i.vbReader = nil
i.data.SetGetLazyValue(nil)
i.data.SetGetLazyValuer(nil)
i.data.SetHasValuePrefix(false)
}
switch i := origIter.(type) {
Expand Down

0 comments on commit 9a4f0c8

Please sign in to comment.