Pool buffers per handler and client #192
Conversation
|
@akshayjshah to take this over |
|
Fixed this up - tests and lint are passing now, and I found an unrelated bug along the way. I'll pull the unrelated fix out into a separate PR (it's unfortunately difficult to unit test), get this rebased, and post the performance diff later tonight. The bulk of the actual logic changes are in bufbuild@fccf9fd (marshaling) and bufbuild@f396e2c (unmarshaling). |
When we marshal length-prefixed messages, it's more efficient and a little simpler if we always deal in buffers instead of slices.
akshayjshah
force-pushed
the
per-instance-buffer-pools
branch
from
3413e46 to
09a7397
Compare
When we want to use a pooled buffer as a byte slice, we have to first call `Grow` (to ensure adequate capacity) and then reslice to set `len(raw)` to `cap(raw)`.
akshayjshah
force-pushed
the
per-instance-buffer-pools
branch
from
09a7397 to
8016997
Compare
Currently, the benchmark is a bit unrealistic - the wire size of the payload is single-digit bytes. This makes the payload slightly bigger, though it still compresses unrealistically well.
|
Changed the benchmarks to use a slightly more realistic payload (a 2 MiB string, though it compresses unreasonably well). I ran the benchmarks on main with the benchmark changes cherry-picked in, then on this PR. Comparison: A 20% speed improvement and a 40% reduction in allocated memory seems worthwhile! If we want to simplify the implementation a bit, we can stop pooling |
| rawBuffer := u.bufferPool.Get() | ||
| defer u.bufferPool.Put(rawBuffer) | ||
| rawBuffer.Grow(size) | ||
| raw := rawBuffer.Bytes()[0:size] |
There was a problem hiding this comment.
Just making sure - are there any concerns with this not being memset 0'ed?
There was a problem hiding this comment.
I don't think so - we're being very careful not to continue on if we don't read size bytes off the network. No harm zeroing it, though!
|
Woohoo!
…On Sat, Apr 9, 2022 at 8:08 AM bufdev ***@***.***> wrote:
Merged #192 <https://github.com/bufbuild/connect-go/pull/192> into main.
—
Reply to this email directly, view it on GitHub
<https://github.com/bufbuild/connect-go/pull/192#event-6401748502>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAHNP5QEDLJZAWSG3DPYLGDVEGMQDANCNFSM5S2REA5Q>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
This doesn't actually work yet, I messed up something with lpm where it's causing an infinite loop (likely with my slicing into
raw.Bytes()), but just demonstrating the idea: Instead of a globalsync.Pool, we could just thread it down on a per-Client/Handler basis.