mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-13 03:58:33 +00:00
perf(scope-resolution): stop re-scanning the ParsedFile store once per language
Scope resolution calls `loadParsedFilesForPaths` once per language, and every call walks every shard in the store. The skip decision needs the envelope's path listing, and that listing is only trustworthy after the payload digest has been checked -- so a pass that wants 50 Python files still opens and SHA-256s all 413 shards / 301MB of a TypeScript-dominated store to prove it can skip them. A pass wanting a SINGLE file costs 335ms. The cost scales with language count, not with the files that language has, so a polyglot repo pays it worst. `tryLoadV8Cache` now returns the listing it already parsed for that skip decision, and the store memoizes it per run. Later passes skip on the memoized listing without reopening the file. Measured on a 2234-file, 3-language repo, min-of-5: before python 411ms typescript 2872ms javascript 507ms = 3834ms after python 415ms typescript 2805ms javascript 248ms = 3484ms -350ms here, roughly -250ms per additional language elsewhere. The first pass is unchanged by construction -- it is what populates the memo. End-to-end the graph is byte-identical: 51,288 nodes / 163,094 edges / 2106 clusters / 759 flows on a true incremental run. Keyed on size+mtime as well as name. Shard names are content-addressed, so a name collision across different content should be impossible, but that invariant lives in the parse-cache keying rather than here and one stat per shard is a few ms against the hundreds this saves. The memo holds one store directory at a time, so a new repo in a long-lived MCP process drops the previous set instead of accumulating. The failure mode a listing memo introduces is a FALSE SKIP: a pass concludes a shard holds nothing it wants and those files silently never reach the graph -- an exit-0 wrong answer, not a crash. The new test walks four passes with disjoint wants over one store, plus a shard written after the memo is warm; it fails when the skip is forced. Also records the full scopeResolution breakdown in bench/. The headline is that `emit` is 7161ms of the 14.7s phase and ~21% of the edit loop, spread across a fan of passes with no hot inner loop -- so the win there is not running them for unchanged files, which is a design rather than a patch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
8941bbe22c
commit
489e4b6024
4 changed files with 193 additions and 32 deletions
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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<void> => {
|
||||
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<string[]> => {
|
|||
}
|
||||
};
|
||||
|
||||
/**
|
||||
* 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<string, readonly string[]> } | undefined;
|
||||
|
||||
const shardIdentity = (file: string, size: number, mtimeMs: number): string =>
|
||||
`${file}\0${size}\0${mtimeMs}`;
|
||||
|
||||
const memoFor = (dir: string): Map<string, readonly string[]> => {
|
||||
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<string>,
|
||||
): Promise<Map<string, ParsedFile>> => {
|
||||
const out = new Map<string, ParsedFile>();
|
||||
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<string, string>();
|
||||
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);
|
||||
|
|
|
|||
|
|
@ -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');
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue