Skip to content

Commit

Permalink
sstable: improve TrivialReaderProvider
Browse files Browse the repository at this point in the history
We improve the `TrivialReaderProvider` implementation to avoid
allocation and unexport the type.
  • Loading branch information
RaduBerinde committed Jul 30, 2024
1 parent 034fd8e commit 3fded19
Show file tree
Hide file tree
Showing 9 changed files with 30 additions and 23 deletions.
2 changes: 1 addition & 1 deletion external_iterator.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ func createExternalPointIter(ctx context.Context, it *Iterator) (topLevelIterato
ctx, transforms, it.opts.LowerBound, it.opts.UpperBound, nil, /* BlockPropertiesFilterer */
sstable.NeverUseFilterBlock,
&it.stats.InternalStats, it.opts.CategoryAndQoS, nil,
sstable.TrivialReaderProvider{Reader: r})
sstable.MakeTrivialReaderProvider(r))
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion level_iter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ func (lt *levelIterTest) newIters(
iter, err := lt.readers[file.FileNum].NewIterWithBlockPropertyFiltersAndContextEtc(
ctx, transforms,
opts.LowerBound, opts.UpperBound, nil, sstable.AlwaysUseFilterBlock, iio.stats, sstable.CategoryAndQoS{},
nil, sstable.TrivialReaderProvider{Reader: lt.readers[file.FileNum]})
nil, sstable.MakeTrivialReaderProvider(lt.readers[file.FileNum]))
if err != nil {
return iterSet{}, errors.CombineErrors(err, set.CloseAll())
}
Expand Down
2 changes: 1 addition & 1 deletion merging_iter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ func TestMergingIterCornerCases(t *testing.T) {
set.point, err = r.NewIterWithBlockPropertyFilters(
sstable.NoTransforms,
opts.GetLowerBound(), opts.GetUpperBound(), nil, sstable.AlwaysUseFilterBlock, iio.stats,
sstable.CategoryAndQoS{}, nil, sstable.TrivialReaderProvider{Reader: r})
sstable.CategoryAndQoS{}, nil, sstable.MakeTrivialReaderProvider(r))
if err != nil {
return iterSet{}, errors.CombineErrors(err, set.CloseAll())
}
Expand Down
4 changes: 2 additions & 2 deletions sstable/block_property_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -973,7 +973,7 @@ func TestBlockProperties(t *testing.T) {
}
iter, err := r.NewIterWithBlockPropertyFilters(
NoTransforms, lower, upper, filterer, NeverUseFilterBlock, &stats,
CategoryAndQoS{}, nil, TrivialReaderProvider{Reader: r})
CategoryAndQoS{}, nil, MakeTrivialReaderProvider(r))
if err != nil {
return err.Error()
}
Expand Down Expand Up @@ -1055,7 +1055,7 @@ func TestBlockProperties_BoundLimited(t *testing.T) {
}
iter, err := r.NewIterWithBlockPropertyFilters(
NoTransforms, lower, upper, filterer, NeverUseFilterBlock, &stats,
CategoryAndQoS{}, nil, TrivialReaderProvider{Reader: r})
CategoryAndQoS{}, nil, MakeTrivialReaderProvider(r))
if err != nil {
return err.Error()
}
Expand Down
2 changes: 1 addition & 1 deletion sstable/random_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func runErrorInjectionTest(t *testing.T, seed int64) {
&stats,
CategoryAndQoS{},
nil, /* CategoryStatsCollector */
TrivialReaderProvider{r},
MakeTrivialReaderProvider(r),
)
require.NoError(t, err)
defer it.Close()
Expand Down
2 changes: 1 addition & 1 deletion sstable/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ func (r *Reader) NewIter(transforms IterTransforms, lower, upper []byte) (Iterat
// likely isn't a cache set up.
return r.NewIterWithBlockPropertyFilters(
transforms, lower, upper, nil, AlwaysUseFilterBlock,
nil /* stats */, CategoryAndQoS{}, nil /* statsCollector */, TrivialReaderProvider{Reader: r})
nil /* stats */, CategoryAndQoS{}, nil /* statsCollector */, MakeTrivialReaderProvider(r))
}

