diff --git a/gitnexus/bench/analyze-phase-breakdown.md b/gitnexus/bench/analyze-phase-breakdown.md index 6706c1905..e04d48628 100644 --- a/gitnexus/bench/analyze-phase-breakdown.md +++ b/gitnexus/bench/analyze-phase-breakdown.md @@ -169,31 +169,73 @@ Two things block it today, and neither is small: would then leave the live index with fresh File content and stale symbols, instead of untouched. -## scopeResolution is memory-traffic bound, not algorithmic +## scopeResolution — 14.7s, and it is two different problems -`--cpu-prof` of a one-file-edit run, top main-thread self time: +`PROF_SCOPE_RESOLUTION=1` already exists and reports the internal split. Marks +injected around the phase supply the rest. On a one-file edit: + +| step | ms | share | +| ---------------------------- | -------: | ------: | +| ParsedFile store rehydration | 4426 | 30% | +| **`emit`** | **7161** | **49%** | +| `resolve` | 966 | 7% | +| `finalize` | 552 | 4% | +| `extract` | 369 | 3% | +| per-language teardown, misc | ~750 | 5% | + +`extract` is small because the parse cache works: 2121/2121 pre-extracted hits +on the TypeScript pass. Nothing here re-parses. + +### Rehydration: three full store scans, one per language + +The corpus resolves three languages — python (54 files), typescript (2121), +javascript (59) — and `loadParsedFilesForPaths` walks **every** shard on each +pass. The store is 413 shards / 301 MB. A pass that wants a single file costs +335 ms, because the skip decision needs the envelope's path listing and that +listing is only trustworthy after the payload digest is checked. + +So the fixed cost is paid per language, and **it scales with language count**, +not with how many files that language has. Measured, min-of-5, three passes: ``` -4294ms (garbage collector) -1552ms v8.deserialize - 756ms crypto update - 728ms runScopeResolution - 620ms scope-resolution/pipeline/reconcile-ownership - 380ms v8-sidecar walk - 323ms internString +before python 411ms typescript 2872ms javascript 507ms = 3834ms +after python 415ms typescript 2805ms javascript 248ms = 3484ms ``` -then a tail of passes at 200–750ms (`emitReceiverBoundCalls`, -`emitCallableValueFlow`, `buildGraphNodeLookup`, `resolveReferenceSites`). +Memoizing each shard's authenticated listing for the run removes the repeat +scans (−350 ms here; roughly −250 ms per additional language elsewhere). The +first pass is unchanged by construction — it is what populates the memo. -No dominant hot function, nothing quadratic. The cost is rehydrating every -file's `ParsedFile` from the durable `.v8` shards into main-thread memory and -re-running every pass over them. +What is left is a floor. The digest is **not** the cost: SHA-256 over all +301 MB takes 134 ms (2.25 GB/s, hardware-accelerated), so swapping it for +CRC-32 would buy ~90 ms and cost an envelope-format bump. The remainder is +`v8.deserialize` (~1240 ms) plus the cross-shard string intern walk +(~1085 ms), and the intern walk is not optional — dropping it regresses +retained heap ~59%, which is #2649's constraint. -**So the win is not caching resolution output** — that stores more of exactly -what is already the memory problem, and #2649 (large-repo OOM) is the standing -constraint. The win is skipping rehydration and re-resolution for files whose -inputs provably did not change, as a streaming/bounded design. +### `emit` is the real target + +7.2s, 49% of the phase and ~21% of the whole edit loop. It is a fan of passes, +each walking every reference site in the repo: + +``` +1828ms emitCallableValueFlow 480ms emitFreeCallFallback +1590ms emitReceiverBoundCalls 426ms emitReferencesViaLookup + 681ms resolveDefGraphId 313ms emitUniqueNamePropertyAccesses + 529ms lookupCore 280ms emitReturnShapeMemberAccesses + 472ms tryEmitEdge 222ms emitPropertyDispatchCalls + 449ms getScope +``` + +No hot inner loop, nothing quadratic, no single pass worth more than 12% of the +phase. Micro-optimizing any of them is not the win. + +The win is not running them. A one-file edit re-emits all 163,094 edges to +write a 3,980-node subgraph. Every pass above runs over all 2121 TypeScript +files because the pipeline rebuilds the full in-memory graph every run and only +the DB _write_ is incremental. Making `emit` incremental means knowing which +files' edges can change when one file's registry contribution changes — which +is a design, not a patch, and #2649 rules out "just cache the emitted edges". --- @@ -232,8 +274,12 @@ and 0.8s of post-FTS event-loop drain between them. FTS is closed as an optimization target except for the overlap above, which is a scheduling change to `run-analyze.ts`'s open/close discipline rather than -anything about FTS. `scopeResolution` is the single largest step, memory-traffic -bound, and still untouched. +anything about FTS. + +`scopeResolution` is now broken down. Its rehydration half has a floor and one +repeat-scan win that is taken. Its other half — `emit`, 7.2s — is the largest +remaining target in the whole edit loop, and the only way at it is incremental +resolution. Every optimization in this document that looked compelling from the armchair died under measurement. Measure first, and check the banner. diff --git a/gitnexus/src/storage/parsedfile-store.ts b/gitnexus/src/storage/parsedfile-store.ts index dce10b03d..68ed2e06e 100644 --- a/gitnexus/src/storage/parsedfile-store.ts +++ b/gitnexus/src/storage/parsedfile-store.ts @@ -169,6 +169,7 @@ export const getParsedFileStoreDir = (storagePath: string): string => /** Remove any prior run's shards so a fresh parse starts clean. Idempotent. */ export const clearParsedFileStore = async (storagePath: string): Promise => { await fs.rm(getParsedFileStoreDir(storagePath), { recursive: true, force: true }); + forgetShardListings(); }; const isV8ShardName = (name: string): boolean => name.endsWith('.v8') && !name.includes('.v8.'); @@ -245,13 +246,52 @@ const listV8Shards = async (dir: string): Promise => { } }; +/** + * Per-run memo of each shard's authenticated path listing, so the SECOND and + * later passes over the store can decide "this shard holds nothing I want" + * without reopening it. + * + * Scope resolution calls `loadParsedFilesForPaths` once per language, and each + * call walks every shard in the store. The skip decision needs the envelope's + * path listing, and that listing is only trustworthy once the payload digest + * has been checked — so today a pass that wants 50 Python files still reads and + * SHA-256s all ~300MB of a TypeScript-dominated store to prove it can skip it. + * Measured on a 2234-file repo: a pass wanting a single file costs 335ms, and + * the three language passes together spend 4426ms in here. The cost scales with + * LANGUAGE COUNT, so a polyglot repo pays it worst. + * + * Keyed by size+mtime as well as name. Shard names are content-addressed + * (parse-chunk hash + worker id), so a name collision across different content + * should be impossible — but that invariant lives in the parse-cache keying, + * not here, and one `stat` per shard is a few ms against the hundreds this + * saves. The memo holds one store directory at a time: a different `dir` (a new + * repo in a long-lived MCP process, or a wiped store) drops the previous set + * rather than accumulating. + */ +let shardListingMemo: { dir: string; byIdentity: Map } | undefined; + +const shardIdentity = (file: string, size: number, mtimeMs: number): string => + `${file}\0${size}\0${mtimeMs}`; + +const memoFor = (dir: string): Map => { + if (shardListingMemo?.dir !== dir) shardListingMemo = { dir, byIdentity: new Map() }; + return shardListingMemo.byIdentity; +}; + +/** Drop the memo when the store it describes is removed. */ +const forgetShardListings = (): void => { + shardListingMemo = undefined; +}; + export const loadParsedFilesForPaths = async ( storagePath: string, wantPaths: ReadonlySet, ): Promise> => { const out = new Map(); if (wantPaths.size === 0) return out; - const shardPaths = await listV8Shards(getParsedFileStoreDir(storagePath)); + const storeDir = getParsedFileStoreDir(storagePath); + const shardPaths = await listV8Shards(storeDir); + const listings = memoFor(storeDir); const pool = new Map(); let droppedSites = 0; let filesWithDroppedSites = 0; @@ -274,11 +314,28 @@ export const loadParsedFilesForPaths = async ( } }; for (const shardFull of shardPaths) { + // Re-stat rather than trusting the name alone, then reuse this run's + // authenticated listing to skip without opening the file. A shard with no + // memoized listing — first pass, or an envelope whose listing did not + // validate — falls through to the full read, so this only ever removes + // work that a later pass had already proved unnecessary. + const st = await fs.stat(shardFull).catch(() => undefined); + const identity = st ? shardIdentity(shardFull, st.size, st.mtimeMs) : undefined; + const memoized = identity === undefined ? undefined : listings.get(identity); + if (memoized !== undefined && !memoized.some((p) => wantPaths.has(p))) { + bytesSinceGc += st?.size ?? 0; + await maybeYieldAndGc(bytesSinceGc >= parsedFileLoadGc.byteBudget); + continue; + } + const loaded = await tryLoadV8Cache(shardFull, pool, wantPaths); if (loaded === undefined) { await maybeYieldAndGc(false); continue; } + if (identity !== undefined && loaded.paths !== undefined) { + listings.set(identity, loaded.paths); + } if (loaded.kind === 'skip') { bytesSinceGc += loaded.bytes; await maybeYieldAndGc(bytesSinceGc >= parsedFileLoadGc.byteBudget); diff --git a/gitnexus/src/storage/v8-sidecar.ts b/gitnexus/src/storage/v8-sidecar.ts index e3b2f92c9..deee9364c 100644 --- a/gitnexus/src/storage/v8-sidecar.ts +++ b/gitnexus/src/storage/v8-sidecar.ts @@ -185,8 +185,22 @@ const decodePrefix = (buf: Buffer): EnvelopeMeta | undefined => { const runtimeCompatible = (meta: EnvelopeMeta): boolean => meta.recordedNodeMajor === nodeMajor() && meta.recordedV8 === process.versions.v8; -export type V8CacheHit = { kind: 'hit'; value: unknown; bytes: number }; -export type V8CacheSkip = { kind: 'skip'; bytes: number }; +/** + * `paths` is the envelope's digest-authenticated path listing, present only + * when it parsed and its entry count matched the recorded `pathCount` — i.e. + * exactly when the skip decision below is willing to trust it. A caller that + * loads the same immutable shard more than once per run can memoize it and + * make its own skip decision without reopening the file (see + * `parsedfile-store.ts`). Absent means "this envelope has no listing worth + * trusting", which is fail-closed: load, never skip. + */ +export type V8CacheHit = { + kind: 'hit'; + value: unknown; + bytes: number; + paths?: readonly string[]; +}; +export type V8CacheSkip = { kind: 'skip'; bytes: number; paths: readonly string[] }; export type V8CacheLoad = V8CacheHit | V8CacheSkip; export type V8CacheInspection = { paths: readonly string[] }; @@ -294,20 +308,29 @@ export const tryLoadV8Cache = async ( const payload = await readVerifiedPayload(fh, meta, pathRaw); if (!payload) return undefined; - if (wantPaths && wantPaths.size > 0 && meta.pathBytes > 0) { + // Parsed once and reused for both the skip decision and the returned + // listing, so a caller that memoizes it is trusting exactly the bytes this + // function was already willing to skip on. Anything that fails these checks + // stays `undefined` and falls through to the load. + let listedPaths: readonly string[] | undefined; + if (meta.pathBytes > 0) { const listed = parseCachePathListing(pathRaw); - if ( - listed !== null && - listed.length === meta.pathCount && - listed.length > 0 && - !listed.some((p) => wantPaths.has(p)) - ) { - return { kind: 'skip', bytes: st.size }; + if (listed !== null && listed.length === meta.pathCount && listed.length > 0) { + listedPaths = listed; } } + + if ( + wantPaths && + wantPaths.size > 0 && + listedPaths && + !listedPaths.some((p) => wantPaths.has(p)) + ) { + return { kind: 'skip', bytes: st.size, paths: listedPaths }; + } const value = v8.deserialize(payload); if (internPool) internGraphStrings(value, internPool); - return { kind: 'hit', value, bytes: st.size }; + return { kind: 'hit', value, bytes: st.size, paths: listedPaths }; } catch (err) { if (!isEnoent(err)) { logger.debug({ err, filePath }, 'v8 cache: load failed; treating as miss'); diff --git a/gitnexus/test/unit/parsedfile-store.test.ts b/gitnexus/test/unit/parsedfile-store.test.ts index 85a96e3da..c788d2221 100644 --- a/gitnexus/test/unit/parsedfile-store.test.ts +++ b/gitnexus/test/unit/parsedfile-store.test.ts @@ -86,6 +86,41 @@ describe('parsedfile-store', () => { } }); + it('repeated loads with different wantPaths each see their own files (shard-listing memo)', async () => { + // Scope resolution calls loadParsedFilesForPaths once per LANGUAGE over the + // same store, so the second and later passes reuse the shard path listings + // the first pass authenticated instead of re-reading every shard. The + // failure mode that memo introduces is a FALSE SKIP: pass 2 concludes a + // shard holds nothing it wants, and those files silently never reach the + // graph — an exit-0 wrong answer, not a crash. Each pass below wants files + // the previous pass did not, so a listing carried over from the wrong shard + // shows up as a missing file here. + const dir = await mkdtemp(path.join(tmpdir(), 'pfstore-')); + try { + await persistParsedFileChunk(dir, 'chunk-0', [makeParsedFile('a.c'), makeParsedFile('b.c')]); + await persistParsedFileChunk(dir, 'chunk-1', [makeParsedFile('c.c')]); + await persistParsedFileChunk(dir, 'chunk-2', [makeParsedFile('d.c')]); + + const first = await loadParsedFilesForPaths(dir, new Set(['a.c'])); + const second = await loadParsedFilesForPaths(dir, new Set(['c.c', 'd.c'])); + const third = await loadParsedFilesForPaths(dir, new Set(['b.c'])); + const fourth = await loadParsedFilesForPaths(dir, new Set(['a.c', 'b.c', 'c.c', 'd.c'])); + + expect([...first.keys()].sort()).toEqual(['a.c']); + expect([...second.keys()].sort()).toEqual(['c.c', 'd.c']); + expect([...third.keys()].sort()).toEqual(['b.c']); + expect([...fourth.keys()].sort()).toEqual(['a.c', 'b.c', 'c.c', 'd.c']); + + // A shard written AFTER the memo was populated is still found: the memo + // holds listings, not the shard roster, and the roster is re-read per call. + await persistParsedFileChunk(dir, 'chunk-3', [makeParsedFile('e.c')]); + const fifth = await loadParsedFilesForPaths(dir, new Set(['e.c', 'a.c'])); + expect([...fifth.keys()].sort()).toEqual(['a.c', 'e.c']); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + it('writes no shard for an empty chunk', async () => { const dir = await mkdtemp(path.join(tmpdir(), 'pfstore-')); try {