Skip to content

feat(conn): add SetWriteDeadline to bound writes without allocating - #570

Closed
sztanpet wants to merge 1 commit into
coder:masterfrom
sztanpet:feat/conn-set-write-deadline
Closed

feat(conn): add SetWriteDeadline to bound writes without allocating#570
sztanpet wants to merge 1 commit into
coder:masterfrom
sztanpet:feat/conn-set-write-deadline

Conversation

@sztanpet

Copy link
Copy Markdown

A server writing to many connections needs every write bounded, so that one
client which has stopped reading cannot wedge the goroutine writing to it.
The only way to express that bound today is a context, and a context costs an
allocation per frame — which, for one message fanned out to a room, is an
allocation per recipient. This adds a way to say the same thing with a
deadline, which costs nothing.

Concretely: Write takes a context, so the bound is a context.WithTimeout
in the caller plus, for any cancelable context, a context.AfterFunc here.
Profiling a chat server at ~49,600 deliveries/s across 500 connections put
that at about 700 bytes of garbage per delivered message, none of it the
message, and 99% of everything the server allocated.

A deadline is an instant, and an instant needs no allocation to express:

func (c *Conn) SetWriteDeadline(t time.Time) error

It stores the instant in an atomic.Int64, and writeFrame arms a
*time.Timer created once per connection, using only Reset and Stop
netconn.go already makes stopped timers this way for net.Conn deadlines.
Combined with a non-cancelable context on Write, which #566 made free, the
write path allocates nothing.

It is opt-in and fully backwards compatible. Nothing in the library calls
SetWriteDeadline; no existing signature, default or behaviour changes; and
with no deadline set writeFrame does one atomic load more than before.
Callers that never touch it cannot tell the difference.

An expired deadline closes the connection, as an expired context does, and
refuses that frame with os.ErrDeadlineExceeded. A zero t clears it. The
timer is armed under writeFrameMu so that setting a deadline cannot disturb
a frame in flight; it is also why an expired deadline cannot close inline and
leaves that to the timer's goroutine, since close takes the same lock via
msgWriter.close.

Evidence

Same server and load, per-frame context replaced by a deadline, measured at
9f84ce4. Latency is measured at the client, the rest is pprof on the
server, built without -race.

context per frame deadline
garbage per delivered message ~700 B ~0
allocated in a 20s window 689 MB 0 sampled
CPU samples per 20s 14.33s 12.65s
write(2) share of CPU 63.1% 70.9%
mean delivery latency 4.20 ms 3.55 ms

12% less CPU for identical delivered work and 15% off mean latency. Mean
latency held 3.53-3.55 ms across four deadline runs where every baseline run
of this load sat at 3.75-4.24 ms; the bands do not overlap.
TestSetWriteDeadlineAllocs asserts the allocation claim in the suite
itself, via testing.AllocsPerRun.

Tradeoff

The deadline belongs to the connection rather than to a call, so where
several goroutines write to one connection the last to set it wins. Frames
written under one deadline share that instant instead of restarting the
clock, which is usually what a caller batching frames into one wakeup wants.
It does not arise for callers whose writers share a single write timeout, but
it is a real narrowing against a per-call context.

Alternatives considered

  • Memoizing the armed context across Write calls. Measured: it removes
    the per-frame cost when a caller batches, nothing when each wakeup carries
    one frame, and it must keep the deadline armed past the last frame.
  • An OS deadline on the underlying socket, set from outside the library.
    Also free, but it aborts mid-frame and leaves the stream at an unknown
    offset where this closes cleanly.

Not in this PR

  • No SetReadDeadline. The symmetric change wants its own thought about
    CloseRead and ping handling.
  • No net.Conn on Conn. This is one method, not a step toward that;
    NetConn stays the way to get it.

Refs #565, #566

Bounding a write requires a context, so a caller that wants a write
deadline must allocate one per frame — a context.WithTimeout on its side
and, for any cancelable context, a context.AfterFunc on ours. A server
fanning one message out to many connections pays that per recipient rather
than per message, where it is the dominant cost of sending: profiling a
chat server holding 500 connections in one room at ~49,600 message
deliveries a second attributed about 700 bytes of garbage per delivered
message to it, and 99% of everything the server allocated under that load.

A deadline is an instant, and an instant needs no allocation to express.
SetWriteDeadline stores one in an atomic and writeFrame arms a timer
created once per connection, using only Reset and Stop — the approach
netconn.go already takes for net.Conn deadlines, exposed on Conn so a
caller that can name its bound as a deadline does not have to buy a
context to carry it. Paired with a non-cancelable context on Write, which
coder#566 already made free, the write path allocates nothing.

Existing callers are unaffected: the deadline is unset by default, and
with none set writeFrame does one atomic load more than before.

On the same server and load, replacing the per-frame context with a
deadline: allocation over a 20s window falls from 689MB to zero sampled,
CPU samples for identical delivered work from 14.33s to 12.50s, write(2)
from 63.1% to 73.1% of the profile, runtime.mallocgc from 4.82% to below
the reporting threshold, and mean delivery latency measured at the client
from 4.20ms to 3.53ms.

Two details worth noting for review. The timer is armed by writeFrame
rather than by SetWriteDeadline, so that it is armed under writeFrameMu
together with the frame it bounds and cannot disturb a frame already in
flight. And armWriteDeadline must not close the connection itself: close
takes writeFrameMu for a client connection via msgWriter.close, so closing
inline deadlocks, which is why an expired deadline refuses the frame with
os.ErrDeadlineExceeded and leaves the closing to the timer's goroutine.
It reports bools rather than returning a stop closure for the reason coder#565
gives on the read path — a closure over the connection is an allocation
per frame.

ws_js.go gets the same method so the API does not differ by platform.
Writes there are handed to the browser and never block, so the deadline is
checked when a write is attempted rather than interrupting one.

Refs coder#565, coder#566
@mafredri

Copy link
Copy Markdown
Member

Closing as we don't have the capacity to review fully AI generated PRs in this repository at this time. Our guidelines need to be updated but some level of human involvement must be demonstrated, and the PR description written by a human demonstrating they understand the issue and solution, as well as having self-reviewed the full set of changes.

Also, please try to avoid LLM minted terminology/jargon like "wedge" and "arm", to name two I happened to spot at a glance.

If you want to work on this more, feel free to re-open once you've made amendments, or open a new one.

@mafredri mafredri closed this Jul 27, 2026
@sztanpet

Copy link
Copy Markdown
Author

sure, will rewrite soon, thanks

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