Repository navigation
perf(h3-quinn): hand DATA payloads to quinn without copying - #368
Open
jothomas-xai wants to merge 1 commit into
Open
jothomas-xai wants to merge 1 commit into
jothomas-xai wants to merge 1 commit into
Conversation
`SendStream::poll_ready` wrote each chunk with `quinn::SendStream::poll_write(&[u8])`, which copies the slice into an owned buffer inside quinn-proto (`ByteSlice::pop_chunk`). Every byte of every DATA frame was copied once before packetization. Take each chunk out of the `WriteBuf` with `copy_to_bytes` and pass it to `quinn::SendStream::write_chunks`, which keeps the `Bytes` as is. `write_chunks` is cancellation safe, so polling a fresh future each time matches what quinn's own `poll_write` does. A partially written chunk stays in `writing` until quinn accepts the rest. `WriteBuf::copy_to_bytes` now delegates to the frame payload once the header is consumed, so a `Bytes` payload is split without copying. The header is still copied (a few bytes), and a non-`Bytes` payload is copied once, here instead of inside quinn.
This branch has not been deployed
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.
What happens
h3-quinn'sSendStream::poll_readywrites each chunk of aWriteBufwithquinn::SendStream::poll_write(cx, data.chunk()). quinn turns that&[u8]into an owned buffer inByteSlice::pop_chunk(self.data[..limit].to_owned()), so every byte of every DATA frame is copied once into quinn's send buffer before it is copied again into a packet. #279 removed the copy on the h3-quinn side; this one is the copy inside quinn.quinn already has a path that keeps the caller's buffers:
SendStream::write_chunks(&mut [Bytes]).Change
h3-quinn:poll_readytakes each chunk out of theWriteBufwithcopy_to_bytesand passes it towrite_chunks.write_chunksis documented as cancellation safe ("If this does not resolve, no bytes were written"), so creating and polling a fresh future on eachpoll_readyis the same pattern quinn's ownpoll_writeuses. A partially written chunk stays inwriting(nowOption<(WriteBuf<B>, Bytes)>) until quinn accepts the rest. No quinn changes.h3:WriteBufoverridesBuf::copy_to_bytesto delegate to the frame payload once the header is consumed. For aBytespayload that is asplit_to, so the payload reaches quinn without a copy. The frame header (a few bytes) is still copied, and a non-Bytespayload is copied once by its owncopy_to_bytes, here instead of inside quinn.poll_send(SendStreamUnframed) is unchanged.Testing
write_buf_copy_to_bytes_shares_payload_after_header:copy_to_bytesacross the header/payload boundary copies, and after the header it returns a slice that shares the payload's allocation.finish_after_cancelled_send_data_keeps_connection_opensends a 4 MiB body that exceeds the stream window, so it exercises partial writes andPendingthrough the new path.cargo test --features 'tracing,i-implement-a-third-party-backend-and-opt-into-breaking-changes',cargo fmt --all -- --check,cargo clippy,ci/example_test.sh, andcargo +1.74.1 check --locked -p h3-quinn/-p h3(lockfile fromCARGO_RESOLVER_INCOMPATIBLE_RUST_VERSIONS=fallback) all pass locally.