perf(input): isolate keyboard input path from pty output processing#21
Conversation
Under heavy pty output, key-to-echo latency degraded (median 1.2ms idle -> 1.7ms loaded, p95 3.1ms): the key-encode path took the terminal lock three times against the io thread's chunk feed, and input bytes routed through the io thread waited out output batches of up to 1MiB. - read the three keyboard-encoding modes under a single terminal lock - write local input (keyboard/paste/IME) straight to the pty writer thread after reserving on the shared byte budget, bypassing the io thread's output-batch loop; IPC-attach input keeps the channel path - share input_echo_pending as an AtomicBool between the main-thread write path and the io thread's redraw decision
Consuming the shared input_echo_pending bool with a plain store(false) could clobber a reservation made between the io thread's load and its clear: the next key's echo then fell back to the normal redraw floor, delaying it up to one floor interval during heavy output. Track an input write generation instead and record only the generation observed at redraw-decision time, so a racing reservation stays owed.
Bumping input_echo_seq at queue time let an output batch the io thread was already parsing — necessarily pre-echo output — consume the echo debt, so the actual echo fell back to the normal redraw floor. Wrap the reserved input in EchoStampedInput, whose Drop on the writer thread advances the generation only once the real PTY write completes; both the main-thread direct path and the IPC-attach forward use it.
When the window lands on a scale factor the monitor probe didn't predict, the probe-sized surface was configured and presented first and then resized by the post-join correction, so a second CAMetalLayer drawable generation was allocated and the stale one stayed resident. On the mismatch path, join the terminal font stack up front, settle the corrected size before the first configure/present, and reuse the prebuilt font for grid construction; the matched-scale path still presents the startup frame before the join to keep first paint early.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8bd4f1cf6f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // pty-output batch. The reservation travels with the bytes and is | ||
| // released after the real write, preserving the overflow-cap | ||
| // accounting `PtyInputQueue::queue` would apply. | ||
| SurfaceTransport::Local(local) => match local.pty_input_tx.reserve(bytes) { |
There was a problem hiding this comment.
When a raw attach/IPC input is already sitting in PtyInputQueue (especially in the overflow drainer), this fast path only reserves budget and then writes the later local key/paste directly to PtyWriter, so it can reach the shell before earlier attach bytes that are still waiting for the io thread to forward them. RawAttachTap::queue_input still uses PtyInputQueue::queue, whose overflow path exists to keep later keys behind deferred paste bytes; bypassing that queue corrupts the PTY byte-stream order whenever local and attach input overlap under backpressure.
Useful? React with 👍 / 👎.
1 participant
Summary
Under heavy pty output, keyboard input latency degraded and got noisy (idle median 1.20ms → loaded 1.70ms, p95 3.1ms, stddev 0.845ms, key→present). Two mechanisms, both fixed:
key_encode_modes).The echo-repaint debt that lets an input's echo bypass the redraw floor is now a generation counter (
input_echo_seq): the writer thread advances it as each real pty write completes (EchoStampedInput's drop), and the io thread consumes only the generation it observed at decision time — so a racing reservation is never clobbered, and output parsed before the write can't consume the debt.Also includes one standalone startup fix (
8bd4f1c): on a monitor/window scale-factor mismatch, settle the corrected size before the first configure/present so only one CAMetalLayer drawable generation is ever allocated.Test plan
cargo test --workspacegreen (noa-pty / noa-ipc suites run unsandboxed),cargo clippyandcargo fmt --checkcleanlatency_trace) to confirm the p95/stddev improvement is pending