Skip to content
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

feat(bin): don't allocate in UDP recv path #2189

Merged
merged 1 commit into from
Oct 18, 2024

Conversation

mxinden
Copy link
Collaborator

@mxinden mxinden commented Oct 17, 2024

Final pull request stacked on top of #2188 which is stacked on top of #2187 which is stacked on top of #2184.

Pass a long lived receive buffer to neqo_udp::recv_inner, receiving an iterator of Datagram<&[u8]>s pointing into that buffer, thus no longer allocating in UDP receive path.


Extracted out of #2093.

Part of #1693.

Pull request stack:

  1. feat: don't allocate in UDP recv path #2184
  2. feat(transport): accept borrowed instead of owned input datagram #2187
  3. feat(udp): return borrowed Datagram on receive #2188
  4. (this one) feat(bin): don't allocate in UDP recv path #2189

Pass a long lived receive buffer to `neqo_udp::recv_inner`, receiving an
iterator of `Datagram<&[u8]>`s pointing into that buffer, thus no longer
allocating in UDP receive path.
break;
}
let dgrams: Vec<Datagram> = dgrams.map(|d| d.to_owned()).collect();
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Server allocates for now. In other words, only client UDP receive path doesn't allocate. Given that it is a test server only, I suggest we fix in a follow-up pull request.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM, open an issue?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #2190

@mxinden mxinden marked this pull request as ready for review October 17, 2024 15:44
break;
}
let dgrams: Vec<Datagram> = dgrams.map(|d| d.to_owned()).collect();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM, open an issue?

@larseggert larseggert merged commit 0a39777 into recv-no-alloc-udp Oct 18, 2024
@larseggert larseggert deleted the recv-no-alloc-bin branch October 18, 2024 09:08
mxinden added a commit that referenced this pull request Oct 20, 2024
Pass a long lived receive buffer to `neqo_udp::recv_inner`, receiving an
iterator of `Datagram<&[u8]>`s pointing into that buffer, thus no longer
allocating in UDP receive path.
larseggert pushed a commit that referenced this pull request Oct 21, 2024
* feat(udp): return borrowed Datagram on receive

Previously `recv_inner` would return `Datagram<Vec<u8>>`. In other words, it
would allocate a new `Vec<u8>` for each UDP datagram payload.

Now `recv_inner` reads into a provided buffer and returns `Datagram<&[u8]>`,
i.e. it returns a view into the provided buffer without allocating.

* feat(bin): don't allocate in UDP recv path (#2189)

Pass a long lived receive buffer to `neqo_udp::recv_inner`, receiving an
iterator of `Datagram<&[u8]>`s pointing into that buffer, thus no longer
allocating in UDP receive path.
mxinden added a commit that referenced this pull request Oct 21, 2024
Pass a long lived receive buffer to `neqo_udp::recv_inner`, receiving an
iterator of `Datagram<&[u8]>`s pointing into that buffer, thus no longer
allocating in UDP receive path.
mxinden added a commit that referenced this pull request Oct 22, 2024
Pass a long lived receive buffer to `neqo_udp::recv_inner`, receiving an
iterator of `Datagram<&[u8]>`s pointing into that buffer, thus no longer
allocating in UDP receive path.
larseggert pushed a commit that referenced this pull request Oct 22, 2024
* feat(transport): accept borrowed instead of owned input datagram

Previously `process_input` (and the like) took a `Datagram<Vec<u8>>` as input.
In other words, it required allocating each UDP datagram payload in a dedicated
`Vec<u8>` before passing it to `process_input`.

With this patch, `process_input` accepts a `Datagram<&[u8]>`. In other words, it
accepts a `Datagram` with a borrowed view into an existing buffer (`&[u8]`),
e.g. a long lived receive buffer.

* feat(udp): return borrowed Datagram on receive

Previously `recv_inner` would return `Datagram<Vec<u8>>`. In other words, it
would allocate a new `Vec<u8>` for each UDP datagram payload.

Now `recv_inner` reads into a provided buffer and returns `Datagram<&[u8]>`,
i.e. it returns a view into the provided buffer without allocating.

* feat(bin): don't allocate in UDP recv path (#2189)

Pass a long lived receive buffer to `neqo_udp::recv_inner`, receiving an
iterator of `Datagram<&[u8]>`s pointing into that buffer, thus no longer
allocating in UDP receive path.
mxinden added a commit that referenced this pull request Oct 22, 2024
Pass a long lived receive buffer to `neqo_udp::recv_inner`, receiving an
iterator of `Datagram<&[u8]>`s pointing into that buffer, thus no longer
allocating in UDP receive path.
mxinden added a commit that referenced this pull request Oct 23, 2024
Pass a long lived receive buffer to `neqo_udp::recv_inner`, receiving an
iterator of `Datagram<&[u8]>`s pointing into that buffer, thus no longer
allocating in UDP receive path.
github-merge-queue bot pushed a commit that referenced this pull request Oct 23, 2024
* feat(common): make Datagram generic over payload type

Previously the payload type of `Datagram` was `Vec<u8>`, thus the `Datagram`
always owned the UDP datagram payload.

This change enables `Datagram` to represent both (a) an owned payload allocation
(i.e. `Vec<u8>`), but also represent (b) a view into an existing payload (e.g. a
long lived receive buffer via `&[u8]`).

The default payload type stays `Vec<u8>`, thus not breaking existing usage of
`Datagram`.

* feat(transport): accept borrowed instead of owned input datagram

Previously `process_input` (and the like) took a `Datagram<Vec<u8>>` as input.
In other words, it required allocating each UDP datagram payload in a dedicated
`Vec<u8>` before passing it to `process_input`.

With this patch, `process_input` accepts a `Datagram<&[u8]>`. In other words, it
accepts a `Datagram` with a borrowed view into an existing buffer (`&[u8]`),
e.g. a long lived receive buffer.

* feat(udp): return borrowed Datagram on receive

Previously `recv_inner` would return `Datagram<Vec<u8>>`. In other words, it
would allocate a new `Vec<u8>` for each UDP datagram payload.

Now `recv_inner` reads into a provided buffer and returns `Datagram<&[u8]>`,
i.e. it returns a view into the provided buffer without allocating.

* feat(bin): don't allocate in UDP recv path (#2189)

Pass a long lived receive buffer to `neqo_udp::recv_inner`, receiving an
iterator of `Datagram<&[u8]>`s pointing into that buffer, thus no longer
allocating in UDP receive path.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants