fix(rio-vt): close synchronized updates whose markers arrive split - #1902
Open
raphamorim wants to merge 1 commit into
Open
raphamorim wants to merge 1 commit into
raphamorim wants to merge 1 commit into
Conversation
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.
Fixes the synchronized-update (mode 2026) bug reported in #1894: a BSU partway through a chunk armed the timeout but let the rest of the chunk bypass the sync buffer, so an ESU split across two reads was never recognized and a one-frame TUI froze on the previous frame until the timeout.
Approach
Instead of #1894's byte-pattern scan over every chunk (which measured 18-26% slower on escape-dense input), this ports the design upstream vte 0.15 chose for the same bug in alacritty: the parser itself stops right after dispatching a BSU. Perform gains a terminated() hook, and the new Parser::advance_until_terminated returns how many bytes it consumed; the Processor routes the remainder to the sync buffer. The cut is exact for every BSU form (single-param, multi-param like \e[?2026;1004h, the DCS =1s form, and any of them split across reads), because it keys off the dispatch rather than a byte pattern. Termination can only flip on a CSI dispatch (parser lands in Ground) or a DCS hook (parser lands in DcsPassthrough), so the check sits at the entry of those two arms only: no per-byte or per-iteration cost.
For reference, ghostty and kitty cannot have this bug class: both parse every byte through the state machine and implement 2026 as render gating (ghostty skips renderer frames with a 1s watchdog; kitty's screen_pause_rendering snapshots the screen, 2s expiry). That architecture needs the renderer's cooperation, which rio-vt cannot assume: it is a library whose embedders (librio C/Swift hosts, wasm) get sync correctness from the byte-buffer design for free. Within that design, the parser-driven cut is the fix upstream alacritty shipped.
Performance
Performer takes a const REPLAY parameter distinguishing the sync-replay instantiation from the PTY hot path. This matters: a naive second advance loop gives every run helper (advance_ground, advance_csi_param_run, ...) two callers, LLVM stops inlining them, and escape-dense throughput drops 5-10% (found via samply profiles: advance_ground went from fused into the loop to a standalone symbol). With the replay path as its own monomorphization, every helper is single-caller again and inlining is restored by construction.
Benchmarked with the vt_input suite, both binaries prebuilt and run back to back against a fresh baseline (stale baselines drift several percent thermally over an hour, verified with a same-code control):
Tests
533 rio-vt tests pass. Five new ones: mid-chunk BSU with split ESU (CSI and DCS forms), multi-param BSU (which a byte-pattern scan cannot cut on), split BSU with a tail in the second chunk, and an exhaustive chunking-equivalence test that replays both streams at every chunk size from 1 upward and asserts identical results.