search

quinn-rs / quinn

Async-friendly QUIC implementation in Rust
Apache License 2.0
3.85k stars 394 forks source link

Remove redundant packet copying #1272

Closed TimonPost closed 2 years ago

TimonPost commented 2 years ago

Both proto::Endpoint.transmits and and quinn::Endpoint.outgoing queue transmit packets, and in drive send both queues are fetched and drained while this isn't required (from what I can tell). This PR removes the outgoing queue in quinn::Endpoint and simplifies it to only use proto::Endpoint.transmits for all outgoing packets.

This should remove redundant copying of transmits, and queue allocations.

left: before, right: this PR

image

To make the API consistent, I renamed poll_transmit to deque_transmit. I could name queue_transmit to push_transmit to make my function more consistent although I am more satisfied with queue terminology because 1) the transmit storage is a queue 2) poll doesn't really say some dequeue operation is going to happen.

The Linux CI error also occurs on a fresh clone: run with cargo test echo_dualstack -- --nocapture to validate this.

TimonPost commented 2 years ago

Feel free to close it if you don't see it fit. Tho I don't see why one would want to have two places to store transmissions while it can easaly be one storage. Even if the move costs are minimal, to me it seems more cleaner this way.

Ralith commented 2 years ago

To avoid significant expansion of the quinn_proto::Endpoint API, at least.