From 0eee1cebac0ae44341be655ffaa65bd501f982dd Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Thu, 8 Oct 2026 00:22:54 -0700 Subject: [PATCH] Remove repeated row copying from terminal redraw ingestion (#26118) --- .../runtime/terminal-tail-redraw-buffer.ts | 41 +++++++++---- .../terminal-tail-redraw-spans.test.ts | 60 +++++++++++++++++++ .../terminal-tail-row-retention.test.ts | 17 +++++- 3 files changed, 106 insertions(+), 12 deletions(-) create mode 100644 src/main/runtime/terminal-tail-redraw-spans.test.ts diff --git a/src/main/runtime/terminal-tail-redraw-buffer.ts b/src/main/runtime/terminal-tail-redraw-buffer.ts index d8971f7ea8b..2dd6d72badd 100644 --- a/src/main/runtime/terminal-tail-redraw-buffer.ts +++ b/src/main/runtime/terminal-tail-redraw-buffer.ts @@ -73,7 +73,7 @@ export function appendNormalizedToMultilineTailBufferUnwindowed( ensureCursorRow() rows[cursorRow]!.completed = false } - const writeChar = (char: string): void => { + const writeText = (text: string): void => { ensureCursorRow() markCursorRowRewritten() const row = rows[cursorRow]! @@ -82,9 +82,9 @@ export function appendNormalizedToMultilineTailBufferUnwindowed( } row.text = cursorColumn >= row.text.length - ? `${row.text}${char}` - : `${row.text.slice(0, cursorColumn)}${char}${row.text.slice(cursorColumn + 1)}` - cursorColumn += 1 + ? `${row.text}${text}` + : `${row.text.slice(0, cursorColumn)}${text}${row.text.slice(cursorColumn + text.length)}` + cursorColumn += text.length } const eraseLine = (mode: number): void => { ensureCursorRow() @@ -100,13 +100,28 @@ export function appendNormalizedToMultilineTailBufferUnwindowed( } } + let textStart = 0 + const writePendingText = (end: number): void => { + if (end > textStart) { + writeText( + end === textStart + 1 ? normalizedChunk[textStart]! : normalizedChunk.slice(textStart, end) + ) + } + textStart = end + 1 + } for (let index = 0; index < normalizedChunk.length; index += 1) { const char = normalizedChunk[index] if (char === '\n') { + writePendingText(index) ensureCursorRow() rows[cursorRow]!.completed = true newCompleteLines += 1 - retainNewlyCompletedLine(trimTerminalLineRight(rows[cursorRow]!.text)) + // Own only retained text; a short erase-to-end result can otherwise pin a large span. + const row = rows[cursorRow]! + const trimmed = trimTerminalLineRight(row.text) + const completedLine = ownRetainedString(trimmed) + row.text = trimmed === row.text ? completedLine : ownRetainedString(row.text) + retainNewlyCompletedLine(completedLine) cursorRow += 1 cursorColumn = 0 ensureCursorRow() @@ -114,19 +129,23 @@ export function appendNormalizedToMultilineTailBufferUnwindowed( continue } if (char === '\r') { + writePendingText(index) cursorColumn = 0 continue } if (char === '\u0008') { + writePendingText(index) cursorColumn = Math.max(0, cursorColumn - 1) continue } if (char === '\u001b') { + writePendingText(index) const parsed = parseAnsiControlSequence(normalizedChunk, index) if (!parsed) { continue } index = parsed.endIndex + textStart = index + 1 if (parsed.kind !== 'csi' || !hasCanonicalNumericCsiParams(parsed.params)) { continue } @@ -145,8 +164,8 @@ export function appendNormalizedToMultilineTailBufferUnwindowed( } continue } - writeChar(char) } + writePendingText(normalizedChunk.length) return finalizeRetainedTerminalRows( rows, @@ -186,7 +205,10 @@ function finalizeRetainedTerminalRows( newlyCompletedLines: string[] } { let truncated = initialTruncated - let retainedRows = rows.map((row) => ({ ...row, text: trimTerminalLineRight(row.text) })) + let retainedRows = rows.map((row) => { + const trimmed = trimTerminalLineRight(row.text) + return { ...row, text: trimmed === row.text ? row.text : ownRetainedString(trimmed) } + }) if (retainedRows.length > MAX_TAIL_LINES + 1) { const removeCount = retainedRows.length - (MAX_TAIL_LINES + 1) @@ -242,10 +264,7 @@ function finalizeRetainedTerminalRows( return { lines, - // Why only the partial: redraw rows are built character by character and never sliced from - // the chunk, but the partial is re-sliced from its own row on every chunk, so it alone can - // accumulate a backing string across frames. Owning the rows too costs 20-36% on TUI floods - // for no measured retention. + // Completed rows own their storage at newline; the final partial may still reference a span. partialLine: ownRetainedString(partialLine), redrawCursor, truncated, diff --git a/src/main/runtime/terminal-tail-redraw-spans.test.ts b/src/main/runtime/terminal-tail-redraw-spans.test.ts new file mode 100644 index 00000000000..d68ca1ad030 --- /dev/null +++ b/src/main/runtime/terminal-tail-redraw-spans.test.ts @@ -0,0 +1,60 @@ +import { describe, expect, it, vi } from 'vitest' +import { appendNormalizedToTailBuffer } from './terminal-tail-buffer' + +describe('retained terminal redraw text spans', () => { + it('overwrites a long row without copying its suffix for every code unit', () => { + const previous = 'p'.repeat(16 * 1024) + const replacement = 'x'.repeat(previous.length) + const slice = vi.spyOn(String.prototype, 'slice') + let copiedSuffixes = 0 + try { + const result = appendNormalizedToTailBuffer([previous], '', `\x1b[1A\r${replacement}\n`) + copiedSuffixes = slice.mock.calls.length + expect(result.lines).toEqual([replacement]) + expect(result.newlyCompletedLines).toEqual([replacement]) + expect(result.redrawCursor).toBeNull() + } finally { + slice.mockRestore() + } + expect(copiedSuffixes).toBeLessThan(20) + }) + + it.each([ + ['shorter overwrite', '\x1b[1A\rXY\n', 'XYcdef'], + ['extending overwrite', '\x1b[1A\r123456789\n', '123456789'], + ['erase to end', '\x1b[1A\rXY\x1b[KZ\n', 'XYZ'], + ['erase to start', '\x1b[1A\rXY\x1b[2C!\x1b[1KZ\n', ' Z'], + ['erase whole line', '\x1b[1A\rXY\x1b[2KZ\n', ' Z'], + ['backspace', '\x1b[1A\rXY\bZ\n', 'XZcdef'], + ['carriage return', '\x1b[1A\rXY\rZ\n', 'ZYcdef'], + ['split surrogate overwrite', '\x1b[1A\r😀\bZ\n', '\ud83dZcdef'], + [ + 'Unicode and non-special controls', + '\x1b[1A\r挢😀e\u0301\ud800\t\u0000\udfff\bZ\n', + '挢😀e\u0301\ud800\t\u0000Z' + ] + ])('preserves %s', (_name, chunk, expected) => { + const result = appendNormalizedToTailBuffer(['abcdef'], '', chunk) + expect(result).toEqual({ + lines: [expected], + partialLine: '', + redrawCursor: null, + truncated: false, + newCompleteLines: 1, + newlyCompletedLines: [expected] + }) + }) + + it('fills a cursor gap before a text span and retains its code-unit column', () => { + const result = appendNormalizedToTailBuffer(['abcdef'], '', '\x1b[1A\x1b[9G😀') + expect(result.lines).toEqual([]) + expect(result.partialLine).toBe('abcdef 😀') + expect(result.redrawCursor).toBeNull() + }) + + it('keeps completed trailing spaces available to a later cursor-up rewrite', () => { + const result = appendNormalizedToTailBuffer(['abcdef'], '', '\x1b[1A\rab \n\x1b[1A\x1b[5GX\n') + expect(result.lines).toEqual(['ab Xf']) + expect(result.newlyCompletedLines).toEqual(['ab f', 'ab Xf']) + }) +}) diff --git a/src/main/runtime/terminal-tail-row-retention.test.ts b/src/main/runtime/terminal-tail-row-retention.test.ts index 0e4effbf968..e49fd7fdd0c 100644 --- a/src/main/runtime/terminal-tail-row-retention.test.ts +++ b/src/main/runtime/terminal-tail-row-retention.test.ts @@ -79,6 +79,21 @@ describe('retained terminal tail row storage', () => { } }) + it('does not pin overwritten multiline redraw chunks through completed rows', () => { + let lines: string[] = [] + const before = collectHeap() + for (let index = 0; index < 200; index += 1) { + const chunk = `\x1b[0A${'x'.repeat(64 * 1024)}\rstep ${index} done\x1b[K\n` + const next = appendNormalizedToTailBuffer(lines, '', chunk) + lines = next.lines + } + const retained = collectHeap() - before + + expect(lines).toHaveLength(200) + expect(lines.at(-1)).toBe('step 199 done') + expect(retained).toBeLessThan(4 * 1024 * 1024) + }) + it('owns the redraw partial line without re-owning carried rows', () => { const own = vi.spyOn(ownership, 'ownRetainedString') try { @@ -93,7 +108,7 @@ describe('retained terminal tail row storage', () => { expect(redrawn.lines).toEqual(['row one']) expect(redrawn.partialLine).toBe('rewritten two') - // One call for the partial only: redraw rows are built character by character. + // Only the final partial is retained; carried rows need no copies. expect(own).toHaveBeenCalledTimes(1) expect(own).toHaveBeenLastCalledWith('rewritten two') } finally {