Skip to content

avoid multiple serialize in tcp broadcast#84

Merged
ltitanb merged 2 commits into
mainfrom
tcp-broadcast
Jun 4, 2026
Merged

avoid multiple serialize in tcp broadcast#84
ltitanb merged 2 commits into
mainfrom
tcp-broadcast

Conversation

@ltitanb

@ltitanb ltitanb commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Problem

TcpConnector's SendBehavior::Broadcast ran the caller's serialise closure
once per connection. ConnectionManager::broadcast looped over every
connection and called the per-stream write_or_enqueue_with(registry, &serialise),
which in turn called serialise_frameserialise(&mut self.send_buf) against
that stream's own buffer. So broadcasting to N connections meant N
serialisations plus N independently built frame headers (each with its own
Nanos::now() send-ts).

Change

Broadcast now serialises the frame once and writes the identical bytes to
every connection.

stream.rs

  • Factored the [len][ts] header layout into a shared write_frame_header()
    so there is a single source of truth for the wire framing; serialise_frame
    now delegates to it.
  • Added build_frame_vec() for contiguous backlog allocation from borrowed
    slices.
  • Added TcpStream::write_or_enqueue_shared(registry, &header, &payload): the
    happy path does write_vectored([header, payload]) straight from the borrowed
    slices with no per-connection copy; only a blocked or partially-written
    socket allocates a backlog copy (unchanged behaviour — backpressure is
    inherently per-connection).
  • FRAME_HEADER_SIZE is now pub(crate).

connector.rs

  • ConnectionManager gained bcast_header + bcast_payload scratch buffers.
  • broadcast clears bcast_payload, runs the closure once into it, stamps a
    single shared send-ts, then loops connections calling write_or_enqueue_shared.
    Empty payloads short-circuit.

Cost

Case Before After
N clients, writable closure ×N + N header builds + N writes closure ×1 + 1 header build + N vectored writes
1 client, writable closure ×1 + write same (and callers can drop their scratch buffer → 1 fewer copy)
WouldBlock / partial per-conn backlog copy per-conn backlog copy (unchanged)

Behavioural note

A broadcast now carries a single shared send-ts across all connections
instead of a per-connection Nanos::now(). Receivers see that one timestamp via
poll_with — arguably more correct for measuring fan-out latency.

Testing

flux-network builds clean; tcp_broadcast_burst,
tcp_multi_client_backpressure, tcp_roundtrip, and tcp_dcache all pass.

@ltitanb ltitanb requested a review from a team June 1, 2026 15:24

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 4 additional findings.

Open in Devin Review

@ltitanb ltitanb merged commit 0563196 into main Jun 4, 2026
1 check passed
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.

2 participants