From 5650be5200950acadda81a8c7f6c20d2c83649a6 Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Wed, 7 Oct 2026 06:00:41 +0200 Subject: [PATCH] perf(tiles): replay a capture at xterm's own pace, not one slice a frame A tile's replay (writeChunked) wrote its capture 32 KB per animation frame, so a 1 MiB load took about a second of frames, and in the grid the load queue's slot was held across all of it: tile N+1's capture waited for tile N's last frame. xterm 6 already parses its write queue in 12 ms slices and yields between them, so the slices now all go in at once (up to a 1 MiB window, since xterm's queue throws past 50 MB and Pane B's unbounded full=1 capture can reach the server's 32 MB) and the replay resolves on the callback of an empty write queued behind them, i.e. once xterm has parsed the last slice. The single-flight flag is still held for the whole replay. A disposed xterm never runs that callback, so destroy() now settles a replay in progress: a removed tile can no longer hold its flag or the grid's one load queue. Queued up front, the capture also stays in one piece during a refresh: live output written meanwhile lands after it, not between two of its slices. Measured (tileperf, 6 printing shells with 1 MiB histories, headless, n=3 interleaved A/B against the starting file, load 8.6 to 11.8): - grid fresh open, 6 tiles, all painted: 5.10 s -> 2.57 s (-50%); restore after reload: 6.49 s -> 3.86 s (-41%); per-tile replay 669 to 734 ms -> 298 to 321 ms (median). - Same work in half the time: frames over 20 ms 26% -> 40% of the (shorter) load window, about 86 -> 62 slow frames in all; longest long task on restore 304 -> 227 ms; server event-loop delay unchanged (max 111 to 122 -> 122 to 134 ms, one capture in flight throughout). - Split Pane B (the other TerminalTile) with the main terminal on WebGL and its long-task guard armed: load 1.6 to 3.8 s -> 0.8 to 1.7 s over 15 loads each; 0 long tasks of 200 ms or more either way, the guard never tripped. With an unbounded full=1 capture (about 21k lines): 2.5 to 3.4 s -> 1.9 to 2.6 s, 0 long tasks of 200 ms or more. - At checkpoint 1 (equivalent patch, n=3 to 6): fresh 6.4 -> 2.7 s, restore 8.9 -> 4.2 s, TUI-style reconnect 11.2 to 11.8 -> 6.2 s. Tests: the replay queues every slice at once and holds the flag until xterm has parsed it; a replay larger than the window goes one window at a time; a pane destroyed mid-parse settles at once; in the grid, a tile destroyed while xterm still parses its replay releases the queue and the next tile loads (fake xterm whose callbacks never run). The rAF-driven tests now hold the parse callbacks instead. All mutation-checked (no settle in destroy, settle before the parse, no window). Browser split-pane-terminal: same 1 failed / 2 passed as at the starting HEAD (the failure is in the test's own setup, before connect). Scope: PR 1 (terminal-tile.js writeChunked and destroy(); Pane B replays the same way). Moving it onto PR 1 needs its two call sites adapted (PR 1 has no _runLoad yet) and leaves the tile-grid-load-queue.test.ts hunk with PR 2. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/web/public/terminal-tile.js | 93 +++++++++++++------- test/terminal-tile-unit.test.ts | 136 +++++++++++++++++++++++------- test/tile-grid-load-queue.test.ts | 29 ++++++- 3 files changed, 195 insertions(+), 63 deletions(-) diff --git a/src/web/public/terminal-tile.js b/src/web/public/terminal-tile.js index e7d6f8a7..ff4e07c8 100644 --- a/src/web/public/terminal-tile.js +++ b/src/web/public/terminal-tile.js @@ -24,42 +24,58 @@ // How long a scroll-to-top history pull may hold Pane B's live output. const HISTORY_PULL_TIMEOUT_MS = 10000; + // How much of a replay is queued in xterm at once: a 1 MiB load goes in one + // window, and xterm's write queue throws past 50 MB, which an unbounded + // `full=1` capture (up to the server's 32 MB) would otherwise come near. + const REPLAY_WINDOW_BYTES = 1024 * 1024; + /** - * Minimal chunked write for Pane B's own xterm instance — write() in - * TERMINAL_CHUNK_SIZE slices, yielding a frame between each, instead of one - * giant synchronous write that blocks the main thread while parsing a long - * scrollback. Deliberately NOT the primary pane's chunkedTerminalWrite - * (terminal-ui.js): that one is wired into session-switch generation - * counters and the live-output gate this simpler, independently - * created/destroyed pane has no equivalent of. + * Replays a capture into a pane's own xterm: TERMINAL_CHUNK_SIZE slices, all + * of a window queued at once. xterm 6 parses its write queue in 12 ms slices + * and yields between them, so a long scrollback never becomes a long task, + * and it is not held to one slice per animation frame either (that pacing + * took about a second per 1 MiB, with the grid's load queue waiting behind + * it). Queued up front, the capture also stays in one piece: live output + * written during the parse lands after it, not between two of its slices. + * Deliberately NOT the primary pane's chunkedTerminalWrite (terminal-ui.js): + * that one is wired into session-switch generation counters and the + * live-output gate this simpler, independently created/destroyed pane has no + * equivalent of. + * + * Resolves once xterm has parsed the last slice (a write's callback runs once + * everything queued before it is parsed), so _loadBuffer() below holds its + * single-flight flag across the whole replay. A disposed xterm never runs its + * callbacks, so `setCancel` hands the owner a function that settles the + * replay at once: destroy() calls it, or the pane's flag and the grid's load + * queue would wait forever. */ - function writeChunked(terminal, buffer, isDestroyed) { - if (!buffer) return Promise.resolve(); - if (buffer.length <= TERMINAL_CHUNK_SIZE) { - terminal.write(buffer); - return Promise.resolve(); - } - // Resolves once the LAST chunk is written (or the pane was destroyed - // mid-replay), so _loadBuffer() below can hold its single-flight flag - // across the whole replay rather than just the fetch that precedes it. + function writeChunked(terminal, buffer, isDestroyed, setCancel) { + if (!buffer || !terminal) return Promise.resolve(); return new Promise((resolve) => { let offset = 0; - const writeNext = () => { - if (isDestroyed() || !terminal) { - resolve(); + let settled = false; + const settle = () => { + if (settled) return; + settled = true; + setCancel?.(null); + resolve(); + }; + const writeWindow = () => { + if (settled) return; + if (isDestroyed()) { + settle(); return; } - const chunk = buffer.slice(offset, offset + TERMINAL_CHUNK_SIZE); - offset += chunk.length; - terminal.write(chunk); - if (offset < buffer.length) { - if (typeof requestAnimationFrame === 'function') requestAnimationFrame(writeNext); - else setTimeout(writeNext, 16); - } else { - resolve(); + const end = Math.min(buffer.length, offset + REPLAY_WINDOW_BYTES); + while (offset < end) { + const chunk = buffer.slice(offset, Math.min(end, offset + TERMINAL_CHUNK_SIZE)); + offset += chunk.length; + terminal.write(chunk); } + terminal.write('', offset < buffer.length ? writeWindow : settle); }; - writeNext(); + setCancel?.(settle); + writeWindow(); }); } @@ -103,6 +119,9 @@ // Aborts the running load's fetch; destroy() uses it so a removed tile // does not hold the owner's queue for a whole deadline. this._loadAbort = null; + // Settles a replay xterm is still parsing (writeChunked): destroy() calls + // it, because a disposed xterm never runs the callback the replay awaits. + this._cancelReplay = null; // Scroll-to-top history pull (shell panes only), see _maybeLoadMoreHistory(). // `_liveQueue` is non-null from the pull's response until its finally // block: live frames are held there with their arrival time instead of @@ -640,7 +659,12 @@ this._loadAbort = null; } if (payload.terminalBuffer && this.terminal) { - await writeChunked(this.terminal, payload.terminalBuffer, () => this._destroyed); + await writeChunked( + this.terminal, + payload.terminalBuffer, + () => this._destroyed, + (cancel) => (this._cancelReplay = cancel) + ); } } catch { /* Best-effort: live output still arrives once the socket connects. */ @@ -825,7 +849,12 @@ term.write('\x1bc'); replayed = true; if (this._wsClosed) this._markerOwed = true; - await writeChunked(term, buffer, () => this._destroyed); + await writeChunked( + term, + buffer, + () => this._destroyed, + (cancel) => (this._cancelReplay = cancel) + ); if (this._destroyed || !this.terminal) return; // xterm parses asynchronously: an empty write's callback fires only // after everything before it, so the row count below is the settled one. @@ -964,6 +993,10 @@ /* Already settled. */ } this._loadAbort = null; + // Likewise a replay still parsing: the xterm is disposed below, so the + // write callback it waits for would never come. + this._cancelReplay?.(); + this._cancelReplay = null; if (this._onWheel) { this.mountEl?.removeEventListener('wheel', this._onWheel, { capture: true }); this._onWheel = null; diff --git a/test/terminal-tile-unit.test.ts b/test/terminal-tile-unit.test.ts index 17bb55de..07a46294 100644 --- a/test/terminal-tile-unit.test.ts +++ b/test/terminal-tile-unit.test.ts @@ -70,8 +70,6 @@ type PaneUnderTest = { }; const fetchMock = vi.fn(); -/** requestAnimationFrame stand-in: chunked writes queue here and are drained by hand. */ -const rafQueue: Array<() => void> = []; /** Recorded deadline timers (see the context's setTimeout); `fn` aborts the request. */ const deadlines: Array<{ fn: () => void; ms: number; cleared: boolean }> = []; const SOURCE = readFileSync(resolve(import.meta.dirname, '../src/web/public/terminal-tile.js'), 'utf8'); @@ -105,7 +103,6 @@ function loadTerminalTile() { else clearTimeout(id as Parameters[0]); }, fetch: (...args: unknown[]) => fetchMock(...args), - requestAnimationFrame: (fn: () => void) => rafQueue.push(fn), // The constants.js globals the module reads at call time. TERMINAL_CHUNK_SIZE, TERMINAL_TAIL_SIZE, @@ -179,12 +176,36 @@ function deferred() { // Every marker variant (reconnecting, session ended, refused, taken over) starts the same way. const isMarker = (data: unknown) => typeof data === 'string' && data.includes('[disconnected'); +/** + * What reached the pane's screen. A replay also queues an empty write, only to + * hear through its callback that everything before it has been parsed + * (writeChunked); it puts nothing on screen, so it is left out. + */ +const screenWrites = (pane: { terminal: FakeTerminal }) => + pane.terminal.write.mock.calls.map((call) => call[0]).filter((data) => data !== ''); + +/** + * Holds xterm's write callbacks, as a real xterm still parsing a replay does: + * the replay stays in progress until `parse()` runs the ones held so far. + */ +function holdParses(pane: { terminal: FakeTerminal }) { + const held: Array<() => void> = []; + pane.terminal.write = vi.fn((_data: string, done?: () => void) => { + if (done) held.push(done); + }); + return { + held, + parse: () => { + for (const done of held.splice(0)) done(); + }, + }; +} + /** Lets every microtask the vm-side promise chain queued run. */ const settle = () => new Promise((r) => setTimeout(r, 0)); beforeEach(() => { fetchMock.mockReset(); - rafQueue.length = 0; deadlines.length = 0; clock = 0; }); @@ -265,40 +286,88 @@ describe('TerminalTile server-refresh single-flight', () => { second.resolve(jsonResponse('replay-2')); await settle(); - expect(pane.terminal.write).toHaveBeenLastCalledWith('replay-2'); + expect(screenWrites(pane).at(-1)).toBe('replay-2'); expect(fetchMock).toHaveBeenCalledTimes(2); expect(pane._bufferLoading).toBe(false); expect(pane._bufferRefreshPending).toBe(false); }); - it('holds the flag across the chunked write, not just the fetch', async () => { + it('queues the whole replay at once and holds the flag until xterm has parsed it', async () => { const pane = makePane(); - // Three chunks: two full ones plus a tail, so the last two are queued on - // requestAnimationFrame and the replay is mid-write after the fetch lands. + const xterm = holdParses(pane); + // Three slices: two full ones plus a tail. const big = 'x'.repeat(TERMINAL_CHUNK_SIZE * 2 + 5); fetchMock.mockResolvedValueOnce(jsonResponse(big)); pane._refreshBuffer(); await settle(); - expect(pane.terminal.write).toHaveBeenCalledTimes(1); - expect(rafQueue).toHaveLength(1); + // Every slice queued at once, then the empty write whose callback ends the + // replay: nothing waits for an animation frame. + expect(pane.terminal.write.mock.calls.map((call) => call[0].length)).toEqual([ + TERMINAL_CHUNK_SIZE, + TERMINAL_CHUNK_SIZE, + 5, + 0, + ]); + // xterm is still parsing: the replay, and with it the flag, is not done. expect(pane._bufferLoading).toBe(true); - // A refresh mid-write must not clear the terminal under the chunks still - // to come, nor start a second fetch. + // A refresh mid-parse must not clear the terminal under the replay, nor + // start a second fetch. pane._refreshBuffer(); expect(pane.terminal.clear).toHaveBeenCalledTimes(1); expect(fetchMock).toHaveBeenCalledTimes(1); fetchMock.mockResolvedValueOnce(jsonResponse('after')); - rafQueue.shift()!(); - rafQueue.shift()!(); + xterm.parse(); await settle(); - - expect(pane.terminal.write).toHaveBeenCalledTimes(4); - expect(pane.terminal.write).toHaveBeenLastCalledWith('after'); + // Parsed: the coalesced refresh runs now, once. expect(pane.terminal.clear).toHaveBeenCalledTimes(2); expect(fetchMock).toHaveBeenCalledTimes(2); + expect(pane._bufferLoading).toBe(true); + + xterm.parse(); + await settle(); + expect(screenWrites(pane).at(-1)).toBe('after'); + expect(pane._bufferLoading).toBe(false); + }); + + it('queues a replay larger than the window one window at a time', async () => { + // xterm's write queue throws past 50 MB, and an unbounded `full=1` capture + // can reach the server's 32 MB: at most 1 MiB is queued before xterm has + // parsed what came before it. + const pane = makePane(); + const xterm = holdParses(pane); + fetchMock.mockResolvedValueOnce(jsonResponse('z'.repeat(TERMINAL_TAIL_SIZE + 5))); + + pane._refreshBuffer(); + await settle(); + const queued = () => pane.terminal.write.mock.calls.reduce((n, call) => n + call[0].length, 0); + expect(queued()).toBe(TERMINAL_TAIL_SIZE); + + xterm.parse(); + await settle(); + expect(queued()).toBe(TERMINAL_TAIL_SIZE + 5); + expect(pane._bufferLoading).toBe(true); + + xterm.parse(); + await settle(); + expect(pane._bufferLoading).toBe(false); + }); + + it('a replay still parsing when the pane is destroyed settles at once', async () => { + // A disposed xterm never runs a write callback: without destroy() settling + // the replay, the flag (and in the grid the one load queue) would wait forever. + const pane = makePane(); + holdParses(pane); + fetchMock.mockResolvedValueOnce(jsonResponse('replay')); + + pane._refreshBuffer(); + await settle(); + expect(pane._bufferLoading).toBe(true); + + pane.destroy(); + await settle(); expect(pane._bufferLoading).toBe(false); }); @@ -503,6 +572,7 @@ describe('TerminalTile scroll-to-top history pull', () => { it('holds live output during the replay and replays only what arrived after the capture', async () => { const pane = makePane('shell'); const term = pane.terminal; + const xterm = holdParses(pane); const response = deferred>(); fetchMock.mockReturnValueOnce(response.promise); @@ -518,24 +588,26 @@ describe('TerminalTile scroll-to-top history pull', () => { await settle(); // 200 rows (more than the pane holds, so it replays) of 400 columns each: - // three chunks, which leaves the replay mid-write once the fetch lands. + // three chunks, still being parsed once the fetch lands. const bigReplay = Array.from({ length: 200 }, () => 'y'.repeat(400)).join('\n'); expect(bigReplay.length).toBeGreaterThan(TERMINAL_CHUNK_SIZE * 2); clock = 2; // the response arrives: this is the cutoff response.resolve(jsonResponse(bigReplay)); await settle(); - expect(rafQueue).toHaveLength(1); + expect(xterm.held).toHaveLength(1); - // Arrives while the snapshot is still being written: must not land under it. + // Arrives while the snapshot is still being parsed: must not land under it. clock = 3; pane._onLiveOutput('late'); expect(term.write).not.toHaveBeenCalledWith('late'); - rafQueue.shift()!(); - rafQueue.shift()!(); + // The replay parsed, then the pull's own settle write before it scrolls. + xterm.parse(); + await settle(); + xterm.parse(); await settle(); - const written = term.write.mock.calls.map((call) => call[0]); + const written = screenWrites(pane); // 'early' went out before the reset, so the replay wiped it and it is not repeated. expect(written.indexOf('early')).toBeLessThan(written.indexOf('\x1bc')); expect(written.filter((w) => w === 'early')).toHaveLength(1); @@ -831,7 +903,7 @@ describe('TerminalTile scroll-to-top history pull', () => { second.resolve(jsonResponse('second')); await settle(); - const writes = pane.terminal.write.mock.calls.map((c) => c[0]); + const writes = screenWrites(pane); expect(writes.at(-1)).toSatisfy(isMarker); expect(writes.lastIndexOf('second')).toBe(writes.length - 2); expect(writes.filter(isMarker)).toHaveLength(1); @@ -887,25 +959,27 @@ describe('TerminalTile scroll-to-top history pull', () => { } ); - it('a close during the chunked replay writes exactly one marker, at the end', async () => { + it('a close during the replay writes exactly one marker, at the end', async () => { const pane = makePane('shell'); + const xterm = holdParses(pane); const response = deferred>(); fetchMock.mockReturnValueOnce(response.promise); const pull = pane._pullHistory(); - // Three chunks, so the replay is still mid-write once the fetch lands. + // Three chunks, still being parsed once the fetch lands. const bigReplay = Array.from({ length: 200 }, () => 'y'.repeat(400)).join('\n'); response.resolve(jsonResponse(bigReplay)); await settle(); - expect(rafQueue).toHaveLength(1); + expect(xterm.held).toHaveLength(1); - // Written now, the marker would land between two chunks of recovered history. + // Written now, the marker would land above the recovered history's end. pane._onSocketClosed(); - rafQueue.shift()!(); - rafQueue.shift()!(); + xterm.parse(); + await settle(); + xterm.parse(); await pull; - const writes = pane.terminal.write.mock.calls.map((c) => c[0]); + const writes = screenWrites(pane); expect(writes[0]).toBe('\x1bc'); expect(writes.filter(isMarker)).toHaveLength(1); expect(isMarker(writes.at(-1))).toBe(true); diff --git a/test/tile-grid-load-queue.test.ts b/test/tile-grid-load-queue.test.ts index ddb84fb1..59b564e0 100644 --- a/test/tile-grid-load-queue.test.ts +++ b/test/tile-grid-load-queue.test.ts @@ -86,9 +86,13 @@ class FakeTerminal { attachCustomKeyEventHandler() {} registerLinkProvider() {} textarea = { addEventListener() {}, removeEventListener() {} }; + /** Set by a test: write callbacks never run, as on a disposed xterm. */ + holdParse = false; write(data: string, cb?: () => void) { - this.writes.push(data); - cb?.(); + // An empty write puts nothing on screen; the replay queues one only to hear + // (its callback) that everything before it has been parsed. + if (data) this.writes.push(data); + if (!this.holdParse) cb?.(); } clear() { this.writes.push(''); @@ -412,6 +416,27 @@ describe('teardown', () => { await Promise.all(connecting); }); + it('destroying a tile while xterm still parses its replay settles the load, and the queue moves on', async () => { + // The replay waits for xterm's write callback (writeChunked), which a + // disposed xterm never runs: unsettled, the tile's load would hold the + // grid's one queue, and every other tile behind it, forever. + const { tiles } = makeGrid(['a', 'b']); + const connecting = tiles.map((t) => t.connect()); + await settle(); + tiles[0].terminal!.holdParse = true; + captures[0].answer('replay of a'); + await settle(); + // a's replay is still parsing: b waits its turn. + expect(captures.map((c) => c.url.split('/')[3])).toEqual(['a']); + + tiles[0].destroy(); + await settle(); + expect(captures.map((c) => c.url.split('/')[3])).toEqual(['a', 'b']); + captures[1].answer('b'); + await settle(); + await Promise.all(connecting); + }); + it('a capture that never answers is cut off by its deadline, and the next tile loads', async () => { vi.useFakeTimers(); const { tiles } = makeGrid(['a', 'b']);