Avoid quadratic buffer copies in EnvelopeReader streaming path#304
Merged
Conversation
Use in-place `del buffer[:n]` instead of `buffer = buffer[n:]` to avoid allocating a new bytearray and copying all remaining bytes on every message. For long-running streaming RPCs with N messages, this reduces total bytes copied from O(N²) to O(N), yielding up to 66x throughput improvement in single-feed scenarios. Signed-off-by: dongjiang <dongjiang1989@126.com>
Contributor
Author
|
cc @anuraaga PTAL |
Remove comments explaining in-place deletion for buffer. Signed-off-by: Anuraag (Rag) Agrawal <anuraaga@gmail.com>
Collaborator
|
Thanks @dongjiang1989 - while the large single feed case should be rare in practice, the tweak is simple enough to go with. Note that I think the reason it improves isn't because of memmove, which would only reduce allocations but not copies, but because bytearray itself amortizes prefix deletion by keeping track of a start ptr and reclaiming with heuristics. I think these heuristics will work fine for the normal streaming case so no worries. |
anuraaga
approved these changes
Jul 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
del buffer[:n]instead ofbuffer = buffer[n:]inEnvelopeReader._read_messages()bytearrayand copying all remaining bytes on every messageBackground
bytearray[n:]creates a new bytearray and copies all remaining bytes. In the envelope reader's message loop, this happened after every message was consumed:In a long-running streaming RPC receiving N messages, the cumulative copy volume becomes:
The fix uses in-place deletion, which reuses the existing buffer via C-level
memmove:Benchmark Results
A benchmark was run comparing the old and new implementation across several streaming scenarios. Each scenario was executed 50 times and averaged.
Quadratic Behavior Confirmation
The "Single feed" scenarios clearly expose the O(n²) behavior in the original implementation:
A 10x increase in data resulted in a 145x slowdown in the original code, closely matching the theoretical 10² = 100x expected from quadratic scaling.
Streaming Scenarios
Chunked feeding (64B/256B chunks) does not accumulate large buffers, so the quadratic effect is less pronounced. However, the fix still eliminates the latent risk of quadratic behavior for large-buffer cases and provides modest gains for small buffers.
Conclusion
The fix eliminates quadratic memory copying in the streaming RPC hot path, delivering 3.8x to 66.7x throughput improvement depending on workload. Memory allocations drop from O(N) to O(1) per message, and total bytes copied drops from O(N²) to O(N). This is particularly important for: