From 94c2f96ea46e9f0a182eb2c7ad18068ab08a2b91 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 11 Sep 2026 01:50:13 -0700 Subject: [PATCH] perf(daemon): stop scanning every cell for OSC links that cannot exist (#20077) collectHeadlessOscLinkRanges walks every cell of every row on each snapshot, and called xterm's getCell without the reuse argument its own docs recommend, so a link-free scrollback paid a CellData allocation per cell for a guaranteed empty result. Skip the scan when xterm holds no OSC 8 registration, and reuse one cell when it does. Measured over a 5000-row link-free buffer at 200 cols, same harness back to back, median of 25: 43.85ms -> 0.00ms. This is our bug, not xterm's: xterm already reuses cells in its own serializer and documents the getCell(x, cell) overload for exactly this. --- .../daemon/headless-osc-link-ranges.test.ts | 63 +++++++++++++++++++ src/main/daemon/headless-osc-link-ranges.ts | 27 ++++++-- 2 files changed, 86 insertions(+), 4 deletions(-) create mode 100644 src/main/daemon/headless-osc-link-ranges.test.ts diff --git a/src/main/daemon/headless-osc-link-ranges.test.ts b/src/main/daemon/headless-osc-link-ranges.test.ts new file mode 100644 index 00000000000..cf8f9f757e2 --- /dev/null +++ b/src/main/daemon/headless-osc-link-ranges.test.ts @@ -0,0 +1,63 @@ +import { afterEach, describe, expect, it } from 'vitest' +import { HeadlessEmulator } from './headless-emulator' + +// Why this suite: collectHeadlessOscLinkRanges skips its per-cell scan when +// xterm holds no OSC 8 registration. That skip is only safe if it can never +// fire while a link is reachable, so each case below pins one way it could. +let emulator: HeadlessEmulator | undefined + +const link = (uri: string, text: string): string => `\x1b]8;;${uri}\x1b\\${text}\x1b]8;;\x1b\\` + +afterEach(() => { + emulator?.dispose() + emulator = undefined +}) + +describe('headless OSC link ranges', () => { + it('finds a link written into the buffer', async () => { + emulator = new HeadlessEmulator({ cols: 80, rows: 24 }) + await emulator.write(`before ${link('https://example.com/a', 'CLICK')} after`) + + const ranges = emulator.getSnapshot().oscLinks ?? [] + expect(ranges).toHaveLength(1) + expect(ranges[0]).toMatchObject({ row: 0, uri: 'https://example.com/a' }) + }) + + it('returns nothing for a buffer that never emitted a link', async () => { + emulator = new HeadlessEmulator({ cols: 80, rows: 24 }) + await emulator.write('plain output with no hyperlink\r\n'.repeat(50)) + + expect(emulator.getSnapshot().oscLinks).toEqual([]) + }) + + // The dangerous case: restored ranges are seeded without xterm registering + // anything, so an early-out keyed only on the registry would drop them. + it('still maps restored ranges when the buffer itself has no link', async () => { + emulator = new HeadlessEmulator({ cols: 80, rows: 24 }) + await emulator.write('restored row') + const restored = { row: 0, startCol: 0, endCol: 4, uri: 'https://example.com/restored' } + emulator.setRestoredOscLinks([restored]) + + expect(emulator.getSnapshot().oscLinks).toEqual([restored]) + }) + + it('finds links far down a long scrollback, not just the visible screen', async () => { + emulator = new HeadlessEmulator({ cols: 80, rows: 24, scrollback: 5_000 }) + await emulator.write(`${link('https://example.com/top', 'TOP')}\r\n`) + await emulator.write('filler\r\n'.repeat(2_000)) + + const ranges = emulator.getSnapshot({ scrollbackRows: 5_000 }).oscLinks ?? [] + expect(ranges.map((range) => range.uri)).toContain('https://example.com/top') + }) + + it('keeps every distinct link when several are present', async () => { + emulator = new HeadlessEmulator({ cols: 80, rows: 24 }) + await emulator.write( + `${link('https://example.com/1', 'ONE')} ${link('https://example.com/2', 'TWO')}` + ) + + const uris = (emulator.getSnapshot().oscLinks ?? []).map((range) => range.uri) + expect(uris).toContain('https://example.com/1') + expect(uris).toContain('https://example.com/2') + }) +}) diff --git a/src/main/daemon/headless-osc-link-ranges.ts b/src/main/daemon/headless-osc-link-ranges.ts index 418a0c65166..ea017a7b928 100644 --- a/src/main/daemon/headless-osc-link-ranges.ts +++ b/src/main/daemon/headless-osc-link-ranges.ts @@ -1,10 +1,14 @@ -import type { Terminal } from '@xterm/headless' +import type { IBufferCell, IBufferLine, Terminal } from '@xterm/headless' import type { TerminalOscLinkRange } from '../../shared/terminal-osc-link-ranges' type TerminalWithOscLinks = Terminal & { _core?: { _oscLinkService?: { getLinkData: (linkId: number) => { uri?: string } | undefined + // Why read it: xterm registers every OSC 8 id here, so an empty registry + // proves the buffer holds no hyperlink and the per-cell scan can be skipped. + // Optional because it is private — an xterm that renames it just scans. + _dataByLinkId?: { size?: number } } } } @@ -14,6 +18,11 @@ type CellWithOscLink = { hasExtendedAttrs?: () => boolean } +/** True when xterm holds no OSC 8 registration at all, so no cell can carry one. */ +function hasNoRegisteredOscLinks(service: { _dataByLinkId?: { size?: number } }): boolean { + return service._dataByLinkId?.size === 0 +} + export function collectHeadlessOscLinkRanges( terminal: Terminal, scrollbackRows: number | undefined, @@ -26,9 +35,19 @@ export function collectHeadlessOscLinkRanges( return [] } const buffer = terminal.buffer.active + // Why before the scan: the walk below reads every cell of every row, and a + // session that never emitted a hyperlink — the overwhelming majority — would + // pay that for a guaranteed-empty result. `restoredLinks` still needs mapping. + if (hasNoRegisteredOscLinks(service) && restoredLinks.length === 0) { + return [] + } const startRow = scrollbackRows === undefined ? 0 : Math.max(0, buffer.length - terminal.rows - scrollbackRows) const ranges: TerminalOscLinkRange[] = [] + // Why one cell for the whole walk: xterm's getCell allocates a fresh CellData + // per call unless handed a target, which is a per-cell allocation across the + // entire scrollback. See the IBufferLine.getCell docs. + const scratchCell = buffer.getNullCell() for (let row = startRow; row < buffer.length; row += 1) { const line = buffer.getLine(row) if (!line) { @@ -38,7 +57,7 @@ export function collectHeadlessOscLinkRanges( let currentUrlId = 0 let currentStart = -1 for (let col = 0; col <= lineLength; col += 1) { - const urlId = col < lineLength ? getOscLinkIdAtCell(line, col) : 0 + const urlId = col < lineLength ? getOscLinkIdAtCell(line, col, scratchCell) : 0 if (urlId === currentUrlId) { continue } @@ -83,8 +102,8 @@ function dedupeOscLinkRanges(ranges: TerminalOscLinkRange[]): TerminalOscLinkRan }) } -function getOscLinkIdAtCell(line: { getCell: (col: number) => unknown }, col: number): number { - const cell = line.getCell(col) as CellWithOscLink | undefined +function getOscLinkIdAtCell(line: IBufferLine, col: number, scratchCell: IBufferCell): number { + const cell = line.getCell(col, scratchCell) as (IBufferCell & CellWithOscLink) | undefined // Why: OSC link IDs live in extended cell attrs; missing attrs means no link. return cell?.hasExtendedAttrs?.() && cell.extended?.urlId ? cell.extended.urlId : 0 }