Skip to content

perf(datagrams): add batch send/recv APIs (N2+N4)#735

Open
ifdario wants to merge 8 commits into
n0-computer:mainfrom
rayfish:perf/datagram-batching
Open

perf(datagrams): add batch send/recv APIs (N2+N4)#735
ifdario wants to merge 8 commits into
n0-computer:mainfrom
rayfish:perf/datagram-batching

Conversation

@ifdario

@ifdario ifdario commented Jul 1, 2026

Copy link
Copy Markdown

Description

Add Connection::send_datagrams / read_datagrams and the underlying DatagramState::recv_many / Datagrams::send_many to amortize the per-datagram connection-mutex cost, which is the dominant overhead at high pps for a mesh VPN forwarding bursts of TUN packets.

  • DatagramState::recv_many drains all buffered incoming datagrams in one VecDeque drain instead of N pop_fronts.
  • Datagrams::send_many queues a whole batch under one logical operation: peer-support check once, drop-oldest backpressure pass once, oversized datagrams skipped individually (not aborting the batch).
  • Connection::send_datagrams takes the mutex once, wakes the driver once.
  • Connection::read_datagrams is a ReadDatagrams future that parks on the datagram_received notify then drains everything in one lock hold.
  • New SendMany { queued, rejected } result struct.
  • 2 new unit tests: batch send/recv roundtrip, oversized-skip behavior.
  • bench: datagram benchmark harness (BenchOpt/DatagramOpt, DatagramCounters).

Baseline (loopback, release, 2-core): 440 MiB/s @ 1200B, 4.17M pps @ 64B.

Breaking Changes

None. All additions are new, purely additive methods and types. Existing send_datagram / read_datagram / send_datagram_wait are unchanged.

Notes & open questions

  • Oversized-datagram semantics differ from the single API. send_datagram returns Err(TooLarge) for an oversized datagram; send_many skips oversized datagrams and counts them in SendMany::rejected, so a mixed-size batch doesn't lose the valid entries. Err(TooLarge) is returned only if every element is too large (or datagrams are unsupported/disabled). This is deliberate but worth a reviewer's eyes.
  • One Vec allocation per batch. Connection::send_datagrams collects the iterator into a Vec<Bytes> to compute rejected = total - queued. This is one small alloc per batch, negligible versus the N mutex acquisitions it replaces, but a future PR could have the proto send_many return {queued, rejected} directly to avoid it.
  • No benchmark delta included. The existing loopback datagram benchmark exercises the single-datagram APIs; the batch win is realized by callers that already have bursts ready (e.g. a TUN reader with multiple packets queued). Measuring the real-world improvement is left to downstream callers.
  • send_datagram_wait has no batch equivalent. A batched backpressuring send is a harder design (partial-batch wait semantics, per-element wakeups) and is deferred.

Change checklist

  • Self-review.
  • Documentation updates following the style guide, if relevant.
  • Tests if relevant.
  • All breaking changes documented. (none)

Add Connection::send_datagrams / read_datagrams and the underlying
DatagramState::recv_many / Datagrams::send_many to amortize the
per-datagram connection-mutex cost, which is the dominant overhead at
high pps for a mesh VPN forwarding bursts of TUN packets.

- DatagramState::recv_many drains all buffered incoming datagrams in one
  VecDeque drain instead of N pop_fronts.
- Datagrams::send_many queues a whole batch under one logical operation:
  peer-support check once, drop-oldest backpressure pass once, oversized
  datagrams skipped individually (not aborting the batch).
- Connection::send_datagrams takes the mutex once, wakes the driver once.
- Connection::read_datagrams is a ReadDatagrams future that parks on the
  datagram_received notify then drains everything in one lock hold.
- New SendMany { queued, rejected } result struct.
- 2 new unit tests: batch send/recv roundtrip, oversized-skip behavior.
- bench: datagram benchmark harness (BenchOpt/DatagramOpt, DatagramCounters).

Baseline (loopback, release, 2-core): 440 MiB/s @ 1200B, 4.17M pps @ 64B.

@flub flub left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Probably not an unreasonable request, left some initial notes from a quick skim. I haven't looked at tests or benchmarks at all yet.

Comment thread noq-proto/src/connection/datagrams.rs Outdated
Comment thread noq-proto/src/connection/datagrams.rs Outdated
Comment thread noq/src/connection.rs Outdated
Comment thread noq/src/connection.rs Outdated
@github-project-automation github-project-automation Bot moved this from 🚑 Needs Triage to 🏗 In progress in iroh Jul 1, 2026
@flub

flub commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

Please run cargo make locally before pushing so that the obvious CI failures are avoided. And please follow up on fixing CI failures of course :)

