Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

db: mergingIter.heap.items recycling does not work #3092

Open
nvanbenschoten opened this issue Nov 21, 2023 · 0 comments
Open

db: mergingIter.heap.items recycling does not work #3092

nvanbenschoten opened this issue Nov 21, 2023 · 0 comments

Comments

@nvanbenschoten
Copy link
Member

There seems to be an effort made to recycle the mergingIter.heap.items slice here:

pebble/merging_iter.go

Lines 317 to 321 in 56834cb

if cap(m.heap.items) < len(levels) {
m.heap.items = make([]*mergingIterLevel, 0, len(levels))
} else {
m.heap.items = m.heap.items[:0]
}

This doesn't play well with the iterAlloc memory pooling:

pebble/db.go

Lines 971 to 980 in 56834cb

type iterAlloc struct {
dbi Iterator
keyBuf []byte
boundsBuf [2][]byte
prefixOrFullSeekKey []byte
merging mergingIter
mlevels [3 + numLevels]mergingIterLevel
levels [3 + numLevels]levelIter
levelsPositioned [3 + numLevels]bool
}

That's because the mergingIter field is cleared before these structs are placed back into the iterAllocPool:

pebble/iterator.go

Lines 2330 to 2335 in 56834cb

*alloc = iterAlloc{
keyBuf: alloc.keyBuf,
boundsBuf: alloc.boundsBuf,
prefixOrFullSeekKey: alloc.prefixOrFullSeekKey,
}
iterAllocPool.Put(alloc)

As a result, I see 1 memory allocation in each call to (*DB).NewIterWithContext:

Screenshot 2023-11-20 at 7 04 46 PM

This may be working as intended. If so, feel free to close.

@blathers-crl blathers-crl bot added this to Incoming in Storage Nov 21, 2023
@nicktrav nicktrav moved this from Incoming to Backlog in Storage Nov 28, 2023
@aadityasondhi aadityasondhi self-assigned this Jan 29, 2024
@aadityasondhi aadityasondhi moved this from Backlog to Next in Storage Jan 29, 2024
@aadityasondhi aadityasondhi removed their assignment Apr 23, 2024
@aadityasondhi aadityasondhi moved this from Next to Backlog in Storage Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Storage
  
Backlog
Development

No branches or pull requests

2 participants