From 1b141b17da4ad9532487b58cd612bce4ad80cece Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Mon, 25 May 2026 12:55:59 +0300 Subject: [PATCH] perf: dedupe VS Code git read execs Reduce duplicate git rev-parse work in VS Code Cache only safe git read commands Add VS Code bridge cache coverage --- packages/vscode/src/bridge-fs-runtime.test.js | 88 ++++++++++++ packages/vscode/src/bridge-fs-runtime.ts | 132 +++++++++++++++--- 2 files changed, 202 insertions(+), 18 deletions(-) create mode 100644 packages/vscode/src/bridge-fs-runtime.test.js diff --git a/packages/vscode/src/bridge-fs-runtime.test.js b/packages/vscode/src/bridge-fs-runtime.test.js new file mode 100644 index 00000000..d7502725 --- /dev/null +++ b/packages/vscode/src/bridge-fs-runtime.test.js @@ -0,0 +1,88 @@ +import { beforeEach, describe, expect, it, mock } from 'bun:test'; +import { promisify } from 'node:util'; + +const execCalls = []; +const execMock = mock(() => { + throw new Error('exec should be called through promisify'); +}); + +execMock[promisify.custom] = (command, options) => { + execCalls.push({ command, options }); + return new Promise((resolve) => { + setTimeout(() => { + resolve({ stdout: '/repo/.git\n/repo/.git\n', stderr: '' }); + }, 10); + }); +}; + +mock.module('child_process', () => ({ + exec: execMock, +})); + +mock.module('vscode', () => ({ + workspace: { + workspaceFolders: [{ uri: { fsPath: '/workspace' } }], + fs: {}, + }, + Uri: { + file: (fsPath) => ({ fsPath }), + }, + FileType: { + Directory: 2, + }, + window: {}, +})); + +const { clearGitReadCacheForTests, handleFsBridgeMessage } = await import('./bridge-fs-runtime'); + +const deps = { + resolveUserPath: (value) => value, + listDirectoryEntries: mock(), + normalizeFsPath: (value) => value, + execGit: mock(), + searchDirectory: mock(), + resolveFileReadPath: mock(), + parseDroppedFileReference: mock(), + readUriAsAttachment: mock(), +}; + +describe('bridge fs exec git read cache', () => { + beforeEach(() => { + execCalls.length = 0; + clearGitReadCacheForTests(); + }); + + it('dedupes in-flight cacheable git reads and reuses fresh results', async () => { + const command = 'git rev-parse --absolute-git-dir --git-common-dir'; + const cwd = '/repo'; + + const [first, second] = await Promise.all([ + handleFsBridgeMessage({ id: '1', type: 'api:fs:exec', payload: { commands: [command], cwd } }, deps), + handleFsBridgeMessage({ id: '2', type: 'api:fs:exec', payload: { commands: [command], cwd } }, deps), + ]); + + expect(first?.success).toBe(true); + expect(second?.success).toBe(true); + expect(execCalls).toHaveLength(1); + + const spacedCommand = 'git rev-parse --absolute-git-dir --git-common-dir'; + const cached = await handleFsBridgeMessage({ id: '3', type: 'api:fs:exec', payload: { commands: [spacedCommand], cwd } }, deps); + + expect(execCalls).toHaveLength(1); + expect(cached?.data?.results?.[0]).toMatchObject({ + command: spacedCommand, + success: true, + stdout: '/repo/.git\n/repo/.git', + }); + }); + + it('does not cache arbitrary exec commands', async () => { + const command = 'git status --porcelain'; + const cwd = '/repo'; + + await handleFsBridgeMessage({ id: '1', type: 'api:fs:exec', payload: { commands: [command], cwd } }, deps); + await handleFsBridgeMessage({ id: '2', type: 'api:fs:exec', payload: { commands: [command], cwd } }, deps); + + expect(execCalls).toHaveLength(2); + }); +}); diff --git a/packages/vscode/src/bridge-fs-runtime.ts b/packages/vscode/src/bridge-fs-runtime.ts index b9ae8708..cbe8a1e1 100644 --- a/packages/vscode/src/bridge-fs-runtime.ts +++ b/packages/vscode/src/bridge-fs-runtime.ts @@ -36,6 +36,65 @@ type DirectoryEntry = { isDirectory: boolean; }; +type FsExecCommandResult = { + command: string; + success: boolean; + exitCode?: number; + stdout?: string; + stderr?: string; + error?: string; +}; + +const createGitReadCacheTtlMs = () => { + const raw = Number(process.env.OPENCHAMBER_GIT_READ_CACHE_TTL_MS); + if (Number.isFinite(raw) && raw >= 0) return raw; + return 30 * 1000; +}; + +const normalizeCommand = (command: unknown): string => + typeof command === 'string' ? command.trim().replace(/\s+/g, ' ') : ''; + +const isCacheableGitReadCommand = (command: string): boolean => { + const normalized = normalizeCommand(command); + return /^git rev-parse(?: --(?:absolute-git-dir|git-common-dir|show-toplevel)){1,3}$/.test(normalized); +}; + +const GIT_READ_CACHE_TTL_MS = createGitReadCacheTtlMs(); +const GIT_READ_CACHE_MAX_ENTRIES = 500; +const GIT_READ_CACHE_MAX_BYTES = 1024 * 1024; +const gitReadCache = new Map(); +const inFlightGitReadCache = new Map>(); + +const gitReadEntryBytes = (key: string, result: FsExecCommandResult): number => + key.length + (result.stdout?.length ?? 0) + (result.stderr?.length ?? 0); + +const setGitReadCacheEntry = (key: string, result: FsExecCommandResult): void => { + gitReadCache.delete(key); + gitReadCache.set(key, { result, at: Date.now() }); + + let totalBytes = 0; + for (const [entryKey, entry] of gitReadCache) { + totalBytes += gitReadEntryBytes(entryKey, entry.result); + } + + while ( + gitReadCache.size > GIT_READ_CACHE_MAX_ENTRIES || + (totalBytes > GIT_READ_CACHE_MAX_BYTES && gitReadCache.size > 1) + ) { + const oldest = gitReadCache.entries().next().value; + if (!oldest) { + break; + } + totalBytes -= gitReadEntryBytes(oldest[0], oldest[1].result); + gitReadCache.delete(oldest[0]); + } +}; + +export const clearGitReadCacheForTests = (): void => { + gitReadCache.clear(); + inFlightGitReadCache.clear(); +}; + type FsDeps = { resolveUserPath: (value: string, baseDirectory: string) => string; listDirectoryEntries: (directoryPath: string) => Promise; @@ -301,44 +360,81 @@ export async function handleFsBridgeMessage( PATH: process.env.PATH, }; - const results: Array<{ - command: string; - success: boolean; - exitCode?: number; - stdout?: string; - stderr?: string; - error?: string; - }> = []; - - for (const cmd of commands) { - if (typeof cmd !== 'string' || !cmd.trim()) { - results.push({ command: cmd, success: false, error: 'Invalid command' }); - continue; - } + const runCommand = async (cmd: string): Promise => { try { const { stdout, stderr } = await execAsync(`${shell} ${shellFlag} "${cmd.replace(/"/g, '\\"')}"`, { cwd: resolvedCwd, env: augmentedEnv, timeout: 300000, }); - results.push({ + return { command: cmd, success: true, exitCode: 0, stdout: (stdout || '').trim(), stderr: (stderr || '').trim(), - }); + }; } catch (execError) { const err = execError as { code?: number; stdout?: string; stderr?: string; message?: string }; - results.push({ + return { command: cmd, success: false, exitCode: typeof err.code === 'number' ? err.code : 1, stdout: (err.stdout || '').trim(), stderr: (err.stderr || '').trim(), error: err.message, - }); + }; } + }; + + const runCommandWithGitReadCache = async (cmd: string): Promise => { + const cacheable = GIT_READ_CACHE_TTL_MS > 0 && isCacheableGitReadCommand(cmd); + const cacheKey = cacheable ? `${resolvedCwd}\0${normalizeCommand(cmd)}` : null; + + if (cacheKey) { + const cached = gitReadCache.get(cacheKey); + if (cached && Date.now() - cached.at < GIT_READ_CACHE_TTL_MS) { + gitReadCache.delete(cacheKey); + gitReadCache.set(cacheKey, cached); + return { ...cached.result, command: cmd }; + } + if (cached) { + gitReadCache.delete(cacheKey); + } + + const inFlight = inFlightGitReadCache.get(cacheKey); + if (inFlight) { + const result = await inFlight; + return { ...result, command: cmd }; + } + } + + const runPromise = runCommand(cmd).then((result) => { + if (cacheKey && result.success) { + setGitReadCacheEntry(cacheKey, result); + } + return result; + }).finally(() => { + if (cacheKey && inFlightGitReadCache.get(cacheKey) === runPromise) { + inFlightGitReadCache.delete(cacheKey); + } + }); + + if (cacheKey) { + inFlightGitReadCache.set(cacheKey, runPromise); + } + + return runPromise; + }; + + const results: FsExecCommandResult[] = []; + + for (const cmd of commands) { + if (typeof cmd !== 'string' || !cmd.trim()) { + results.push({ command: cmd, success: false, error: 'Invalid command' }); + continue; + } + results.push(await runCommandWithGitReadCache(cmd)); } const allSucceeded = results.every((r) => r.success);