From a56b01c2a4408ef97006b49a573382962843319b Mon Sep 17 00:00:00 2001 From: johnhldiver-beep Date: Tue, 27 Oct 2026 11:21:41 +0000 Subject: [PATCH] perf(query): lazy-load the MCPServer re-export from the package entry `src/index.ts` statically re-exported MCPServer from './mcp', so every consumer of the entry -- including `codegraph query` through loadCodeGraph() -- pulled the whole MCP server module graph (daemon, proxy, engine, session, writer-lock, tools) into startup, though no query path touches it. Replace the static re-export with a lazy getter: the same value and type surface, the same ESM named-export interop (the getter is written in the exact shape cjs-module-lexer recognizes), and `./mcp` loads on first access. `mcp/version` stays eager (dependency-free, used by indexAll()). A/B benchmark (n=30, alternating): shipped query mean 3454ms -> 2957ms (-14.4%); single process 3026ms -> 2538ms (-16.1%); module graph 252 -> 227. Numbers and method in docs/benchmarks/query-startup-lazy-mcp.md. New tests lock in the laziness contract from CJS, ESM and the spawned CLI. --- CHANGELOG.md | 4 + __tests__/lazy-mcp-export.test.ts | 189 ++++++++++++++++++++++ docs/benchmarks/query-startup-lazy-mcp.md | 171 ++++++++++++++++++++ src/index.ts | 65 +++++++- 4 files changed, 428 insertions(+), 1 deletion(-) create mode 100644 __tests__/lazy-mcp-export.test.ts create mode 100644 docs/benchmarks/query-startup-lazy-mcp.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 17022e6343..54849e02ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Performance + +- `codegraph query`, `codegraph files`, `codegraph callers` and the other library-backed commands now start faster: the MCP server behind `codegraph serve` — the daemon, proxy, tool engine and session machinery — is no longer loaded at startup just because the package exports `MCPServer`. On a benchmark project this shrank `query`'s module graph by 25 modules and cut startup time by about a seventh — roughly half a second per run (exact numbers in `docs/benchmarks/query-startup-lazy-mcp.md`). SDK consumers see the same `MCPServer` export as before, loaded on first use. + ### Fixes - In TypeScript, an awaited call with type arguments like `await fetchApi("/me")` or `await api.get(url)` now links to the function it calls. Before, the call was dropped, so typed fetch wrappers looked like they had no callers. (#2586) diff --git a/__tests__/lazy-mcp-export.test.ts b/__tests__/lazy-mcp-export.test.ts new file mode 100644 index 0000000000..ce84d7d7cc --- /dev/null +++ b/__tests__/lazy-mcp-export.test.ts @@ -0,0 +1,189 @@ +/** + * The `MCPServer` re-export of `src/index.ts` is lazy (perf/query-startup-lazy-load). + * + * The old `export { MCPServer } from './mcp'` pulled the whole MCP module + * graph — daemon, proxy, engine, session, writer-lock, tools — into every + * consumer of the package entry the moment it was required. The CLI's + * `query` reaches that entry through `loadCodeGraph()` and never touches the + * MCP server, so it paid for all of it at startup. See + * docs/benchmarks/query-startup-lazy-mcp.md for the before/after numbers. + * + * The contract these tests lock in: + * + * 1. Requiring the built entry loads no `dist/mcp` module beyond the + * dependency-free `mcp/version`. + * 2. Touching `MCPServer` loads `./mcp` on demand and yields exactly the + * class `./mcp` exports (same identity, same constructor behaviour). + * 3. `import { MCPServer } from 'codegraph'` keeps working from ESM — the + * lazy getter is deliberately written in the shape `tsc` emits for a + * static re-export, because CommonJS named-export detection + * (cjs-module-lexer) recognizes nothing else. + * 4. `codegraph query` — the command this optimization is for — runs its + * whole startup without loading the MCP server graph. + * + * The CLI entry itself eagerly imports a handful of small `mcp/*` support + * modules for its supervision/handshake plumbing (see `src/bin/codegraph.ts`): + * those are unchanged by this work and are not part of the MCP server graph + * that `src/index.ts` used to pull in. + */ +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { spawnSync } from 'child_process'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; + +const ROOT = path.resolve(__dirname, '..'); +const ENTRY = path.join(ROOT, 'dist', 'index.js'); +const BIN = path.join(ROOT, 'dist', 'bin', 'codegraph.js'); + +/** + * The `mcp/*` modules `src/bin/codegraph.ts` loads for supervision and the + * daemon handshake, independently of `src/index.ts`. Everything else under + * `mcp/` — `index.js`, `tools.js`, `engine.js`, `session.js`, `proxy.js`, + * `daemon.js`, `transport.js`, … — is the server graph `query` must not load. + */ +const CLI_SUPPORT_MCP_MODULES = new Set([ + 'mcp/daemon-paths.js', + 'mcp/daemon-registry.js', + 'mcp/early-ppid.js', + 'mcp/liveness-watchdog.js', + 'mcp/ppid-watchdog.js', + 'mcp/process-identity.js', + 'mcp/version.js', + 'mcp/writer-lock.js', +]); + +/** + * JS that prints `require.cache`'s `dist/mcp` modules as `mcp/` names. + * The path match and the normalization are separator-agnostic (Windows uses + * backslashes). + */ +const mcpModulesScript = `Object.keys(require.cache) + .filter((m) => /[\\\\/]dist[\\\\/]mcp[\\\\/]/.test(m)) + .map((m) => m.replace(/^.*[\\\\/]dist[\\\\/]mcp[\\\\/]/, 'mcp/')) + .sort()`; + +function node(script: string, extraArgs: string[] = []): string { + const r = spawnSync(process.execPath, [...extraArgs, '-e', script], { + cwd: ROOT, + encoding: 'utf-8', + env: { ...process.env, CODEGRAPH_TELEMETRY: '0' }, + }); + if (r.status !== 0) { + throw new Error(`node ${extraArgs.join(' ')} exited ${r.status}: ${(r.stdout ?? '') + (r.stderr ?? '')}`); + } + return (r.stdout ?? '').trim(); +} + +describe('the lazy MCPServer re-export of the package entry', () => { + it('loads no MCP server module until MCPServer is touched, then resolves to the real class', () => { + const out = node(` + const entry = require(${JSON.stringify(ENTRY)}); + const mods = () => ${mcpModulesScript}; + const before = mods(); + // The export is listed before it is ever read. + const listed = Object.keys(entry).includes('MCPServer'); + const after = mods(); + const Server = entry.MCPServer; + const loaded = mods(); + const identity = Server === require(${JSON.stringify(path.join(ROOT, 'dist', 'mcp', 'index.js'))}).MCPServer; + const instance = new Server('/tmp/lazy-mcp-export'); + console.log(JSON.stringify({ before, listed, after, loaded, identity, + construct: instance instanceof Server, + name: Server.name, + surface: ['default', 'CodeGraph', 'QueryBuilder', 'initGrammars', 'FileWatcher', 'LockUnavailableError', 'Mutex'] + .every((k) => typeof entry[k] === 'function') })); + `); + const r = JSON.parse(out); + // (1) Nothing from the MCP server graph at load time — at most the + // dependency-free mcp/version, which this entry imports eagerly on + // purpose. + expect(r.before.length).toBeLessThanOrEqual(1); + expect(r.before.every((m: string) => m.endsWith('mcp/version.js'))).toBe(true); + // Listing the export does not trigger the load. + expect(r.listed).toBe(true); + expect(r.after).toEqual(r.before); + // (2) First access loads ./mcp and is the real class. + expect(r.loaded.length).toBeGreaterThan(5); + expect(r.identity).toBe(true); + expect(r.construct).toBe(true); + expect(r.name).toBe('MCPServer'); + // The rest of the export surface is untouched. + expect(r.surface).toBe(true); + }); + + it('keeps the ESM named export working', () => { + const out = node(` + import { MCPServer, CodeGraph } from ${JSON.stringify(ENTRY)}; + const instance = new MCPServer('/tmp/lazy-mcp-export-esm'); + console.log(JSON.stringify({ + type: typeof MCPServer, + name: MCPServer.name, + construct: instance instanceof MCPServer, + codegraph: typeof CodeGraph, + })); + `, ['--input-type=module']); + const r = JSON.parse(out); + expect(r.type).toBe('function'); + expect(r.name).toBe('MCPServer'); + expect(r.construct).toBe(true); + expect(r.codegraph).toBe('function'); + }); + + describe('codegraph query', () => { + let root: string; + + beforeAll(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-lazy-mcp-')); + fs.mkdirSync(path.join(root, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(root, 'src', 'handler.ts'), + 'export function handleEvent(event: string): string {\n return event;\n}\n', + ); + const cg = await CodeGraph.init(root, { index: true }); + cg.close(); + }); + + afterAll(() => { + fs.rmSync(root, { recursive: true, force: true }); + }); + + it('never loads the MCP server graph while starting up and searching', () => { + // The preload dumps require.cache's dist/mcp modules at exit. The CLI + // re-executes itself when WASM runtime flags are missing, and the + // re-exec'd child would not carry the preload — skip the relaunch so + // the probe observes the process that actually runs the query (the + // same env the other CLI suites pass). + const probe = path.join(root, 'module-probe.cjs'); + fs.writeFileSync( + probe, + `const fs = require('fs'); +process.on('exit', () => { + const mods = ${mcpModulesScript}; + fs.writeFileSync(process.env.CG_MODULE_PROBE_OUT, JSON.stringify(mods)); +}); +`, + ); + const probeOut = path.join(root, 'module-probe.json'); + const r = spawnSync(process.execPath, ['-r', probe, BIN, 'query', 'handleEvent', '--json', '--path', root], { + cwd: ROOT, + encoding: 'utf-8', + env: { + ...process.env, + CODEGRAPH_TELEMETRY: '0', + CODEGRAPH_WASM_RELAUNCHED: '1', + CODEGRAPH_NO_DAEMON: '1', + CG_MODULE_PROBE_OUT: probeOut, + }, + }); + expect(r.status).toBe(0); + // The search really ran through the library entry. + expect((r.stdout ?? '')).toContain('handleEvent'); + const loaded: string[] = JSON.parse(fs.readFileSync(probeOut, 'utf-8')); + expect(loaded.every((m) => CLI_SUPPORT_MCP_MODULES.has(m))).toBe(true); + expect(loaded).not.toContain('mcp/index.js'); + expect(loaded).not.toContain('mcp/tools.js'); + }); + }); +}); diff --git a/docs/benchmarks/query-startup-lazy-mcp.md b/docs/benchmarks/query-startup-lazy-mcp.md new file mode 100644 index 0000000000..83c64d78fd --- /dev/null +++ b/docs/benchmarks/query-startup-lazy-mcp.md @@ -0,0 +1,171 @@ +# `query` startup: the lazy `MCPServer` re-export + +`codegraph query` used to pay for the whole MCP server module graph — daemon, +proxy, engine, session, writer-lock, the 8.3k-line `tools` module — at +startup, because the package entry re-exported `MCPServer` statically and the +CLI's `query` command loads that entry through `loadCodeGraph()`. The query +command never touches the MCP server. This change makes the re-export lazy, +which removes the server graph from every non-MCP code path. + +Branch: `perf/query-startup-lazy-load` (base `main` @ +`69af19ace270f2e940444d514de6dde971df12df`). + +## Background + +Profiling of `codegraph query` startup showed ~88% of the time in module +loading, dominated by `stat` calls (2446/3089 samples ≈ 79%, largest single +stack only 6.8%) — i.e. the cost of resolving a large `require()` graph, not +any one hot module. The root cause is `src/index.ts`'s top-level import graph +(`extraction` with tree-sitter and every grammar, `resolution`, `mcp`, +`context`, `sync`), which `loadCodeGraph()` turns into the price of admission +for `query` and every other library command. + +Mechanically deferring all of those symbols is risky — they appear at ~52 +usage points inside `src/index.ts` alone. The heavy imports differ sharply in +how many of those usage points they have: + +| Import in `src/index.ts` | Usage points in the file | Marginal `require()` cost inside the entry* | +|---|---|---| +| `./mcp` (the `MCPServer` re-export) | **0 call sites** (one re-export line) | min −185 ms / p50 −296 ms | +| `./context` | 2 | no measurable win (within run-to-run noise) | +| `./sync` | ~8 (type uses + `new FileWatcher` + 3 `LockUnavailableError` throws) | no measurable win (within run-to-run noise) | +| `./extraction` | ~19 | min −22 ms / p50 −115 ms | +| `./resolution` | ~29 | min −119 ms / p50 −363 ms | + +\* Measured on this machine by replacing one `require()` at a time in the +built `dist/index.js` with a lazy proxy and timing `require('dist/index.js')` +cold (9 runs each, module caches cleared between runs). The container is +shared, so the smaller deltas are approximate; `./mcp` is re-confirmed +end-to-end by the benchmarks below. + +`./mcp` is the heavy import with the fewest usage points **and** the largest +safe win: `resolution` is comparable on time but is exactly the +mechanical-lazy-loading hazard above, and `context`/`sync` are small enough +that deferring them bought nothing measurable. +`src/bin/codegraph.ts`'s eager imports were measured too — every one of them +resolves in 2–11 ms (they are type-light or dependency-free by design), so +there was nothing worth deferring there. + +## What changed + +- **`src/index.ts`** — `export { MCPServer } from './mcp'` becomes a lazy + re-export: an `export const` placeholder keeps the export's type surface, + and a CommonJS `exports` getter loads `./mcp` on first access. The getter is + deliberately written in the exact shape `tsc` emits for a static re-export, + because CommonJS named-export detection (cjs-module-lexer) recognizes + nothing else — an arrow-function getter, or the same getter nested in a + block, silently breaks `import { MCPServer } from 'codegraph'` from ESM. + `mcp/version` stays eagerly imported: it is dependency-free by design and + `indexAll()` records the package version into the index. +- **`__tests__/lazy-mcp-export.test.ts`** — locks the contract (below). Both + laziness tests fail against the pre-change build; the ESM-export test passes + against both, as a regression guard for the cjs-module-lexer shape. +- **`docs/benchmarks/query-startup-lazy-mcp.md`** (this file) and a CHANGELOG + entry. + +Nothing else changes: `codegraph serve` and the daemon already reached the +server through `await import('../mcp/index')`, and no internal code path used +the re-export. + +## Behaviour preserved + +| Surface | Before | After | +|---|---|---| +| `require('@colbymchenry/codegraph').MCPServer` | the class | the same class (identity with `require('…/dist/mcp')` is `true`) | +| `new MCPServer(path)` / `instanceof` | works | works | +| `import { MCPServer } from 'codegraph'` (ESM→CJS) | works | works | +| `MCPServer` as a type (`let s: MCPServer`) | works | works (`export type MCPServer`) | +| Other named exports, default export | unchanged | unchanged | +| `dist/index.d.ts` | `export { MCPServer } from './mcp'` | `export declare const MCPServer: typeof import('./mcp').MCPServer` + `export type MCPServer = import('./mcp').MCPServer` — same value and type meanings | + +One documented caveat: the lazy getter resolves `./mcp` through CommonJS +`require()`, which exists in the built package (the only form npm ships). +When the *source* is loaded by a test runner such as vitest there is no CJS +resolution for `./mcp`, so source-level consumers should import the server +from `../src/mcp` directly. No test in the suite touches `MCPServer` through +`src/index.ts`; the error message says exactly this. + +## Benchmarks + +Workload (identical for both builds): a 5-file / 50-node TypeScript fixture +project, indexed once, then `codegraph query "Handler" --json --path ` +with `CODEGRAPH_TELEMETRY=0`. Node v22.23.1, Linux x64. Runs alternate +before/after in 3 blocks × 10 runs per configuration (`bench-ab.sh`), so slow +drift on the shared machine hits both builds equally. Two configurations are +reported because the CLI re-executes itself when WASM runtime flags are +missing: **shipped** is what a user gets; **no-relaunch** sets +`CODEGRAPH_WASM_RELAUNCHED=1` (and `CODEGRAPH_NO_DAEMON=1`) to measure a +single process. + +### Wall time — `codegraph query "Handler" --json`, 30 runs per build + +| Configuration | Build | min | p50 | mean | max | +|---|---|---|---|---|---| +| shipped | before | 3179 ms | 3424 ms | 3454 ms | 4030 ms | +| shipped | after | 2588 ms | 2970 ms | 2957 ms | 3413 ms | +| no-relaunch | before | 2525 ms | 3033 ms | 3026 ms | 3424 ms | +| no-relaunch | after | 2143 ms | 2581 ms | 2538 ms | 2977 ms | + +Mean delta: **shipped −496 ms (14.4% faster)**, no-relaunch −488 ms (16.1% +faster). The win survives the self-relaunch because the re-exec'd child pays +the same `loadCodeGraph()` cost the single-process run does. + +### Module graph + +| | before | after | +|---|---|---| +| modules loaded by `codegraph query` (single process) | 252 | 227 (−25) | +| of which `dist/mcp/*` | 26 (whole server graph) | 8 (the CLI's own support modules) | +| modules loaded by `require('dist/index.js')` | 236 (26 `mcp/*`) | 199 (1 `mcp/*`: `version`) | + +The remaining eight `mcp/*` modules of a CLI run are the small support files +`src/bin/codegraph.ts` imports for supervision and the daemon handshake — +they load with or without this change and are listed in +`__tests__/lazy-mcp-export.test.ts`. + +### Library entry load — `require('dist/index.js')`, cold module cache + +3 batches × 7 runs per build, module cache cleared between runs: + +| Build | min (per batch) | p50 (per batch) | +|---|---|---| +| before | 1494–1542 ms | 1918–1970 ms | +| after | 1277–1393 ms | 1452–1633 ms | + +That is the cost `loadCodeGraph()` adds to every library-backed CLI command, +and where the `query` wall-time delta comes from. + +## Verification + +- `npx tsc --noEmit` — clean. +- `npm run build` — clean (`dist/index.js`, `dist/index.d.ts` checked by hand + and by the tests). +- Tests — the suites covering this change are green: `lazy-mcp-export` (3/3), + `default-export-declaration` (which loads `src/index.ts` heavily) and + `cli-index-explicit-path` (spawns the built CLI) — 12/12 in one run. The + full `npm test` run was started but did not finish within the session + (single-worker run over 200+ suites on a shared container; killed after + ~25 minutes without completing) — the remaining suites are untouched by + this change and only read `dist/` or `src/` the same way the passing ones + do. +- `__tests__/lazy-mcp-export.test.ts` — 3 tests; the two laziness tests fail + when run against a build of the pre-change source, confirming they pin the + optimization rather than passing vacuously. + +## Limits and follow-ups + +- `resolution` (29 usage points) and `extraction` (~19) remain eager. On this + measurement `resolution` alone is worth a comparable amount of time + (`min` −119 ms / `p50` −363 ms inside the entry), so deferring the + search-only path from them is the next lever — but it means touching those + ~48 usage points (or restructuring `searchNodes` away from the facade), + which is deliberately out of scope for a minimal, behaviour-preserving + change. +- The small `mcp/*` support modules the CLI entry imports for supervision and + the daemon handshake (`early-ppid`, `ppid-watchdog`, `liveness-watchdog`, + `writer-lock`, `daemon-paths`, `daemon-registry`, `process-identity`) are + unchanged and still load for every command — they are a handful of small, + dependency-light files and were never part of the problem. +- Dependency upgrades are covered by PR #2885; `mcp/query-pool` workers + (#2862) and `init` (#2888) are separate efforts — nothing here touches + them. diff --git a/src/index.ts b/src/index.ts index 2610c7ad84..c5d76df0b7 100644 --- a/src/index.ts +++ b/src/index.ts @@ -104,7 +104,70 @@ export { } from './errors'; export { Mutex, FileLock, processInBatches, debounce, throttle, MemoryMonitor } from './utils'; export { FileWatcher, WatchOptions, PendingFile, LockUnavailableError } from './sync'; -export { MCPServer } from './mcp'; +/* + * `MCPServer` is re-exported lazily. + * + * The static `export { MCPServer } from './mcp'` this replaces pulled the + * whole MCP module graph — daemon, proxy, engine, session, writer-lock, + * tools — into every consumer of this entry the moment it was required. The + * CLI's `query` reaches this module through `loadCodeGraph()` and never + * touches the MCP server, but still paid for all of it at startup (see + * docs/benchmarks/query-startup-lazy-mcp.md). + * + * How the lazy value works: + * + * - The `export const` placeholder keeps the export's type surface identical + * to the old static re-export (value = the class constructor, type = an + * instance). + * - The `Object.defineProperty` getter below replaces that placeholder. It is + * written in the exact shape `tsc` emits for `export { X } from './y'`, + * except the `require('./mcp')` runs on first access instead of at module + * load. After access, `MCPServer` is the very class `./mcp` exports — same + * identity, same constructor behaviour. The exact shape is load-bearing: + * CommonJS named-export detection (cjs-module-lexer) only recognizes that + * getter form, and any other — an arrow-function getter, or a getter nested + * inside a block — makes `import { MCPServer } from 'codegraph'` fail from + * ESM. + * + * `./mcp/version` — the only mcp module this entry still loads eagerly — is + * dependency-free by design and stays eager. + */ +/** The CodeGraph MCP server, loaded from `./mcp` on first access. */ +export const MCPServer: typeof import('./mcp').MCPServer = undefined as unknown as typeof import('./mcp').MCPServer; +export type MCPServer = import('./mcp').MCPServer; + +// The lazy holder for the `./mcp` namespace. Its getter is the only place +// that touches `require('./mcp')`. `require` resolves in the built package +// (dist/, the only form npm ships) — where this re-export keeps working from +// both CommonJS and ESM. When this *source* is loaded by a test runner like +// vitest there is no CommonJS `require` for `./mcp`, so in that case import +// the server directly (`import { MCPServer } from '../src/mcp'`). +const lazyMcp = { + get MCPServer(): typeof import('./mcp').MCPServer { + try { + // eslint-disable-next-line @typescript-eslint/no-require-imports + return require('./mcp').MCPServer; + } catch (err) { + const reason = err instanceof Error ? err.message : String(err); + throw new Error( + `MCPServer could not be loaded from './mcp' (${reason}). ` + + 'In the built package this resolves through CommonJS require(); ' + + "when this source is loaded by a test runner, import it from '../src/mcp' instead.", + { cause: err }, + ); + } + }, +}; +declare const exports: Record | undefined; +// Deliberately in the exact shape `tsc` emits for `export { X } from './y'` +// (`Object.defineProperty(exports, "X", { enumerable: true, get: function () { return y.X; } })`): +// CommonJS named-export detection (cjs-module-lexer) only recognizes that +// exact shape, and any deviation — an extra descriptor field like +// `configurable`, an arrow-function getter, or a getter nested inside a +// block — makes `import { MCPServer } from 'codegraph'` fail from ESM. The +// export above supplies the type surface; this getter replaces its runtime +// placeholder (which stays configurable, so the redefinition is legal). +Object.defineProperty(exports, "MCPServer", { enumerable: true, get: function () { return lazyMcp.MCPServer; } }); /** * Options for initializing a new CodeGraph project