Avoid copies of large payloads #1529
Merged
Conversation
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
Motivation: Messages are prefixed with a 5-byte header. Currently messages of all sizes are written into a new buffer with their header. For smaller payloads this is good: we avoid the extra allocations associated with creating HTTP/2 frames. For large payloads the cost of the copy outweighs the cost of extra allocations. Modifications: - For messages larger than 8KB emit an extra HTTP/2 DATA frame containing just the message header. Result: Better performance for large payloads.
glbrntt
force-pushed
the
gb-avoid-large-framing-copies
branch
from
5f26354 to
564f522
Compare
Lukasa
reviewed
Dec 7, 2022
| } else if buffer.readableBytes > Self.singleBufferSizeLimit { | ||
| // Buffer is larger than the limit for emitting a single buffer: create a second buffer | ||
| // containing just the message header. | ||
| var prefixed = allocator.buffer(capacity: 5) |
There was a problem hiding this comment.
I wonder if we can be even cleverer with this and re-use the same fixed buffer.
Co-authored-by: Cory Benfield <[email protected]>
glbrntt
commented
Dec 7, 2022
Lukasa
approved these changes
Dec 8, 2022
glbrntt
added a commit
to glbrntt/grpc-swift
that referenced
this pull request
Jan 4, 2023
Motivation: A change made in grpc#1529 allowed multiple DATA frames to be emitted for a single message. One of the perf tests preconditions on there being exactly one DATA frame per message which is no longer the case. Modifications: - Update the precondition Result: Perf test does not crash.
Lukasa
added a commit
that referenced
this pull request
Jan 4, 2023
Motivation: A change made in #1529 allowed multiple DATA frames to be emitted for a single message. One of the perf tests preconditions on there being exactly one DATA frame per message which is no longer the case. Modifications: - Update the precondition Result: Perf test does not crash. Co-authored-by: Cory Benfield <[email protected]>
WendellXY
pushed a commit
to sundayfun/grpc-swift
that referenced
this pull request
Aug 24, 2023
Motivation: Messages are prefixed with a 5-byte header. Currently messages of all sizes are written into a new buffer with their header. For smaller payloads this is good: we avoid the extra allocations associated with creating HTTP/2 frames. For large payloads the cost of the copy outweighs the cost of extra allocations. Modifications: - For messages larger than 8KB emit an extra HTTP/2 DATA frame containing just the message header. Result: Better performance for large payloads.
WendellXY
pushed a commit
to sundayfun/grpc-swift
that referenced
this pull request
Aug 24, 2023
Motivation: A change made in grpc#1529 allowed multiple DATA frames to be emitted for a single message. One of the perf tests preconditions on there being exactly one DATA frame per message which is no longer the case. Modifications: - Update the precondition Result: Perf test does not crash. Co-authored-by: Cory Benfield <[email protected]>
pinlin168
pushed a commit
to sundayfun/grpc-swift
that referenced
this pull request
Aug 24, 2023
Motivation: Messages are prefixed with a 5-byte header. Currently messages of all sizes are written into a new buffer with their header. For smaller payloads this is good: we avoid the extra allocations associated with creating HTTP/2 frames. For large payloads the cost of the copy outweighs the cost of extra allocations. Modifications: - For messages larger than 8KB emit an extra HTTP/2 DATA frame containing just the message header. Result: Better performance for large payloads.
pinlin168
pushed a commit
to sundayfun/grpc-swift
that referenced
this pull request
Aug 24, 2023
Motivation: A change made in grpc#1529 allowed multiple DATA frames to be emitted for a single message. One of the perf tests preconditions on there being exactly one DATA frame per message which is no longer the case. Modifications: - Update the precondition Result: Perf test does not crash. Co-authored-by: Cory Benfield <[email protected]>
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.
Motivation:
Messages are prefixed with a 5-byte header. Currently messages of all sizes are written into a new buffer with their header. For smaller payloads this is good: we avoid the extra allocations associated with creating HTTP/2 frames. For large payloads the cost of the copy outweighs the cost of extra allocations.
Modifications:
Result:
Better performance for large payloads.