-
Notifications
You must be signed in to change notification settings - Fork 126
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
perf: use GSO #2138
Closed
Closed
perf: use GSO #2138
Conversation
This file contains 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
This change is best summarized by the `process` function signature. On `main` branch the `process` function looks as such: ```rust pub fn process(&mut self, dgram: Option<&Datagram>, now: Instant) -> Output { ``` - It takes as **input** an optional reference to a `Datagram`. That `Datagram` owns an allocation of the UDP payload, i.e. a `Vec<u8>`. Thus for each incoming UDP datagram, its payload is allocated in a new `Vec`. - It returns as **output** an owned `Output`. Most relevantly the `Output` variant `Output::Datagram(Datagram)` contains a `Datagram` that again owns an allocation of the UDP payload, i.e. a `Vec<u8>`. Thus for each outgoing UDP datagram too, its payload is allocated in a new `Vec`. This commit changes the `process` function to: ```rust pub fn process_into<'a>( &mut self, input: Option<Datagram<&[u8]>>, now: Instant, write_buffer: &'a mut Vec<u8>, ) -> Output<&'a [u8]> { ``` (Note the rename to `process_into` is temporary.) - It takes as **input** an optional `Datagram<&[u8]>`. But contrary to before, `Datagram<&[u8]>` does not own an allocation of the UDP payload, but represents a view into a long-lived receive buffer containing the UDP payload. - It returns as **output** an `Output<&'a [u8]>` where the `Output::Datagram(Datagram<&'a [u8]>)` variant does not own an allocation of the UDP payload, but here as well represents a view into a long-lived write buffer the payload is written into. That write buffer lives outside of `neqo_transport::Connection` and is provided to `process` as `write_buffer: &'a mut Vec<u8>`. Note that both `write_buffer` and `Output` use the lifetime `'a`, i.e. the latter is a view into the former. This change to the `process` function enables the following: 1. A user of `neqo_transport` (e.g. `neqo_bin`) has the OS write incoming UDP datagrams into a long-lived receive buffer (via e.g. `recvmmsg`). 2. They pass that receive buffer to `neqo_transport::Connection::process` along with a long-lived write buffer. 3. `process` reads the UDP datagram from the long-lived receive buffer through the `Datagram<&[u8]>` view and writes outgoing datagrams into the provided long-lived `write_buffer`, returning a view into said buffer via a `Datagram<&'a [u8]>`. 4. The user, after having called `process` can then pass the write buffer to the OS (e.g. via `sendmsg`). To summarize a user can receive and send UDP datagrams, without allocation in the UDP IO path. As an aside, the above is compatible with GSO and GRO, where a send and receive buffer contains a consecutive number of UDP datagram segments.
This reverts commit 995c499.
One can just use process(None, ...)
This reverts commit 3df6660.
No need to play with fire (uninitialized memory). Simply initialize the recv buffer once at startup.
Failed Interop TestsQUIC Interop Runner, client vs. server neqo-latest as client
neqo-latest as server
All resultsSucceeded Interop TestsNone ❓ Unsupported Interop TestsQUIC Interop Runner, client vs. server neqo-latest as client
neqo-latest as server
|
If we do want to tackle GSO in the future, this can serve as guidance. That said, best to start fresh. Work tracked in #1693. Closing here. |
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.
Depends on #2093. All, but last commit, part of #2093.
Part of #1693.
Proof of concept for now. Thus Draft.
Compiles. Download benchmark works. Without PMTUD I see up to 64 UDP segments on a GSO
sendmsg
call.