Skip to content

Commit

Permalink
sstable: remove ResetForReuse
Browse files Browse the repository at this point in the history
Perform necessary zeroing as a part of Close.

```
goos: darwin
goarch: arm64
pkg: github.com/cockroachdb/pebble/internal/crdbtest
cpu: Apple M1 Pro
                                 │   old.txt   │              new.txt               │
                                 │   sec/op    │   sec/op     vs base               │
RandSeekInSST/v4/single-level-10   724.9n ± 4%   692.2n ± 2%  -4.52% (p=0.001 n=10)
RandSeekInSST/v4/two-level-10      1.454µ ± 7%   1.453µ ± 7%       ~ (p=0.492 n=10)
RandSeekInSST/v5/single-level-10   508.0n ± 5%   489.0n ± 3%  -3.74% (p=0.011 n=10)
RandSeekInSST/v5/two-level-10      897.5n ± 2%   894.9n ± 7%       ~ (p=0.739 n=10)
geomean                            832.6n        814.4n       -2.18%
```
  • Loading branch information
jbowens committed Oct 21, 2024
1 parent 11ff3b6 commit ad898ed
Show file tree
Hide file tree
Showing 10 changed files with 25 additions and 65 deletions.
4 changes: 3 additions & 1 deletion sstable/block/block.go
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,9 @@ type IndexBlockIterator interface {
// false if the index block is exhausted in the reverse direction. A call to
// Prev while already exhausted in the reverse direction is a no-op.
Prev() bool
// Close closes the iterator, releasing any resources it holds.
// Close closes the iterator, releasing any resources it holds. After Close,
// the iterator must be reset such that it could be reused after a call to
// Init or InitHandle.
Close() error
}

Expand Down
7 changes: 0 additions & 7 deletions sstable/colblk/data_block.go
Original file line number Diff line number Diff line change
Expand Up @@ -1051,13 +1051,6 @@ func (i *DataBlockIter) IsDataInvalidated() bool {
return i.d == nil
}

// ResetForReuse resets the iterator for reuse, retaining buffers and
// configuration supplied to InitOnce, to avoid future allocations.
func (i *DataBlockIter) ResetForReuse() {
i.d = nil
i.kv = base.InternalKV{}
}

