perf(server): batch terminal output and bound replay buffer eviction - #1729
Merged
Merged
Conversation
Coalesce PTY output bursts into <=64 KiB host events on a 2ms window so each onData callback no longer pays a full IPC, Zod, and WebSocket frame per chunk. Apply PTY-level backpressure (pause/resume) when the IPC queue crosses 768/512 KiB watermarks instead of letting a flood trip the 1 MiB outbound cap and kill the host; a per-session 4 MiB pending bound kills a session that outruns the pipe, and a 2s deadline keeps an exited session's exit event publishable so closes cannot wedge the serial queue. In the server runtime, evict replay chunks by advancing a head index with cumulative byte offsets instead of Array.shift plus a full-array scan on every append while a checkpoint exists, and drop two redundant Uint8Array copies per output event. The PTY host also stops Zod-parsing its own outbound events; the supervisor already validates every inbound event at the trust boundary.
A 4 MiB bound is reachable within the pause-latency window when the server stalls during heavy output, which would kill a developer's terminal for a transient stall. The pause/resume backpressure is the primary defense; the bound now only trips when pause fails to stem a sustained flood, at which point the session was already undeliverable.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The terminal output path paid a fixed serialization gauntlet per PTY chunk and had no flow control. This PR batches output at the host, adds real backpressure, and removes quadratic decay in the server-side replay buffer.
publishOutputnow buffers per session and flushes on a 2ms window (or immediately at a full 64 KiB chunk), collapsing output bursts into far fewer IPC events, Zod parses, WS frames, andxterm.writecalls.outputSeqstays contiguous; no contract change.outboundBytespast the 1 MiB cap, makingpublishthrow andfailHostkill every session on every thread.exitevent publishes (bounded by a 2s deadline that drops unsequenced bytes), soclose/shutdowncannot wedge the serial inbound queue. Inbound commands are dropped for exited-but-mapped sessions instead of throwing intofailHost.Array.shift(O(n) per append at capacity), and per-chunk cumulativeendBytesmakesbytesAfterO(1) instead of a full-array reduce on every append while a checkpoint exists.handleOutputno longer copies each decoded buffer twice more for headless/attached delivery.Verified:
bun run --cwd apps/server test --on the five touched suites, 64/64 pass including new tests for coalescing, immediate full-chunk flush, pressure pause/resume, hysteresis band, exit-after-flush ordering, pressured-exit deadline, and the 4 MiB kill path.tsc --noEmitclean, oxlint clean. Live proof:verify-mcode runtime healthon a rebuilt bundle;pty-host-supervisor.real.test.tsandterminal-session-runtime.real.test.ts(real ConPTY + real shell detach/reattach) pass. Two independent reviewers audited the diff; two major findings (dead-scope command window, close-under-pressure deadlock) were fixed and rechecked.Limits: known environment flakes unrelated to this diff (
EBUSYtemp-dir cleanup in composition/diagnostics tests,pty-host-runtime.realConPTY timeout) reproduce identically on the pre-change baseline.Why
Under heavy output (build logs, floods) each PTY chunk paid base64 encode, two Zod parses with canonical re-encode, two JSON stringifies, base64 decode, and multiple buffer copies, then one WS frame per chunk. A sustained flood could exceed the shared 1 MiB outbound cap and crash the entire PTY host, taking every terminal on every thread down. The replay buffer also degraded quadratically once full or while a checkpoint was retained.
UI Changes
None — server-side pipeline only.
Config Changes
None.
Review Notes
reason: "natural"because the protocol'sexitReasonenum has no fitting value; noted in a code comment. Dropped bytes were never assigned seqs, so contiguity holds.queueBytesintentionally sums inbound + outbound queue pressure; the 2s exit deadline bounds the worst-case close delay to a delay, not a deadlock.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.