// NewCompactionIter returns an iterator similar to NewIter but it also increments
Expand Down
4 changes: 2 additions & 2 deletions sstable/reader_iter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func TestIteratorErrorOnInit(t *testing.T) {
&stats,
CategoryAndQoS{},
nil, /* statsCollector */
TrivialReaderProvider{Reader: r},
MakeTrivialReaderProvider(r),
&pool,
)
require.Error(t, err)
Expand All @@ -78,7 +78,7 @@ func TestIteratorErrorOnInit(t *testing.T) {
&stats,
CategoryAndQoS{},
nil, /* statsCollector */
TrivialReaderProvider{Reader: r},
MakeTrivialReaderProvider(r),
&pool,
)
require.Error(t, err)
Expand Down
12 changes: 6 additions & 6 deletions sstable/reader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ func runVirtualReaderTest(t *testing.T, path string, blockSize, indexBlockSize i
transforms := IterTransforms{SyntheticSuffix: syntheticSuffix}
iter, err := v.NewIterWithBlockPropertyFiltersAndContextEtc(
context.Background(), transforms, lower, upper, filterer, NeverUseFilterBlock,
&stats, CategoryAndQoS{}, nil, TrivialReaderProvider{Reader: r})
&stats, CategoryAndQoS{}, nil, MakeTrivialReaderProvider(r))
if err != nil {
return err.Error()
}
Expand Down Expand Up @@ -837,7 +837,7 @@ func runTestReader(t *testing.T, o WriterOptions, dir string, r *Reader, printVa
&stats,
CategoryAndQoS{},
nil,
TrivialReaderProvider{Reader: r},
MakeTrivialReaderProvider(r),
)
if err != nil {
return err.Error()
Expand Down Expand Up @@ -977,7 +977,7 @@ func TestCompactionIteratorSetupForCompaction(t *testing.T) {
var pool block.BufferPool
pool.Init(5)
citer, err := r.NewCompactionIter(
NoTransforms, CategoryAndQoS{}, nil, TrivialReaderProvider{Reader: r}, &pool)
NoTransforms, CategoryAndQoS{}, nil, MakeTrivialReaderProvider(r), &pool)
require.NoError(t, err)
switch i := citer.(type) {
case *compactionIterator:
Expand Down Expand Up @@ -1033,7 +1033,7 @@ func TestReadaheadSetupForV3TablesWithMultipleVersions(t *testing.T) {
var pool block.BufferPool
pool.Init(5)
citer, err := r.NewCompactionIter(
NoTransforms, CategoryAndQoS{}, nil, TrivialReaderProvider{Reader: r}, &pool)
NoTransforms, CategoryAndQoS{}, nil, MakeTrivialReaderProvider(r), &pool)
require.NoError(t, err)
defer citer.Close()
i := citer.(*compactionIterator)
Expand Down Expand Up @@ -1328,7 +1328,7 @@ func TestRandomizedPrefixSuffixRewriter(t *testing.T) {
block.IterTransforms{SyntheticSuffix: syntheticSuffix, SyntheticPrefix: syntheticPrefix},
nil, nil, nil,
AlwaysUseFilterBlock, nil, CategoryAndQoS{}, nil,
TrivialReaderProvider{Reader: eReader}, &virtualState{
MakeTrivialReaderProvider(eReader), &virtualState{
lower: base.MakeInternalKey([]byte("_"), base.SeqNumMax, base.InternalKeyKindSet),
upper: base.MakeRangeDeleteSentinelKey([]byte("~~~~~~~~~~~~~~~~")),
})
Expand Down Expand Up @@ -2451,7 +2451,7 @@ func BenchmarkIteratorScanObsolete(b *testing.B) {
iter, err := r.NewIterWithBlockPropertyFiltersAndContextEtc(
context.Background(), transforms, nil, nil, filterer,
AlwaysUseFilterBlock, nil, CategoryAndQoS{}, nil,
TrivialReaderProvider{Reader: r})
MakeTrivialReaderProvider(r))
require.NoError(b, err)
b.ResetTimer()
for i := 0; i < b.N; i++ {
Expand Down
23 changes: 15 additions & 8 deletions sstable/value_block.go
Original file line number Diff line number Diff line change
Expand Up @@ -719,21 +719,28 @@ type ReaderProvider interface {
Close()
}

// TrivialReaderProvider implements ReaderProvider for a Reader that will
// outlive the top-level iterator in the iterator tree.
type TrivialReaderProvider struct {
*Reader
// MakeTrivialReaderProvider creates a ReaderProvider which always returns the
// given reader. It should be used when the Reader will outlive the iterator
// tree.
func MakeTrivialReaderProvider(r *Reader) ReaderProvider {
return (*trivialReaderProvider)(r)
}

var _ ReaderProvider = TrivialReaderProvider{}
// trivialReaderProvider implements ReaderProvider for a Reader that will
// outlive the top-level iterator in the iterator tree.
//
// Defining the type in this manner (as opposed to a struct) avoids allocation.
type trivialReaderProvider Reader

var _ ReaderProvider = (*trivialReaderProvider)(nil)

// GetReader implements ReaderProvider.
func (trp TrivialReaderProvider) GetReader(ctx context.Context) (*Reader, error) {
return trp.Reader, nil
func (trp *trivialReaderProvider) GetReader(ctx context.Context) (*Reader, error) {
return (*Reader)(trp), nil
}

// Close implements ReaderProvider.
func (trp TrivialReaderProvider) Close() {}
func (trp *trivialReaderProvider) Close() {}

// valueBlockReader is used to retrieve values in value
// blocks. It is used when the sstable was written with
Expand Down

0 comments on commit 3fded19

Please sign in to comment.