Repository navigation
perf(cli): kill zoom/resize freeze by bounding the history re-feed (#230) - #238
Merged
Merged
Conversation
) Zoom / unzoom / list hide-show change the view pane width. vt100 can't reflow soft-wrapped lines on `set_size`, so `replay_cached` rebuilds the parser at the new width — and re-fed the ENTIRE session history synchronously on the UI thread. A non-tool session accumulates its whole (unbounded) history in one `pending_chunk`, so on a long session every width change re-parsed megabytes, freezing the TUI. Fix: re-feed only the scrollback tail. The parser keeps just `SCROLLBACK_MAX` (5000) lines, so everything older produces lines the ring discards immediately — re-feeding it is pure waste. `reflow_tail_offset` finds the byte offset of the last `SCROLLBACK_MAX + viewport + warmup` lines and the rebuild feeds only from there. Cost is now constant regardless of history size. Alt-screen apps (codex / vim / htop) are handled separately: their grid has no scrollback to reflow and the app repaints on the resize SIGWINCH, so the width-change branch just `set_size`s the grid in place. Bounding their re-feed would be unsafe anyway — the `ESC[?1049h` enter escape lives at the very start of the stream, and a truncated tail would render into the normal screen. Measured (release, e2e `zoom_latency`, 200k-line shell session): zoom 25.2ms -> 2.3ms unzoom 18.2ms -> 2.3ms ~11x, and now flat vs history size (was linear: 40k=6.6ms, 200k=25ms, so 500k≈60ms, worse in debug). The freeze is gone. Regression guards (timing-free, run on CI): - reflow_tail_offset_bounds_long_buffer / _keeps_short_whole — the tail-offset logic. - resize_long_history_renders_correct_tail — the bounded re-feed still shows the true newest lines. - resize_in_alt_screen_keeps_alt_grid — a long alt-screen stream stays in alt-screen across a resize (enter escape not truncated); fails if the bound is applied unconditionally. - resize_long_history_rebuild_is_bounded — timing budget (generous, debug-safe) that fails by multiples if the bound is removed. Plus `crates/e2e/tests/zoom_latency.rs` (`#[ignore]`): drives the real TUI in a PTY, floods a deep history, times the zoom toggle. Scope: this fixes the `replay_cached` path (shell / codex / claude — the sessions that flood huge output). Tool-block sessions (zarvis, `replay_full`) re-feed via the items list and aren't bounded here — typically much shorter histories; a follow-up if it bites. 180 cli unit tests pass (5 new); tui_smoke + restart e2e pass.
edwin-zvs
added a commit
that referenced
this pull request
May 25, 2026
* test(e2e): repro for typing latency under multiple streaming sessions
The TUI feeds EVERY session's PTY output into its per-session
history on the main event loop (`on_notification`) — `feed_pty`
(items + a shadow parser, per byte) plus a matrix-rain word
harvest (per byte). When several background sessions stream
heavily, that feed work competes with input handling on the same
loop, so held keys in the focused session queue up behind it and
the cursor hangs then jumps multiple chars.
`crates/e2e/tests/multi_session_latency.rs` (`#[ignore]`,
hardware-dependent timing): drives the real TUI in a PTY,
measures focused-session backspace-burst settle latency with no
background activity, then again while N background sessions
flood (started via IPC, bypassing the TUI which only types into
the focused session).
Measured:
- release, 4 background floods: 2.3ms -> 6.9ms (~3x).
- debug, same: baseline settles; under load the focused burst
does NOT settle within 30s (the loop is fully starved). The
benchmark reports "SATURATED" instead of panicking.
Investigation notes (see PR): the keystroke -> feed -> render
hot path in both `app.rs` and `pty_render.rs` is byte-identical
to v0.7.1 (the only post-0.7.1 change there is the #238
width-change-only zoom fix), so this is NOT a code regression
introduced after 0.7.1 — the mechanism existed in 0.7.1 too.
The ~10x debug amplification (saturation above) is the most
likely explanation for "worse than 0.7.1" if 0.7.1 was run as a
release build.
* perf(tui): bound matrix-rain word harvest to the chunk tail
The matrix rain reveals at most one word per session per
PTY_REVEAL_GAP and uses only the most-recent word, but
observe_pty_activity scanned the entire PTY chunk on every
notification. When a background session floods output, that
O(chunk) scan ran on the main event loop for every chunk,
competing with focused-session input handling.
Scan only the last PTY_HARVEST_TAIL_BYTES (512) of each chunk.
Earlier bytes cannot surface anyway (only the most-recent word is
revealed), and the pool is already capped at PTY_WORD_POOL_MAX, so
cross-chunk accumulation behavior is preserved. Repro
(multi_session_latency, release): under-load typing penalty drops
from ~3.0x to ~2.0x baseline.
* perf(tui): bias the event loop toward input under background load
The run_loop select! had no priority, so tokio picked a ready arm
at random each iteration. When several sessions flood PTY output,
notifications.recv() is almost always ready, so input handling was
serviced only ~half the time it competed with the feed work —
adding keystroke→render latency in the focused session (the
'hold arrow key, cursor hangs then jumps' symptom).
Add `biased;` and move the input arm to the top so a ready
keystroke is always handled before draining the background
notification batch. Input is human-paced/bursty, so top priority
can't starve the lower arms: between keystrokes input_stream is
Pending and the notification + tick arms run exactly as before
(behavior-preserving). Repro (multi_session_latency, release):
under-load typing penalty drops from ~2.0x to ~1.0x baseline.
* test(tui): regression guards for the harvest + input-bias optimizations
Add a deterministic gate for each optimization, since the e2e
multi_session_latency benchmark can't reliably gate on them (the
latency delta lives in a narrow, noisy, hardware-dependent load
window — a sustained flood saturates even the fixed build, a
bounded flood sometimes finishes before the measurement window).
- matrix_rain: pty_activity_harvest_scans_only_the_chunk_tail —
a word placed before the 512-byte tail window, with the tail
carrying only digits (what a seq-style flood emits), must not be
harvested. Fails if the harvest reverts to an O(chunk) scan.
- app: run_loop_select_biases_input_ahead_of_notifications —
the input-priority win is a tokio scheduling property with no
stable runtime probe, so assert the structural invariant that
produces it: the run_loop select! is `biased;` with the input
arm ahead of the notification-drain arm. Fails if `biased;` is
dropped or the arms are reordered.
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.
Closes #230.
Root cause
Zoom / unzoom / list hide-show change the view pane width. vt100 can't reflow soft-wrapped lines on
set_size, soreplay_cachedrebuilds the parser at the new width — and re-fed the entire session history synchronously on the UI thread. A non-tool session accumulates its whole (unbounded) history in onepending_chunk, so on a long session every width change re-parsed megabytes → the freeze.Fix
Re-feed only the scrollback tail. The parser keeps just
SCROLLBACK_MAX(5000) lines; everything older produces lines the ring discards immediately, so re-feeding it is pure waste.reflow_tail_offsetfinds the byte offset of the lastSCROLLBACK_MAX + viewport + warmuplines and the rebuild feeds only from there → cost is constant regardless of history size. A 1024-line warmup margin before the retained window re-establishes terminal state (SGR/cursor) so the visible scrollback isn't parsed cold from an arbitrary cut.Alt-screen apps (codex / vim / htop) are handled separately: their grid has no scrollback to reflow and the app repaints on the resize SIGWINCH, so the width-change branch just
set_sizes the grid in place. Bounding their re-feed would be unsafe — theESC[?1049henter escape lives at the very start of the stream, and a truncated tail would render into the normal screen. (This was a correctness trap I hit while building it; the alt-screen test guards against it.)Measured
Release, e2e
zoom_latencybenchmark, 200k-line shell session:~11×, and now flat vs history size (was linear: 40k→6.6ms, 200k→25ms, so 500k≈60ms; ~10× worse in debug). The freeze is gone.
Regression guards (timing-free, run on CI)
reflow_tail_offset_bounds_long_buffer/_keeps_short_buffer_whole— the tail-offset logic.resize_long_history_renders_correct_tail— the bounded re-feed still shows the true newest lines (no dropped content).resize_in_alt_screen_keeps_alt_grid— a long alt-screen stream stays in alt-screen across a resize; fails if the bound is applied unconditionally.resize_long_history_rebuild_is_bounded— timing budget (generous, debug-safe) that fails by multiples if the bound is removed.Plus
crates/e2e/tests/zoom_latency.rs(#[ignore]): drives the real TUI in a PTY, floods a deep history, times the zoom toggle (the measurement above).Scope
Fixes the
replay_cachedpath — shell / codex / claude, the sessions that flood huge output and where the freeze bites. Tool-block sessions (zarvis,replay_full) re-feed via theitemslist and aren't bounded here; their histories are typically much shorter (chat turns, not command floods). Follow-up if it bites —replay_full's item-layout positioning makes a safe bound more involved.Test plan
cargo build --workspaceclean.tui_smoke(2) +restart(3) pass — no regression.