-
Notifications
You must be signed in to change notification settings - Fork 26
Pool result matches #476
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
Pool result matches #476
Conversation
This change adds a resultBuf field to nfaBuffers and a matchesInto() method to matchSet. Instead of allocating a new slice for each match result, we reuse the pooled buffer by appending matches to it. The buffer is reset to length 0 before each use while preserving capacity.
timbray
left a comment
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.
saves one allocation and 41nsec per op. We're getting into diminishing returns. One question in the review.
| // matchesInto appends matches to the provided buffer and returns it. | ||
| // This avoids allocating a new slice for the result. | ||
| func (m *matchSet) matchesInto(buf []X) []X { | ||
| for x := range m.set { |
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.
Might the builtin copy() work here? Might even be a tiny bit faster.
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.
oh sorry, of course not, m.set is a map not a slice
| // matchesInto appends matches to the provided buffer and returns it. | ||
| // This avoids allocating a new slice for the result. | ||
| func (m *matchSet) matchesInto(buf []X) []X { | ||
| for x := range m.set { |
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.
oh sorry, of course not, m.set is a map not a slice
|
It's a big win in memory footprint, even if only modest in CPU: === Heap Profile Comparison: main vs PR3 (pool-result-buffer) === TOTAL MEMORY ALLOCATED: TOTAL ALLOCATION OBJECTS: KEY CHANGES: matchSet.matches() (eliminated by pooling): transmap.all: traverseNFA: |
This change adds a resultBuf field to nfaBuffers and a matchesInto()
method to matchSet. Instead of allocating a new slice for each match
result, we reuse the pooled buffer by appending matches to it. The
buffer is reset to length 0 before each use while preserving capacity.
See #470.
It only saves one alloc/op, but the bytes/op go way down.
Before:
After: