From 5a5a411e1857cd3f87017f4ca83e046fb39bbc5e Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 10 Aug 2026 00:29:26 -0700 Subject: [PATCH] perf(main): skip impossible advertised URL scans (#13514) (cherry picked from commit c5023fa0ab07a9a9800f0885b6d6d7b23aec40bf) --- .../advertised-url-watcher-benchmark.mjs | 255 ++++++++++++++++++ src/main/ports/advertised-url-watcher.test.ts | 36 +++ src/main/ports/advertised-url-watcher.ts | 23 +- 3 files changed, 310 insertions(+), 4 deletions(-) create mode 100644 config/scripts/advertised-url-watcher-benchmark.mjs diff --git a/config/scripts/advertised-url-watcher-benchmark.mjs b/config/scripts/advertised-url-watcher-benchmark.mjs new file mode 100644 index 00000000000..cae7195d024 --- /dev/null +++ b/config/scripts/advertised-url-watcher-benchmark.mjs @@ -0,0 +1,255 @@ +#!/usr/bin/env node +// Measures the no-URL PTY path that every terminal chunk enters. +import { spawnSync } from 'node:child_process' +import { existsSync, readFileSync } from 'node:fs' +import nodeModule from 'node:module' +import { performance } from 'node:perf_hooks' +import { fileURLToPath } from 'node:url' + +if (!process.execArgv.includes('--experimental-transform-types')) { + const result = spawnSync( + process.execPath, + ['--experimental-transform-types', '--no-warnings', import.meta.filename], + { stdio: 'inherit' } + ) + process.exit(result.status ?? 1) +} + +nodeModule.registerHooks({ + resolve(specifier, context, nextResolve) { + if (specifier.startsWith('.') && !/\.[cm]?[jt]s$/.test(specifier) && context.parentURL) { + const candidate = new URL(`${specifier}.ts`, context.parentURL) + if (existsSync(fileURLToPath(candidate))) { + return { url: candidate.href, shortCircuit: true } + } + } + return nextResolve(specifier, context) + } +}) + +const source = readFileSync( + new URL('../../src/main/ports/advertised-url-watcher.ts', import.meta.url), + 'utf8' +) +for (const marker of [ + "return mayContainHttpUrl(finalized) ? stripTerminalControls(finalized) : ''", + 'if (chunk.length >= PER_PTY_BUFFER_LIMIT)', + 'this.raw.length + chunk.length > PER_PTY_BUFFER_LIMIT' +]) { + if (!source.includes(marker)) { + throw new Error(`advertised-url-watcher.ts no longer contains \`${marker}\``) + } +} + +const { AdvertisedUrlWatcher, extractUrlCandidates, stripTerminalControls } = await import( + new URL('../../src/main/ports/advertised-url-watcher.ts', import.meta.url).href +) + +const BUFFER_LIMIT = 4096 +const ITERATIONS = Number(process.env.ORCA_ADVERTISED_URL_BENCH_ITERATIONS ?? '10000') +const ROUNDS = Number(process.env.ORCA_ADVERTISED_URL_BENCH_ROUNDS ?? '12') +const WARMUP = Number(process.env.ORCA_ADVERTISED_URL_BENCH_WARMUP ?? '1000') + +for (const [name, value] of [ + ['ORCA_ADVERTISED_URL_BENCH_ITERATIONS', ITERATIONS], + ['ORCA_ADVERTISED_URL_BENCH_ROUNDS', ROUNDS], + ['ORCA_ADVERTISED_URL_BENCH_WARMUP', WARMUP] +]) { + if (!Number.isSafeInteger(value) || value <= 0) { + throw new Error(`${name} must be a positive integer, received ${value}`) + } +} +if (ROUNDS % 2 !== 0) { + throw new Error('ORCA_ADVERTISED_URL_BENCH_ROUNDS must be even') +} + +class BeforePtyBuffer { + raw = '' + + ingest(chunk) { + const chunkHasLineBreak = chunk.includes('\n') || chunk.includes('\r') + this.raw += chunk + if (this.raw.length > BUFFER_LIMIT) { + this.raw = this.raw.slice(-BUFFER_LIMIT) + } + if (!chunkHasLineBreak) { + return '' + } + const lastNewline = lastLineBreak(this.raw) + if (lastNewline === -1) { + return '' + } + const finalized = this.raw.slice(0, lastNewline + 1) + this.raw = this.raw.slice(lastNewline + 1) + return stripTerminalControls(finalized) + } +} + +class BeforeWatcher { + buffers = new Map() + ptyToWorktree = new Map() + + bindPty(ptyId, worktreeId) { + this.ptyToWorktree.set(ptyId, worktreeId) + } + + ingest(ptyId, chunk) { + if (!this.ptyToWorktree.has(ptyId)) { + return 0 + } + let buffer = this.buffers.get(ptyId) + if (!buffer) { + buffer = new BeforePtyBuffer() + this.buffers.set(ptyId, buffer) + } + const finalized = buffer.ingest(chunk) + return finalized ? extractUrlCandidates(finalized).length : 0 + } +} + +function lastLineBreak(text) { + for (let index = text.length - 1; index >= 0; index -= 1) { + const code = text.charCodeAt(index) + if (code === 0x0a || code === 0x0d) { + return index + } + } + return -1 +} + +function repeatToLine(text, length, sample) { + const unit = `${text}${sample}\n` + return `${unit.repeat(Math.ceil(length / unit.length)).slice(0, length - 1)}\n` +} + +function repeatWithoutLineBreak(text, length, sample) { + const unit = `${text}${sample}` + return unit.repeat(Math.ceil(length / unit.length)).slice(0, length) +} + +const fixtures = [ + { + label: 'plain 32B line', + chunks: Array.from({ length: 32 }, (_, index) => `compiled module ${index} in 183ms\n`) + }, + { + label: 'ANSI 32B line', + chunks: Array.from({ length: 32 }, (_, index) => `\x1b[2K\x1b[1GThinking ${index}...\r\n`) + }, + { + label: 'guard fallthrough line', + chunks: Array.from( + { length: 32 }, + (_, index) => `path/to/http_server: compiled module ${index}\n` + ) + }, + { + label: 'plain 4KiB line', + chunks: Array.from({ length: 32 }, (_, index) => + repeatToLine('INFO build completed module=renderer duration=12ms ', BUFFER_LIMIT, index) + ) + }, + { + label: 'ANSI 4KiB line', + chunks: Array.from({ length: 32 }, (_, index) => + repeatToLine( + '\x1b[2K\x1b[1Gthinking about compiler output and writing patches ', + BUFFER_LIMIT, + index + ) + ) + }, + { + label: 'guard fallthrough 4KiB', + chunks: Array.from({ length: 32 }, (_, index) => + repeatToLine( + 'path/to/http_server: compiled without an advertised endpoint ', + BUFFER_LIMIT, + index + ) + ) + }, + { + label: '64KiB partial chunk', + chunks: Array.from({ length: 32 }, (_, index) => + repeatWithoutLineBreak('compiler output without a line break ', 64 * 1024, index) + ) + }, + { + label: '256KiB partial chunk', + chunks: Array.from({ length: 32 }, (_, index) => + repeatWithoutLineBreak('compiler output without a line break ', 256 * 1024, index) + ) + } +] + +let checksum = 0 + +function runBefore(chunks, iterations) { + const watcher = new BeforeWatcher() + watcher.bindPty('pty', 'repo::/bench') + let found = 0 + for (let index = 0; index < iterations; index += 1) { + found += watcher.ingest('pty', chunks[index % chunks.length]) + } + checksum = Math.imul(checksum ^ found ^ watcher.buffers.get('pty').raw.length, 16777619) >>> 0 +} + +function runAfter(chunks, iterations) { + const watcher = new AdvertisedUrlWatcher() + watcher.bindPty('pty', 'repo::/bench') + for (let index = 0; index < iterations; index += 1) { + watcher.ingest('pty', chunks[index % chunks.length], index) + } + const buffer = watcher.buffers.get('pty') + checksum = Math.imul(checksum ^ (buffer?.raw.length ?? 0), 16777619) >>> 0 +} + +function median(values) { + const sorted = [...values].sort((left, right) => left - right) + const middle = Math.floor(sorted.length / 2) + return sorted.length % 2 === 0 ? (sorted[middle - 1] + sorted[middle]) / 2 : sorted[middle] +} + +function measure(fixture) { + runBefore(fixture.chunks, WARMUP) + runAfter(fixture.chunks, WARMUP) + const before = [] + const after = [] + for (let round = 0; round < ROUNDS; round += 1) { + const measureBefore = () => { + const startedAt = performance.now() + runBefore(fixture.chunks, ITERATIONS) + before.push(((performance.now() - startedAt) * 1000) / ITERATIONS) + } + const measureAfter = () => { + const startedAt = performance.now() + runAfter(fixture.chunks, ITERATIONS) + after.push(((performance.now() - startedAt) * 1000) / ITERATIONS) + } + if (round % 2 === 0) { + measureBefore() + measureAfter() + } else { + measureAfter() + measureBefore() + } + } + return { before: median(before), after: median(after) } +} + +console.log('Advertised URL watcher no-match cost in microseconds/chunk. Lower is better.') +console.log(`iterations=${ITERATIONS}, rounds=${ROUNDS}, warmup=${WARMUP}`) +console.log('fixture before after speedup') +for (const fixture of fixtures) { + const result = measure(fixture) + console.log( + `${fixture.label.padEnd(26)} ${result.before.toFixed(3).padStart(8)} ${result.after + .toFixed(3) + .padStart(10)} ${(result.before / result.after).toFixed(2).padStart(9)}x` + ) +} +console.log(`checksum=${checksum}`) +console.log( + 'Limitations: synthetic in-process scan; explicit timestamps omit the production clock read and surrounding PTY dispatch.' +) diff --git a/src/main/ports/advertised-url-watcher.test.ts b/src/main/ports/advertised-url-watcher.test.ts index d46b8268fab..17fcc5803bb 100644 --- a/src/main/ports/advertised-url-watcher.test.ts +++ b/src/main/ports/advertised-url-watcher.test.ts @@ -147,6 +147,13 @@ describe('AdvertisedUrlWatcher.ingest', () => { expect(watcher.lookup(WORKTREE, 3001)?.origin).toBe('https://local.getmontecarlo.com:3001') }) + it('preserves a split URL inside the bounded suffix', () => { + const watcher = bindFresh() + watcher.ingest(PTY, `${'x'.repeat(4080)} https://local.`) + watcher.ingest(PTY, 'example.com:3003/\n') + expect(watcher.lookup(WORKTREE, 3003)?.origin).toBe('https://local.example.com:3003') + }) + it('does not scan buffered PTY text until a line break arrives', () => { const watcher = bindFresh() const charCodeAtSpy = vi.spyOn(String.prototype, 'charCodeAt') @@ -168,6 +175,35 @@ describe('AdvertisedUrlWatcher.ingest', () => { expect(watcher.lookup(WORKTREE, 3001)?.origin).toBe('https://example.com:3001') }) + it('keeps URLs whose scheme is split by terminal controls', () => { + const watcher = bindFresh() + watcher.ingest(PTY, 'H\x1b[32mtT\x1b[0mP://example.com:3001/\n') + expect(watcher.lookup(WORKTREE, 3001)?.origin).toBe('http://example.com:3001') + }) + + it('rejects finalized lines missing any character required by an HTTP URL', () => { + const watcher = bindFresh() + for (const line of [ + 'ttp://example.com:3001/\n', + 'hhp://example.com:3001/\n', + 'htt://example.com:3001/\n', + 'http//example.com/3001\n', + 'http:example.com:3001\n' + ]) { + watcher.ingest(PTY, line) + } + expect(watcher.lookup(WORKTREE, 3001)).toBeUndefined() + }) + + it('retains exactly the latest bounded tail across oversized chunks', () => { + const watcher = bindFresh() + watcher.ingest(PTY, `https://stale.example.com:3001/${'x'.repeat(4096)}`) + watcher.ingest(PTY, ' https://fresh.example.com:3002/\n') + + expect(watcher.lookup(WORKTREE, 3001)).toBeUndefined() + expect(watcher.lookup(WORKTREE, 3002)?.host).toBe('fresh.example.com') + }) + it('sanitizes to origin (drops path, query, fragment, userinfo)', () => { const watcher = bindFresh() watcher.ingest(PTY, 'go to https://user:pass@example.com:3001/callback?token=secret#x\n') diff --git a/src/main/ports/advertised-url-watcher.ts b/src/main/ports/advertised-url-watcher.ts index befc11ab94f..3f8ca60bcce 100644 --- a/src/main/ports/advertised-url-watcher.ts +++ b/src/main/ports/advertised-url-watcher.ts @@ -66,9 +66,13 @@ class PtyBuffer { /** Append a chunk; return cleaned text up to the last newline. The tail stays buffered so a URL or ANSI sequence split across chunks survives. */ ingest(chunk: string): string { const chunkHasLineBreak = chunk.includes('\n') || chunk.includes('\r') - this.raw += chunk - if (this.raw.length > PER_PTY_BUFFER_LIMIT) { - this.raw = this.raw.slice(-PER_PTY_BUFFER_LIMIT) + // Keep the suffix directly so oversized chunks never materialize a throwaway full concatenation. + if (chunk.length >= PER_PTY_BUFFER_LIMIT) { + this.raw = chunk.slice(-PER_PTY_BUFFER_LIMIT) + } else if (this.raw.length + chunk.length > PER_PTY_BUFFER_LIMIT) { + this.raw = `${this.raw.slice(-(PER_PTY_BUFFER_LIMIT - chunk.length))}${chunk}` + } else { + this.raw += chunk } if (!chunkHasLineBreak) { return '' @@ -79,10 +83,21 @@ class PtyBuffer { } const finalized = this.raw.slice(0, lastNewline + 1) this.raw = this.raw.slice(lastNewline + 1) - return stripTerminalControls(finalized) + return mayContainHttpUrl(finalized) ? stripTerminalControls(finalized) : '' } } +function mayContainHttpUrl(text: string): boolean { + // Control stripping cannot create any character required by an HTTP scheme. + return ( + (text.includes('h') || text.includes('H')) && + (text.includes('t') || text.includes('T')) && + (text.includes('p') || text.includes('P')) && + text.includes(':') && + text.includes('/') + ) +} + function lastLineBreak(text: string): number { // Accept either \n or \r as a finalize point (\r\n is normalized later in stripTerminalControls). for (let i = text.length - 1; i >= 0; i--) {