Skip to content

Commit b343079

Browse files
committed
db: clear a subset of Iterator fields on Close
Adjust the logic that zeroes an Iterator to only clear fields after a marker `clearForReuseBoundary` field, and use this to delegate clearing of the blob.ValueFetcher to the blob.ValueFetcher's Close method. When initializing the Iterator, only set the fields that need to be set (instead relying on the allocated Iterator's other fields having already been zeroed. This in preparation for recycling a buffer within the blob.ValueFetcher across reuses of the containing Iterator. ``` goos: darwin goarch: arm64 pkg: github.com/cockroachdb/pebble cpu: Apple M1 Pro │ old.txt │ fetcher.txt │ │ sec/op │ sec/op vs base │ NewIterClose/1-10 1014.0n ± 1% 849.2n ± 2% -16.25% (p=0.000 n=25) NewIterClose/10-10 5.246µ ± 0% 5.184µ ± 1% -1.18% (p=0.000 n=25) NewIterClose/100-10 78.68µ ± 1% 79.25µ ± 2% ~ (p=0.151 n=25) geomean 7.480µ 7.040µ -5.89% ```
1 parent 526e102 commit b343079

File tree

3 files changed

+39
-21
lines changed

3 files changed

+39
-21
lines changed

db.go

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1059,22 +1059,20 @@ func (d *DB) newIter(
10591059
// them together.
10601060
buf := newIterAlloc()
10611061
dbi := &buf.dbi
1062-
*dbi = Iterator{
1063-
ctx: ctx,
1064-
alloc: buf,
1065-
merge: d.merge,
1066-
comparer: d.opts.Comparer,
1067-
readState: readState,
1068-
version: newIterOpts.snapshot.vers,
1069-
keyBuf: buf.keyBuf,
1070-
prefixOrFullSeekKey: buf.prefixOrFullSeekKey,
1071-
boundsBuf: buf.boundsBuf,
1072-
fc: d.fileCache,
1073-
newIters: newIters,
1074-
newIterRangeKey: newIterRangeKey,
1075-
seqNum: seqNum,
1076-
batchOnlyIter: newIterOpts.batch.batchOnly,
1077-
}
1062+
dbi.ctx = ctx
1063+
dbi.alloc = buf
1064+
dbi.merge = d.merge
1065+
dbi.comparer = d.opts.Comparer
1066+
dbi.readState = readState
1067+
dbi.version = newIterOpts.snapshot.vers
1068+
dbi.keyBuf = buf.keyBuf
1069+
dbi.prefixOrFullSeekKey = buf.prefixOrFullSeekKey
1070+
dbi.boundsBuf = buf.boundsBuf
1071+
dbi.fc = d.fileCache
1072+
dbi.newIters = newIters
1073+
dbi.newIterRangeKey = newIterRangeKey
1074+
dbi.seqNum = seqNum
1075+
dbi.batchOnlyIter = newIterOpts.batch.batchOnly
10781076
if o != nil {
10791077
dbi.opts = *o
10801078
dbi.processBounds(o.LowerBound, o.UpperBound)

iterator.go

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,17 @@ type LazyValue = base.LazyValue
194194
// Next, Prev) return without advancing if the iterator has an accumulated
195195
// error.
196196
type Iterator struct {
197+
// blobValueFetcher is the ValueFetcher to use when retrieving values stored
198+
// externally in blob files.
199+
blobValueFetcher blob.ValueFetcher
200+
201+
// All fields below this field are cleared during Iterator.Close before
202+
// returning the Iterator to the pool. Any fields above this field must also
203+
// be cleared, but may be cleared as a part of the body of Iterator.Close
204+
// (eg, if these types have their own Close method that zeroes their
205+
// fields).
206+
clearForReuseBoundary struct{}
207+
197208
// The context is stored here since (a) Iterators are expected to be
198209
// short-lived (since they pin memtables and sstables), (b) plumbing a
199210
// context into every method is very painful, (c) they do not (yet) respect
@@ -228,9 +239,6 @@ type Iterator struct {
228239
// For use in LazyValue.Value.
229240
lazyValueBuf []byte
230241
valueCloser io.Closer
231-
// blobValueFetcher is the ValueFetcher to use when retrieving values stored
232-
// externally in blob files.
233-
blobValueFetcher blob.ValueFetcher
234242
// boundsBuf holds two buffers used to store the lower and upper bounds.
235243
// Whenever the Iterator's bounds change, the new bounds are copied into
236244
// boundsBuf[boundsBufIdx]. The two bounds share a slice to reduce
@@ -315,6 +323,13 @@ type Iterator struct {
315323
nextPrefixNotPermittedByUpperBound bool
316324
}
317325

326+
const clearOff = unsafe.Offsetof(Iterator{}.clearForReuseBoundary)
327+
const clearLen = unsafe.Sizeof(Iterator{}) - clearOff
328+
329+
func (i *Iterator) clearForReuse() {
330+
*(*[clearLen]byte)(unsafe.Add(unsafe.Pointer(i), clearOff)) = [clearLen]byte{}
331+
}
332+
318333
// cmp is a convenience shorthand for the i.comparer.Compare function.
319334
func (i *Iterator) cmp(a, b []byte) int {
320335
return i.comparer.Compare(a, b)
@@ -2452,7 +2467,7 @@ func (i *Iterator) Close() error {
24522467
if i.batch != nil {
24532468
alloc.batchState = iteratorBatchState{}
24542469
}
2455-
alloc.dbi = Iterator{}
2470+
alloc.dbi.clearForReuse()
24562471
alloc.merging = mergingIter{}
24572472
clear(mergingIterHeapItems[:cap(mergingIterHeapItems)])
24582473
alloc.merging.heap.items = mergingIterHeapItems

sstable/blob/fetcher.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,8 @@ type ValueFetcher struct {
6161
readerProvider ReaderProvider
6262
env block.ReadEnv
6363
fetchCount int
64-
readers [maxCachedReaders]cachedReader
6564
bufMangler invariants.BufMangler
65+
readers [maxCachedReaders]cachedReader
6666
}
6767

6868
// TODO(jackson): Support setting up a read handle for compaction when relevant.
@@ -184,6 +184,11 @@ func (r *ValueFetcher) Close() error {
184184
err = errors.CombineErrors(err, r.readers[i].Close())
185185
}
186186
}
187+
r.fileMapping = nil
188+
r.readerProvider = nil
189+
r.env = block.ReadEnv{}
190+
r.fetchCount = 0
191+
r.bufMangler = invariants.BufMangler{}
187192
return err
188193
}
189194

0 commit comments

Comments
 (0)