Resolves flub's PR n0-computer#735 review:

- Fix send_many over-allocation: evict oldest per-element before each push
  (not push-all-then-evict), so the outgoing deque never transiently holds
  the whole batch. A datagram larger than the budget but under max_size is
  still enqueued once the queue is emptied, matching send(drop=true).
  Locked in by datagram_batch_send_drop_oldest_per_element.

- send_many now stops at the first oversized datagram and returns Ok(count)
  accepted from the front (mirrors write_chunks / sendmmsg "first K succeeded").
  No longer returns Err(TooLarge); callers recover via &datagrams[count..].
  Empty batch returns Ok(0) (datagram_batch_send_empty_is_ok).

- Rename to the chunks-style API: send_many_datagrams / read_many_datagrams,
  returning ReadManyDatagrams. Drop the SendMany struct.

- Take slices for consistency with read_many_chunks / write_chunks:
  send_many_datagrams(&[Bytes]) -> Result<usize, _>;
  read_many_datagrams(&mut [Bytes]) (fill-slots, returns count drained).
  recv_many return-count bug fixed (now reports slots filled, not bool).

- Extract over_send_budget / would_exceed_send_budget helpers, deduped
  across send and send_many.

- CI: fmt (project unstable config), clippy, rustdoc, all tests green.
Comment thread noq-proto/src/connection/datagrams.rs Outdated
@ifdario
ifdario requested a review from flub July 3, 2026 13:01
Comment thread noq-proto/src/connection/datagrams.rs Outdated
Comment thread noq-proto/src/connection/datagrams.rs Outdated
Comment thread noq-proto/src/connection/datagrams.rs Outdated
Comment thread noq-proto/src/connection/datagrams.rs Outdated
Comment thread noq-proto/src/connection/datagrams.rs Outdated
Comment thread noq-proto/src/connection/datagrams.rs Outdated
Comment thread noq-proto/src/connection/datagrams.rs Outdated
Comment thread noq-proto/src/connection/datagrams.rs Outdated
Comment thread noq/src/connection.rs Outdated
Comment thread noq/src/connection.rs Outdated
- send_many takes a `drop` flag, mirroring send. Reject the whole batch
  with TooLarge up front if any datagram is oversized, so a size error is
  never a partial send. With drop = false, stop when the buffer is full
  and return the count queued.
- Share the drop-oldest loop between send and send_many.
- Rename the send-buffer helpers to is_send_buffer_exceeded and
  has_send_buffer_capacity, and drain in recv_many instead of popping.
- Focus the docs and drop the lock/implementation details.
@ifdario
ifdario requested a review from flub July 11, 2026 09:19
Comment thread noq-proto/src/connection/datagrams.rs Outdated
Comment thread noq-proto/src/connection/datagrams.rs
Comment thread noq/src/connection.rs Outdated
Comment thread bench/src/bin/datagram.rs Outdated
Comment thread bench/src/bin/datagram.rs Outdated
@flub

flub commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Please use the (admittedly tiny, well hidden) re-request review button whenever the PR is ready for another round of review. It is next each reviewer's name at the top right.

@ifdario
ifdario requested a review from flub July 15, 2026 08:27
Comment thread noq/src/connection.rs Outdated
Comment thread bench/src/lib.rs

/// Direction of the datagram flood.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum Direction {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Have you found the direction to be important? In theory the direction could also be done for the stream bench, but for some reason it is not. Which is what makes me wonder if this reveals anything interesting.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Am I crazy or my comments where deleted?

Comment thread bench/src/bin/datagram.rs Outdated
Comment thread bench/src/bin/datagram.rs Outdated
Comment thread bench/src/bin/datagram.rs Outdated
ifdario added 2 commits July 16, 2026 16:13
The future no longer forces the caller's out slice borrow to match the
connection borrow.
An unpaced flood queues datagrams far faster than the driver transmits,
so drop-oldest evicts almost the whole run locally and the benchmark
measures queueing speed instead of throughput. Wait for send buffer
space before each send so every queued datagram reaches the wire and
loss reflects the network.
@ifdario
ifdario requested a review from flub July 16, 2026 15:28
…rker

Derive derive_more::Display on Direction, SendMode, and Congestion so the
JSON report prints them directly instead of hand-mapping strings.

Replace the uni done-stream and the drain grace timeout with an in-band
1-byte DONE datagram: the sender floods, then resends the marker every
250ms until the receiver closes; the receiver counts until it sees the
marker. The duplex coordination stream keeps only the S/F handshake,
where F now means the peer's DONE was received.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 🏗 In progress

Development

Successfully merging this pull request may close these issues.

2 participants