[zshenker]

Problem

quic-egress-profile plans sendmmsg across connections as a lever to build only where the profile shows the cost. It has no data yet, and nothing on the design questions a driver-level batch raises.

Approach

Quest text only. The sendmmsg lever now records a contributor's prototype as a starting point. The quest labels it as out of tree: none of its code is in-tree, and the numbers are the prototype's, not the profile's. It links the port in a fork and the raw samples, and asks the profile to re-measure the port.

• Provenance. The base revision and patches it was measured on, the harness invocation per point, and the setup: loopback, one pinned server core, a dual-socket Broadwell Xeon on Linux 6.12, and on/off runs back to back on one binary. The pre-port revisions and the harness are not public, and the quest says so.

• Numbers. At a 1280-byte payload on one core:

  • about 18 to 19 trains went per sendmmsg, a cross-run strace estimate;
  • server CPU fell 8 to 15% for small-object sessions and 16 to 22% for audio-rate ones (n=3 each);
  • p50 latency rose from tens of microseconds to about 0.3 ms, with p99 up to about 0.77 ms.

• Shape.

  • Linux try_send_many, keeping per-transmit cmsgs and the GSO fallback.
  • A byte-bounded per-socket queue, flushed by one task per socket.
  • Handshake packets go straight to the socket so handle_unreachable can still fail a dial with NO_VIABLE_PATH. That isn't unconditional: on WouldBlock the port queues them.
  • Rebind handling, and a best-effort drop flush that can lose a CONNECTION_CLOSE.

  For each gap, the quest says what a shipped version should do instead.

• Open decisions. Whether batching is opt-in in the driver config, given the latency cost, and whether moq-uring needs any of it. The profile settles these, not this PR.

Impact

None. Quest text only.

Consumer: the egress profile, which decides whether the lever ships.

Alternatives

• A separate quest for send batching. The egress profile already owns this lever and its measurement plan, so a second quest would split one decision across two files.

Follow-ups

None. The profile decides.

Review

• Harness: quest review, Codex gpt-6.1-sol, medium effort, round 1.
• Head: e88c4f4, against origin/main 2d1342e.
• Findings: none.
• Posted review findings:
  • The handshake bypass was not unconditional. Fixed in e88c4f4, after checking batch.rs#L148-L156.
  • Every earlier finding was fixed in prior commits.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

[pullfrog]

ℹ️ Minor suggestions only — two doc-accuracy points, nothing blocking.

Reviewed changes

• Recorded a sendmmsg prototype in the egress profile (quest/m2/quic-egress-profile.md): extends the existing sendmmsg lever with a prototype's measurements (~17 trains/call at 1280 bytes on one core, server CPU down 8-13% small-object and 16-21% audio-rate, median latency +0.3 ms), its shape (try_send_many with per-transmit cmsgs and EIO/EINVAL GSO fallback, a byte-bounded per-socket queue, handshake bypass so handle_unreachable still fails a dial with NO_VIABLE_PATH, rebind and drop handling), and the open opt-in/io_uring decisions.

Quest text only; no wire, API, or behavioral surface.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: f7ae776

P2 — Record reproducible measurement provenance (quest/m2/quic-egress-profile.md:44–47). The CPU and latency numbers lack a prototype revision or patch, paired baseline, benchmark command and workload, machine/kernel/network configuration, samples and raw results. Since the quest uses measured cost to decide whether to ship, those omissions prevent another person from assessing applicability or noise. Link the prototype and raw runs and record the paired setup and variability before relying on the figures.

Measuring before shipping and keeping batching and uring choices open is sensible. Verification: static GitHub review only; no prototype, benchmark or tests executed.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The profile records benchmark measurements and implementation details for a Linux sendmmsg prototype. It describes batching up to 64 transmits per call, per-socket queues and flush tasks, and handshake bypass. It also documents latency changes, queued-send failure and shutdown behavior, and open questions about batching defaults and whether moq-uring needs batching.

Merge Risk: Minimal · up to 45339

This documentation-only update records performance tradeoffs and prototype limitations without changing runtime behavior. No actionable merge-blocking issue is established.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title names the change: it records measurements from a sendmmsg prototype in the quic-egress-profile document. The changeset adds 66 lines of prototype measurements, implementation shape, and open…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description check		The description directly matches the changeset. It explains that the PR adds prototype measurements, implementation details, provenance, and open design decisions to the quic-egress profile, with ques…

Full details: Title check

Explanation

The title names the change: it records measurements from a sendmmsg prototype in the quic-egress-profile document. The changeset adds 66 lines of prototype measurements, implementation shape, and open decisions to quest/m2/quic-egress-profile.md, so the title matches the main change. It is specific and does not use vague terms.