// IsLowerBound implements the block.DataBlockIterator interface.
func (i *DataBlockIter) IsLowerBound(k []byte) bool {
if i.transforms.SyntheticPrefix.IsSet() {
Expand Down
16 changes: 4 additions & 12 deletions sstable/colblk/index_block.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,17 +246,6 @@ func (i *IndexIter) RowIndex() int {
return i.row
}

// ResetForReuse resets the IndexIter for reuse, retaining buffers to avoid
// future allocations.
func (i *IndexIter) ResetForReuse() {
if invariants.Enabled && i.h != (block.BufferHandle{}) {
panic(errors.AssertionFailedf("IndexIter reset for reuse with non-empty handle"))
}
i.d = nil
i.syntheticPrefix = nil
i.syntheticSuffix = nil
}

// Valid returns true if the iterator is currently positioned at a valid block
// handle.
func (i *IndexIter) Valid() bool {
Expand Down Expand Up @@ -393,6 +382,9 @@ func (i *IndexIter) Prev() bool {
// Close closes the iterator, releasing any resources it holds.
func (i *IndexIter) Close() error {
i.h.Release()
*i = IndexIter{}
i.h = block.BufferHandle{}
i.d = nil
i.syntheticPrefix = nil
i.syntheticSuffix = nil
return nil
}
8 changes: 0 additions & 8 deletions sstable/reader_iter.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,6 @@ import (
type dataBlockIterator[D any] interface {
block.DataBlockIterator

// ResetForReuse resets the data block iterator for reuse, retaining buffers
// to avoid future allocations.
ResetForReuse()

*D // non-interface type constraint element
}

Expand All @@ -43,10 +39,6 @@ type dataBlockIterator[D any] interface {
type indexBlockIterator[I any] interface {
block.IndexBlockIterator

// ResetForReuse resets the index iterator for reuse, retaining buffers to
// avoid future allocations.
ResetForReuse()

*I // non-interface type constraint element
}

Expand Down
2 changes: 0 additions & 2 deletions sstable/reader_iter_single_lvl.go
Original file line number Diff line number Diff line change
Expand Up @@ -399,8 +399,6 @@ const _ uintptr = clearLenColBlocks - clearLen

func (i *singleLevelIterator[I, PI, D, PD]) resetForReuse() {
*(*[clearLen]byte)(unsafe.Pointer(i)) = [clearLen]byte{}
PI(&i.index).ResetForReuse()
PD(&i.data).ResetForReuse()
i.inPool = true
}

Expand Down
1 change: 0 additions & 1 deletion sstable/reader_iter_two_lvl.go
Original file line number Diff line number Diff line change
Expand Up @@ -1047,7 +1047,6 @@ func (i *twoLevelIterator[I, PI, D, PD]) Close() error {
err := i.secondLevel.closeInternal()
i.secondLevel.resetForReuse()
err = firstError(err, PI(&i.topLevelIndex).Close())
PI(&i.topLevelIndex).ResetForReuse()
i.useFilterBlock = false
i.lastBloomFilterMatched = false
if pool != nil {
Expand Down
15 changes: 7 additions & 8 deletions sstable/rowblk/rowblk_fragment_iter.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,14 +285,13 @@ func (i *fragmentIter) Close() {
// pool.
return
}

i.blockIter.ResetForReuse()
*i = fragmentIter{
blockIter: i.blockIter,
closeCheck: i.closeCheck,
startKeyBuf: i.startKeyBuf[:0],
endKeyBuf: i.endKeyBuf[:0],
}
i.span = keyspan.Span{}
i.dir = 0
i.fileNum = 0
i.syntheticSuffix = nil
i.syntheticPrefix = nil
i.startKeyBuf = i.startKeyBuf[:0]
i.endKeyBuf = i.endKeyBuf[:0]
fragmentBlockIterPool.Put(i)
}

Expand Down
6 changes: 0 additions & 6 deletions sstable/rowblk/rowblk_index_iter.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,6 @@ func (i *IndexIter) InitHandle(
return i.iter.InitHandle(cmp, split, block, transforms)
}

// ResetForReuse resets the index iterator for reuse, retaining buffers to avoid
// future allocations.
func (i *IndexIter) ResetForReuse() {
i.iter.ResetForReuse()
}

// Valid returns true if the iterator is currently positioned at a valid block
// handle.
func (i *IndexIter) Valid() bool {
Expand Down
29 changes: 10 additions & 19 deletions sstable/rowblk/rowblk_iter.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,21 +301,6 @@ func (i *Iter) IsDataInvalidated() bool {
return i.data == nil
}

// ResetForReuse resets the blockIter for reuse, retaining buffers to avoid
// future allocations.
func (i *Iter) ResetForReuse() {
fullKey := i.fullKey[:0]
cached := i.cached[:0]
cachedBuf := i.cachedBuf[:0]
firstUserKeyWithPrefixBuf := i.firstUserKeyWithPrefixBuf[:0]
*i = Iter{
fullKey: fullKey,
cached: cached,
cachedBuf: cachedBuf,
firstUserKeyWithPrefixBuf: firstUserKeyWithPrefixBuf,
}
}

func (i *Iter) readEntry() {
ptr := unsafe.Pointer(uintptr(i.ptr) + uintptr(i.offset))

Expand Down Expand Up @@ -1568,10 +1553,16 @@ func (i *Iter) Error() error {
// package.
func (i *Iter) Close() error {
i.handle.Release()
i.handle = block.BufferHandle{}
i.val = nil
i.ikv = base.InternalKV{}
i.lazyValueHandling.getValue = nil
fullKey := i.fullKey[:0]
cached := i.cached[:0]
cachedBuf := i.cachedBuf[:0]
firstUserKeyWithPrefixBuf := i.firstUserKeyWithPrefixBuf[:0]
*i = Iter{
fullKey: fullKey,
cached: cached,
cachedBuf: cachedBuf,
firstUserKeyWithPrefixBuf: firstUserKeyWithPrefixBuf,
}
return nil
}

Expand Down
2 changes: 1 addition & 1 deletion sstable/rowblk/rowblk_rewrite.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,6 @@ func (r *Rewriter) RewriteSuffixes(
end.Trailer = r.scratchKey.Trailer
r.keyAlloc, end.UserKey = r.keyAlloc.Copy(r.scratchKey.UserKey)

r.iter.ResetForReuse()
_ = r.iter.Close() // infallible
return start, end, r.writer.Finish(), nil
}

0 comments on commit ad898ed

Please sign in to comment.