comms/uniflow/tcp: parallel data sockets ("lanes") to reach NIC line rate (#3920) by cppccppccppc · Pull Request #3920 · meta-pytorch/torchcomms

cppccppccppc · 2026-09-01T01:32:17Z

Summary:

A single TCP connection is bounded by what one sender thread can push. On
MI350/eth2 that capped a 4 MiB get at ~10.8 GB/s, 46% of the 23.25 GB/s the NIC
can carry. Per-thread sampling put the responder's hottest thread at 90.6% of a
core, and no configuration change adds a thread: not batch size, not tx-depth
(the transport already pipelines a large get's chunks internally), not larger
frames, and not DRAM instead of VRAM -- DRAM measured worse.

This adds N parallel data sockets per peer connection, one reader and one sender
thread per socket, and round-robins frames across them.

lanes 1 2 4 8
GB/s 10.78 15.81 23.14 23.10

4 lanes reaches 99.5% of the NIC ceiling. Two things make that credible beyond
the median: the 4-lane spread over 4 runs is +/-0.4% where every earlier
configuration scattered by +/-20%, which is the signature of a hard ceiling
rather than a noisy bottleneck, and 8 lanes buys nothing, which is what
saturation looks like. put gains equally, 22.42 GB/s. The default is 4 lanes per
connection.

Design notes:

Lane identity comes off the wire (TcpLaneHello), not from accept order: the
dialer opens the lanes with no ordering guarantee.
The hello is skipped entirely at 1 lane, so that path stays byte-identical and
kTcpWireVersion does not move. Bumping it would make every new peer
incompatible with every old one, too high a price for a field only multi-lane
peers read. A lane-count mismatch is a clean handshake error instead.
Send is pinned to lane 0 and never striped. send()/recv() are a FIFO-matched
rendezvous, so striping would pair a SEND with the wrong recv: silent
corruption rather than an error. put/get frames each carry their own
reqId/segId/offset and so need no ordering help.
enqueueFrames keeps a whole group on one lane, so its all-or-nothing contract
costs a single mutex. Spreading a group across lanes would need every target
lane's mutex held while waiting for room, which deadlocks against the very
senders that would free it.
The per-lane queue cap is kMaxOutQueueBytes / laneCount, so the aggregate
buffered bound is unchanged however many lanes are configured.
A failed lane closes every lane's queue: one lane failing already takes the
transport down, so leaving the others admitting frames would queue work for a
connection that is gone, with no consumer.

Rollout: above 1 lane the peers exchange a hello, so a 4-lane peer cannot talk to
one that predates lanes or is pinned to 1. The binary needs to be everywhere
before the default takes effect on connections that straddle a rollout.

Cost is per connection, not per NIC: a process holding 32 peers opens 128 sockets
and 256 transport threads on that interface. Untested at high fan-out.

Differential Revision: D117545615

meta-codesync · 2026-09-01T01:32:58Z

@cppccppccppc has exported this pull request. If you are a Meta employee, you can view the originating Diff in D117545615.

…rate (meta-pytorch#3920) Summary: Pull Request resolved: meta-pytorch#3920 A single TCP connection is bounded by what one sender thread can push. On MI350/eth2 that capped a 4 MiB get at ~10.8 GB/s, 46% of the 23.25 GB/s the NIC can carry. Per-thread sampling put the responder's hottest thread at 90.6% of a core, and no configuration change adds a thread: not batch size, not tx-depth (the transport already pipelines a large get's chunks internally), not larger frames, and not DRAM instead of VRAM -- DRAM measured worse. This adds N parallel data sockets per peer connection, one reader and one sender thread per socket, and round-robins frames across them. lanes 1 2 4 8 GB/s 10.78 15.81 23.14 23.10 4 lanes reaches 99.5% of the NIC ceiling. Two things make that credible beyond the median: the 4-lane spread over 4 runs is +/-0.4% where every earlier configuration scattered by +/-20%, which is the signature of a hard ceiling rather than a noisy bottleneck, and 8 lanes buys nothing, which is what saturation looks like. put gains equally, 22.42 GB/s. The default is 4 lanes per connection. Design notes: - Lane identity comes off the wire (TcpLaneHello), not from accept order: the dialer opens the lanes with no ordering guarantee. - The hello is skipped entirely at 1 lane, so that path stays byte-identical and kTcpWireVersion does not move. Bumping it would make every new peer incompatible with every old one, too high a price for a field only multi-lane peers read. A lane-count mismatch is a clean handshake error instead. - Send is pinned to lane 0 and never striped. send()/recv() are a FIFO-matched rendezvous, so striping would pair a SEND with the wrong recv: silent corruption rather than an error. put/get frames each carry their own reqId/segId/offset and so need no ordering help. - enqueueFrames keeps a whole group on one lane, so its all-or-nothing contract costs a single mutex. Spreading a group across lanes would need every target lane's mutex held while waiting for room, which deadlocks against the very senders that would free it. - The per-lane queue cap is kMaxOutQueueBytes / laneCount, so the aggregate buffered bound is unchanged however many lanes are configured. - A failed lane closes every lane's queue: one lane failing already takes the transport down, so leaving the others admitting frames would queue work for a connection that is gone, with no consumer. Rollout: above 1 lane the peers exchange a hello, so a 4-lane peer cannot talk to one that predates lanes or is pinned to 1. The binary needs to be everywhere before the default takes effect on connections that straddle a rollout. Cost is per connection, not per NIC: a process holding 32 peers opens 128 sockets and 256 transport threads on that interface. Untested at high fan-out. Differential Revision: D117545615

Summary: Folded from two adjacent diffs; each half is stated separately below so the two arguments stay reviewable on their own terms. --- accuracy tolerance and ROCm enablement env (was D114431543) -------------- Root-caused the intermittent disagg "accuracy failures" to the accuracy harness + environment, NOT the uniflow connector. The verbose per-mismatch diff added here showed the disagg KV output is correct in every case; the failures were: 1. A degenerate forward pass on the memory-split MI350 instances emits all-'!'/empty garbage -- on the standalone baseline AND (less often) on the disagg output itself. This is environmental (it reproduces on RDMA too, and earlier host-pinning + CU_POINTER_ATTRIBUTE_SYNC_MEMOPS experiments did not change it), not a KV-transfer/connector bug. 2. Strict exact-match flags benign single-token numerical nondeterminism between the disagg and standalone forward passes (e.g. "more" vs "others", which then re-converges). Changes (vllm/fb/standalone_benchmark/disagg_entry.py): - Verbose per-mismatch accuracy diff: full got/expected, first divergence char offset, token_divergence, and the prompt, so failures can be compared token-by-token across transports/connectors. This diagnostic is what isolated the root cause. - Retry the standalone baseline with a fresh LLM up to 3x when any reference is degenerate (all-'!'/empty). - Skip prompts whose BASELINE is degenerate (do not attribute to the connector). - Symmetric guard: skip prompts whose DISAGG OUTPUT is degenerate against a valid baseline (environmental broken forward pass, not a KV/connector failure). - If no prompt is comparable (all degenerate on either side), emit status=baseline_error and fail as INCONCLUSIVE rather than a correctness failure. - Add --accuracy-max-token-divergence N (default 0 = strict exact match) to tolerate <=N differing whitespace tokens as a near-match. Changes (comms/uniflow/benchmarks/run_mast_benchmarks.py): - Forward --accuracy-max-token-divergence to disagg_entry. - MI350/MI300 (ROCm) vLLM enablement env (AMD hosts only): FLASH_ATTENTION_TRITON_AMD_ENABLE=TRUE, VLLM_ROCM_USE_AITER=0, HIPBLASLT_ALLOW_TF32=1 -- stabilizes the forward pass and eliminates the degenerate all-'!' output on MI350. --- TTFT/TPOT and --perf-only (was D114541963) ------------------------------- Adds per-phase latency instrumentation to the disagg accuracy harness plus a --perf-only flag. TTFT is measured as the prefill POST latency and TPOT as the decode POST latency divided by output length (both non-streaming, so robust to server-side streaming quirks). --perf-only skips the correctness gate and runs the perf path directly, for bandwidth/latency sweeps. Plumbed through run_mast_benchmarks.py (Workload.perf_only, arg parser, make_workload, and the forwarded --perf-only app arg). Stacked on the native TCP transport + MAST-enablement work so the same harness can profile RDMA vs TCP. Differential Revision: D114431543

…rrected numbers Summary: --tcp-sockbuf exposes the data connection's SO_SNDBUF/SO_RCVBUF so the value can be swept instead of argued about, and the bench brackets each size's timed loop with TcpTransport::logAndResetPhaseStats() so every bandwidth line is accompanied by where the time went. The script header carried get ~1.0 GB/s, which predated the pinned-staging work and was stale by roughly 9x. It misled a full round of planning, so it is corrected here to measured values along with the link baseline (200G, MTU 1500, RTT 0.046 ms, iperf3 15.4 GB/s single stream and 23.3 GB/s over 8) that makes the numbers interpretable. Recipe 8 records a disproof rather than a hypothesis. The theory was that the 1 MiB buffer pin caps a stream at window/RTT; at 0.046 ms RTT, 1 MiB permits ~22.8 GB/s, so the window was never the constraint. A 6-point sweep with 3 repeats does show the 1 MiB pin is the worst non-degenerate setting (8.63 GB/s at 1 GiB against 9.67 unpinned, disjoint ranges), but the mechanism is autotuning being disabled while the reader does a multi-MiB copy, not the bandwidth-delay product. The 64K arm is a deliberate control: it drops throughput 75%, which is what makes the flat region above 1 MiB trustworthy rather than merely consistent with a dead knob. Also records that a --no-verify arm must never be compared against a verifying one -- that mistake inverted this exact comparison once. Differential Revision: D117132684

Summary: Phase 3d makes slab-backed VRAM ReadReply copies asynchronous on the caller stream. The transport retains the receive slab and destination write reservation until CUDA event completion, polls all pending events from the EventBase so later copies can retire out of order, and drains or quarantines copies safely across query errors and shutdown. Vector-backed fallback remains synchronous because its source storage is reused immediately. The benchmark exposes --no-tcp-async-h2d for controlled comparisons and reports the active mode. # A failed completion probe is not a failed copy Two paths conflated "I could not observe this copy finish" with "this copy did not finish", and reported a transfer error for data that had demonstrably landed. In `stageAsyncH2d`, a failing `eventRecord` arrives after `memcpyAsync` has already succeeded, so the copy is in flight and only the tracking mechanism is gone. The code falls back to `waitForH2dCopy`, and if that wait succeeds the copy has completed and the destination holds the payload -- but the old code then returned the `eventRecord` error, failing the caller's get for a copy that worked. It now completes with `Ok()`. `pollPendingH2d` had the same shape: a failing `eventQuery` left its error in `result`, and a successful `waitForH2dCopy` fell through without clearing it, retiring a completed copy as failed. The recovery now sets `result = Ok()` before retirement. The sibling site at the end of `stageAsyncH2d` deliberately keeps propagating `status`: there the error is the transport stopping or the pending record failing to track, which is a real operation failure even though the synchronize proves the copy finished. Only the two probe-failure paths change. Differential Revision: D117155852

Summary: TcpAsyncAcceptTest derived the address family by comparing the param's clientHost against the "127.0.0.1" literal. That defaults every other spelling -- "localhost", "[::1]", a resolvable hostname -- to AF_INET6 without saying so, and the two socket-buffer tests feed that family to kernelDefaultRcvBuf/rcvBufForRequest. A param added later would probe the wrong family and surface as a confusing skip or a wrong expectation rather than a clear failure. Store the family on AddrFamily and state it at the two INSTANTIATE_TEST_SUITE_P entries, so a new param has to declare which family it is. A helper function would have deduplicated the comparison but kept the default. The five sites this replaces are the two family derivations in the socket-buffer tests, the socket()/sockaddr pair in AsyncAcceptRejectsNonUniflowClient, and the test-name lambda -- which now derives the name from the same field the bodies use, so the two cannot disagree. AddrFamily is defined separately in four test files; this changes only TcpAsyncAcceptTest.cpp. The same pattern remains in TcpConnTest.cpp:48 (inverse polarity) and the other three name lambdas. Differential Revision: D117407765

… sockets Summary: D117132557 threaded TcpSocketConfig into the accept path but configureAcceptedSocket consumed only socketBufSize and hardcoded the other seven fields. That is a worse shape than the old `int acceptRetryCnt` Differential Revision: D117409938

Summary: sendAllVec took (iovec*, int) and tracked its position with a separate idx cursor, so the loop carried `iov + idx`, `iovCnt - idx` and `iov[idx]` arithmetic. std::span carries the count with the pointer, which lets the cursor go away entirely: the retire step becomes iov = iov.subspan(1) and the loop condition becomes !iov.empty(). That is the real win rather than the shorter signature. The arithmetic being deleted lives in the short-write branch, which the comment there notes is not reachable on a blocking socket except via a signal mid-transfer and is not covered by tests -- so it is the least safe place in the function to keep hand-rolled index math. It also drops the static_cast<size_t> on msg_iovlen, since size() is already size_t, and matches the idiom the rest of the interface uses: std::span<const uint8_t> on send, std::span<uint8_t> on recv. <span> was already included and sendAllVec is private with one call site, so there is no ABI consideration. The call site's count becomes size_t and passes std::span{iov}.first(iovCnt), so the cast is removed rather than relocated to the caller. Kept the doc comment. A non-const std::span<iovec> conveys no more about mutation than a non-const iovec* did; what the comment carries is that the mutation is destructive bookkeeping -- the iov must not be reused after the call -- and no signature expresses that. Differential Revision: D117414473

…the transport Summary: Folded from two adjacent diffs; each half is stated separately below so the two arguments stay reviewable on their own terms. --- controller: recv header-wait and payload-drain timings (was D117632427) --- TcpConn::syncRecv() populates the existing RecvPhaseStats with the time spent waiting for the length prefix versus draining the payload, plus frame and byte counts. Splitting the two phases is what makes a slow receive interpretable: a large headerWaitNs means we were waiting on the peer, while a large payloadDrainNs means the socket itself was the limit. All four counters are relaxed fetch_add on atomics already declared in Controller.h, so this adds two steady_clock reads per frame and no synchronisation. --- transport: receive-slab hits, misses and vector receives (was D117632428) --- Adds receiveSlabAttempts_, receiveSlabMisses_, and vectorReceiveCount_ to TcpTransport and reports them on the existing tcp phases log line. A miss means the reader could not get a pinned slab and fell back to a vector-backed receive, which changes the H2D path for that frame, so distinguishing the two is necessary before drawing conclusions from an aggregate drain number. The counters are relaxed atomics incremented in readerLoop(). Differential Revision: D117632428

…rate (meta-pytorch#3920) Summary: Pull Request resolved: meta-pytorch#3920 A single TCP connection is bounded by what one sender thread can push. On MI350/eth2 that capped a 4 MiB get at ~10.8 GB/s, 46% of the 23.25 GB/s the NIC can carry. Per-thread sampling put the responder's hottest thread at 90.6% of a core, and no configuration change adds a thread: not batch size, not tx-depth (the transport already pipelines a large get's chunks internally), not larger frames, and not DRAM instead of VRAM -- DRAM measured worse. This adds N parallel data sockets per peer connection, one reader and one sender thread per socket, and round-robins frames across them. lanes 1 2 4 8 GB/s 10.78 15.81 23.14 23.10 4 lanes reaches 99.5% of the NIC ceiling. Two things make that credible beyond the median: the 4-lane spread over 4 runs is +/-0.4% where every earlier configuration scattered by +/-20%, which is the signature of a hard ceiling rather than a noisy bottleneck, and 8 lanes buys nothing, which is what saturation looks like. put gains equally, 22.42 GB/s. The default is 4 lanes per connection. Design notes: - Lane identity comes off the wire (TcpLaneHello), not from accept order: the dialer opens the lanes with no ordering guarantee. - The hello is skipped entirely at 1 lane, so that path stays byte-identical and kTcpWireVersion does not move. Bumping it would make every new peer incompatible with every old one, too high a price for a field only multi-lane peers read. A lane-count mismatch is a clean handshake error instead. - Send is pinned to lane 0 and never striped. send()/recv() are a FIFO-matched rendezvous, so striping would pair a SEND with the wrong recv: silent corruption rather than an error. put/get frames each carry their own reqId/segId/offset and so need no ordering help. - enqueueFrames keeps a whole group on one lane, so its all-or-nothing contract costs a single mutex. Spreading a group across lanes would need every target lane's mutex held while waiting for room, which deadlocks against the very senders that would free it. - The per-lane queue cap is kMaxOutQueueBytes / laneCount, so the aggregate buffered bound is unchanged however many lanes are configured. - A failed lane closes every lane's queue: one lane failing already takes the transport down, so leaving the others admitting frames would queue work for a connection that is gone, with no consumer. Rollout: above 1 lane the peers exchange a hello, so a 4-lane peer cannot talk to one that predates lanes or is pinned to 1. The binary needs to be everywhere before the default takes effect on connections that straddle a rollout. Cost is per connection, not per NIC: a process holding 32 peers opens 128 sockets and 256 transport threads on that interface. Untested at high fan-out. Differential Revision: D117545615

meta-codesync · 2026-09-03T18:05:12Z

This pull request has been merged in 1f0ea7c.

meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Sep 1, 2026

meta-codesync Bot added the meta-exported label Sep 1, 2026

meta-codesync Bot changed the title ~~comms/uniflow/tcp: parallel data sockets ("lanes") to reach NIC line rate~~ comms/uniflow/tcp: parallel data sockets ("lanes") to reach NIC line rate (#3920) Sep 1, 2026

cppccppccppc force-pushed the export-D117545615 branch from 2c99093 to 886f800 Compare September 1, 2026 07:05

cppccppccppc force-pushed the export-D117545615 branch from 886f800 to 473c5ea Compare September 1, 2026 07:13

cppccppccppc force-pushed the export-D117545615 branch from 473c5ea to 4f8ef20 Compare September 1, 2026 15:26

cppccppccppc force-pushed the export-D117545615 branch from 4f8ef20 to 0e43d7b Compare September 1, 2026 15:32

cppccppccppc force-pushed the export-D117545615 branch from 0e43d7b to 7c850ab Compare September 2, 2026 07:56

cppccppccppc force-pushed the export-D117545615 branch from 7c850ab to b027db5 Compare September 2, 2026 08:17

cppccppccppc force-pushed the export-D117545615 branch from b027db5 to b173e08 Compare September 2, 2026 08:24

Peng Chen added 8 commits September 2, 2026 12:08

cppccppccppc force-pushed the export-D117545615 branch from b173e08 to 94f52ab Compare September 2, 2026 23:50

meta-codesync Bot closed this in 1f0ea7c Sep 3, 2026

meta-codesync Bot added the Merged label Sep 3, 2026

Navigation Menu

Uh oh!

comms/uniflow/tcp: parallel data sockets ("lanes") to reach NIC line rate (#3920) - #3920

comms/uniflow/tcp: parallel data sockets ("lanes") to reach NIC line rate (#3920)#3920
cppccppccppc wants to merge 8 commits into
meta-pytorch:mainfrom
cppccppccppc:export-D117545615

cppccppccppc commented Sep 1, 2026 •

edited by meta-codesync Bot

Loading

Uh oh!

meta-codesync Bot commented Sep 1, 2026

Uh oh!

meta-codesync Bot commented Sep 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

cppccppccppc commented Sep 1, 2026 • edited by meta-codesync Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

meta-codesync Bot commented Sep 1, 2026

Uh oh!

meta-codesync Bot commented Sep 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

cppccppccppc commented Sep 1, 2026 •

edited by meta-codesync Bot

Loading