-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Release entryBufferPool once #5324
Conversation
if i.buf.entries != nil { | ||
// preserve the underlying slice before releasing to pool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment is a bit weird. Should we just reslice when we get ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could do this after Get
'ing, yeah -- I believe that has equivalent results. The important part is that we don't nil
it here because the reason we're using a pool is to preserve the underlying slice so we don't have to reallocate it. Does that make sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I would just re-slice entries are never nil.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Good catch btw. We definitively lacking a test on close, after iteration finished. |
* release entryBufferPool once * reverseEntryIterator.Next is safe to call past expiry
While investigating an issue, I found a bug where we can put the
*entryBuffer
back into the pool twice. This PR ensures we only release it once.This PR also improves efficiency a bit by ensuring we don't
nil
the underlying slice unnecessarily, instead letting the pool reuse it.This can happen when:
First, we put it into the pool here:
then it gets
Get()
'd by another goroutine, has it'sentries
appended, then the original goroutine puts it into the pool a second time here: