Skip to content

conn: skip timeout callbacks for background contexts#566

Merged
kylecarbs merged 1 commit into
coder:masterfrom
mitchellh:timeout-background-fast-path
Jun 15, 2026
Merged

conn: skip timeout callbacks for background contexts#566
kylecarbs merged 1 commit into
coder:masterfrom
mitchellh:timeout-background-fast-path

Conversation

@mitchellh

@mitchellh mitchellh commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Skip per-operation timeout callback setup when the context cannot be canceled, such as context.Background().

Timeout setup now reports whether it installed a callback, so callers only clear callbacks that exist. The benchmark also joins its writer goroutine to make repeated runs reliable.

Previously, reads and writes called context.AfterFunc for every operation, even when ctx.Done() was nil. This had a measurable cost.

Similar to #565, I know this is a micro-optimization, but its basic and understandable and it produced measurable results in some of my actual workloads that were cheap!

Benchmarks

Benchmark Before After Change
Header read, background context 170.55 ns, 216 B, 6 allocs 64.19 ns, 64 B, 2 allocs -62.4% time, -70.4% bytes, -66.7% allocs
512-byte payload read, background context 305.5 ns, 216 B, 6 allocs 190.1 ns, 64 B, 2 allocs -37.8% time, -70.4% bytes, -66.7% allocs

Each operation avoids 4 allocations and 152 bytes from the unnecessary context.AfterFunc registration.

Cancelable contexts are unchanged because they still require a cancellation callback. Accordingly, BenchmarkConn/disabledCompress, which uses a cancelable context, showed no statistically significant change.

@mitchellh mitchellh force-pushed the timeout-background-fast-path branch from c326b67 to 9e4b7cd Compare June 15, 2026 03:13
Every frame read and write registered a context.AfterFunc callback so a
context cancellation could close the connection. context.Background
cannot be canceled because its Done channel is nil, but the code still
constructed, stored, stopped, and cleared a callback registration for
each operation. Skip that work when ctx.Done() is nil, and only clear a
timeout when one was actually installed. BenchmarkConn also joins its
writer goroutine after timing so repeated benchmark runs finish cleanly.

The results below compare parent 7039364 with this commit on an Apple
M4 Max using GOMAXPROCS=1 over 10 samples. A temporary in-package
harness, not included in this commit, repeatedly called one internal
frame read using either context.Background or an uncanceled
context.WithCancel. Header reads parse a minimal frame header; payload
reads copy 512 bytes from a buffered repeating reader.

Before this change, setupReadTimeout called context.AfterFunc even for
context.Background. Avoiding that dead registration reduces background
header reads from 121.95 to 29.94 ns/op and payload reads from 124.17 to
29.15 ns/op. Both fall from 152 B/op and 4 allocs/op to zero.

Cancelable contexts still need an AfterFunc registration, so they
remain at 152 B/op and 4 allocs/op. BenchmarkConn/disabledCompress uses
a cancelable context as well, so it remains at 1219 B/op and 32
allocs/op.
@mitchellh mitchellh force-pushed the timeout-background-fast-path branch from 9e4b7cd to 97a20d0 Compare June 15, 2026 03:19
@mitchellh

Copy link
Copy Markdown
Contributor Author

Rebased.

@kylecarbs kylecarbs merged commit 9c8faad into coder:master Jun 15, 2026
4 checks 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