Full details: Description check

Explanation

The description directly matches the changeset. It explains that the PR adds prototype measurements, implementation details, provenance, and open design decisions to the quic-egress profile, with quest text as the only change.

Finishing Touches

Simplify code

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 894fa894-abda-414a-ae79-0f87c734765f

Commits

Reviewing files that changed from the base of the PR and between 776d694 and 453399f.

Files selected for processing: 1

• quest/m2/quic-egress-profile.md

Files skipped from review as they are similar to previous changes: 1

• quest/m2/quic-egress-profile.md

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

[zshenker]

Provenance added in fb664af: setup, workloads, CPU and latency measures, sample counts and spread per point, the raw samples, and the prototype ported onto main. The numbers come from the prototype before the port, so the quest asks the profile to re-measure the port.

(Written by Claude Opus 5.5)

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: fb664af

The linked raw samples, workload and paired setup substantially address my earlier reproducibility finding. The exact prototype revision measured before the later port and the benchmark invocation remain unspecified; recording them would make the results reproducible. The independent suggestions to link the port and state the batch limit are addressed.

Profiling before deciding whether to ship is sound. Static GitHub review only; I did not run the benchmark.

[pullfrog]

✅ No new issues found.

Reviewed changes

• Addressed the provenance gap: linked the port onto main (opt-in through Endpoint::batch_sends) and the raw bench/driver-batch-sends/samples.jsonl, so the numbers are checkable against a revision.
• Added the measurement setup: loopback, pinned server/client tokio workers, 1,280-byte payloads, MTU discovery off with GSO and GRO on, BBR3, 15 s runs after a 5 s warmup on a Broadwell Xeon / Linux 6.12, alternating on/off on the same binary.
• Replaced the terse figures with per-point samples: n=3 CPU percentages with ranges for small-object (61.7% off vs 54.9% on) and audio (66.9% vs 53.9%), n=1 video, system-share range and run-to-run variance, raw p50/p99 ranges, and the strace sendmmsg/sendmsg ratio behind the ~17-trains-per-call figure.
• Disambiguated the batch cap: now states up to 64 transmits, one mmsghdr each, per sendmmsg, matching MAX_SEND_MANY in the linked prototype, which resolves the earlier BATCH_SIZE = 32 / GSO-64 conflation.
• Reworked the open question to cite the added p50 and p99 latency rather than a bare "median +0.3 ms".

Every figure reproduces against the linked samples: small-object CPU means 61.68→54.91, audio 66.87→53.90, video1/video3 ~10%/~8%, system-share 0.31-0.38→0.22-0.34, and the p50/p99 ranges. Both prior Pullfrog threads are addressed. Quest text only; no wire, API, or behavioral surface.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}

[coderabbitai]

• Fix comments: Fix CodeRabbit comments on this PR

[zshenker]

Recorded in 776d694: the measured base and prototype revisions, how the steady-state send path matches the linked port and what differs, and the harness invocation per point. Those revisions and the harness are not public.

(Written by Claude Opus 5.5)

[pullfrog]

✅ No new issues found.

Reviewed changes

Incremental re-review of the single commit pushed since the prior Pullfrog review (776d694), which records the prototype's measured revision and harness invocation, and corrects the strace call ratio. Both prior Pullfrog threads were already retired.

• Recorded the measured revision: e072cf604 with #7458, #7418, #7394, #7543, and a GSO byte cap equivalent to #7649, noting the measured revisions predate the handshake bypass, that the last round followed the drop and rebind fixes, and how the port's steady-state send path differs.
• Recorded the harness invocation: the bench command per workload point plus defaults, so the setup is no longer implied.
• Corrected the strace ratio: 135,497 sendmsg unbatched against 7,214 sendmmsg plus 506 sendmsg batched, ~18-19 trains per sendmmsg, with its estimate caveats made explicit.

The batch arithmetic reproduces ((135,497 - 506) / 7,214 ≈ 18.7), and the invocation's object sizes line up with the stated bitrates (9,375-15,625 B at 30/s ≈ 3 Mbit/s; 3,125-5,208 B at 30/s ≈ 1 Mbit/s). This closes the remaining provenance request on the prior human review. Quest text only; no wire, API, or behavioral surface.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}

[kixelated]

Automated review by review (OpenAI)

Reviewed head: 776d694

1. [P2] Qualify the shutdown flush guarantee (quest/m2/quic-egress-profile.md:99-101). The linked port's Drop implementation stops on WouldBlock and drops unsent entries. A flush may also already have moved a batch out of the shared queue; once Drop takes the socket, that flush returns without sending the rest. Consequently a queued CONNECTION_CLOSE can be discarded before it reaches the socket, even though the port is freed. Please describe the flush as best-effort and note the in-flight case, or make the port preserve those sends before claiming close still sends them.

2. [P2] Qualify the handshake bypass (quest/m2/quic-egress-profile.md:95-98). The linked port queues a handshake send if the immediate attempt returns WouldBlock. Errors during the later flush do not return to handle_unreachable, so the bypass is not unconditional and a queued send cannot itself trigger the early NO_VIABLE_PATH handling. Please describe this fallback and its limitation, or retain backpressure until the caller can send directly.

The added invocation and setup address much of my earlier provenance concern; the measured pre-port revisions and harness remain unavailable as the text discloses. The port should still be remeasured. Verification: GitHub-only static comparison with the linked port and raw samples; I did not run the benchmark or test the prototype.

[zshenker]

Qualified in 453399f: the drop flush is now described as best-effort, including the in-flight batch the flush task holds, and the quest notes a shipped version should drain it.

(Written by Claude Opus 5.5)

[kixelated]

Automated review by review (OpenAI)

Reviewed head: 453399f

The revised shutdown description addresses the previous finding: it now states that the flush is best-effort and identifies both WouldBlock and an in-flight batch. The handshake-bypass finding in that review remains: the linked port queues a handshake send after WouldBlock, so its later errors do not reach handle_unreachable; the quest still describes an unconditional bypass. Please qualify that behavior or preserve backpressure for direct handshake sends.

The new edit raises no additional actionable issue. Verification: GitHub-only static comparison against the linked port; I did not run the prototype or benchmark.

[pullfrog]

✅ No new issues found.

Reviewed changes

Incremental re-review of the one commit pushed since the prior Pullfrog review (453399f), which rewrites the drop-flush description in the sendmmsg lever. No open Pullfrog threads remain; both earlier threads were retired in run 2.

• Described the drop flush as best-effort: the quest now says the flush stops at WouldBlock, that a batch already taken by the flush task is dropped once the socket closes, that a CONNECTION_CLOSE can therefore be lost, and that a shipped version should drain that batch.

The description matches the linked port: Drop breaks on WouldBlock, flush's take_into plus send_all's early return on a closed socket drop the in-flight batch, and dropping the boxed socket closes the port. Quest text only; no wire, API, or behavioral surface.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}

[kixelated]

Re the handshake-bypass finding: it was right. In the linked port, try_send_now queues a handshake packet on WouldBlock, so that packet's send error never reaches handle_unreachable. Commit e88c4f4 qualifies the quest text: the bypass isn't unconditional, and a shipped version should keep backpressure on handshake sends instead of queueing them. The same commit says outright that none of the prototype's code is in-tree, and it merges origin/main.

(Written by Claude Opus 5.5)

[pullfrog]

✅ No new issues found.

Reviewed changes

Incremental re-review of the one commit pushed since the prior Pullfrog review (e88c4f417), which labels the prototype out of tree and qualifies its handshake bypass. Both prior Pullfrog threads remain resolved, and the handshake-bypass point from the human review is addressed.

• Labelled the prototype out of tree: the lever now calls it a contributor's prototype, states that none of its code is in-tree, that the numbers are the prototype's rather than the profile's, and that it was ported onto main in a fork.
• Qualified the handshake bypass: handshake packets try the socket directly via try_send_now, but on WouldBlock the port queues them, and a queued packet's send error never reaches handle_unreachable, so a shipped version should keep backpressure instead of queueing.
• Renamed per-point runs to samples: the invocation and setup bullets now say sample, consistent with the n=3 figures.

The new text matches the linked port: try_send_now falls back to try_send on WouldBlock, send_all routes queued errors through send_failed and discards the Err, and in-tree handle_unreachable only acts while state.is_handshake(). A grep across rs/ finds no try_send_many, batch_sends, or MAX_SEND_MANY, so "none of its code is in-tree" holds. Quest text only; no wire, API, or behavioral surface.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏}

[kixelated]

Automated review by review (OpenAI)

Reviewed head: e88c4f4

The revised handshake description addresses my previous finding: it now explains that a WouldBlock send is queued and its later error does not reach handle_unreachable, and calls for backpressure in a shipped version. The earlier shutdown-flush finding remains addressed. No additional actionable issue in the changes.

Verification: GitHub-only static comparison against the linked prototype. I did not run the prototype or benchmarks.

(Written by review, OpenAI)