mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-09-19 00:03:33 +00:00
* perf(parse): batch cache packs into one dispatch round `WorkerPool.dispatch` is a barrier, so dispatching one parse-cache pack at a time leaves most slots idle for every round-trip. Packs are keyed by `(language, hash(path) % 128)`, so the byte budget rarely binds: this repo produces 1285 packs where the budget alone needs 16, and 549 of those hold a single file. In a real analyze, 76 of 221 dispatched chunks carried one file and cost 15.3s — 20% of the parse phase for 3.4% of the files. Chunks now accumulate into a round bounded by `GITNEXUS_PARSE_ROUND_BYTES` of cache-missing source (default: the chunk byte budget) and go out through a new `WorkerPool.dispatchGroups`. Jobs are still cut at pack boundaries, so each job carries exactly one `chunkHash` and every result stays attributable to the pack whose cache key owns it. Cache hits ride along as round entries, and rounds drain in `chunkIdx` order, so deferred aggregation stays deterministic. Cold `analyze --index-only` on this repo (2234 parseable files, 16 workers): 110.3s -> 70.5s total, parse phase 74.0s -> 40.5s, 221 dispatches -> 15. Graph output is unchanged: 51,286 nodes / 163,092 edges / 2106 clusters / 759 flows in both arms. Peak main-thread RSS 3372MB -> 3487MB (+3.4%). `dispatchGroups` also claims the pool synchronously and rejects a concurrent call. Two overlapping dispatches hand the same slots out twice and both stall; the first version of this change did exactly that, and the only symptom was every worker idle-timing out ~10s later with no indication of the cause. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(parse): bound what an open round holds, not just what it dispatches Follow-up to the review of #3196. Three reviewers independently found the same defect: `roundMissBytes` was the only in-loop close condition, but cache HITS were queued into the same round without contributing to it. A warm re-analyze misses nothing, so no round ever closed and every chunk's source plus its cached worker output stayed resident until the tail drain — the #2649 heap failure shape on a large repo. - Hit entries now carry a file COUNT, not the file array, so a replayed chunk never pins its source text. `applyChunkResults` only ever read `.length`. - Track `roundBufferedBytes` across hits and misses and close on either cap. Verified on a warm run: with the cap, draining starts as soon as 2MB is buffered; without it all 221 merges land in the final 10% of the phase. - Warm progress no longer freezes at the phase floor. `filesParsedSoFar` only advances at drain, so a new `queuedFilesSoFar` feeds the progress events while `filesParsedSoFar` stays the merge-accurate throughput number. - A throw from `drainRound` used to unwind straight to `terminate()` while the next round's workers were still busy — the #2432 mid-N-API abort hazard. Settle the in-flight round first, then propagate. - `dispatchGroups` returns one array per group; assert that length instead of `?? []`, which turned a contract break into a silently empty chunk. - Collapse `PendingWorkerChunk` into the `miss` RoundEntry it duplicated. - Repair two stale doc comments: `dispatch`'s JSDoc had been orphaned onto `dispatchGroups`, and `dispatchChunkParse` still described chunk overlap that now lives in parse-impl's round machinery. - New test: a round mixing a cache hit and a cache miss. `drainRound` walks entries in chunkIdx order but pulls results on a separate cursor, and no existing test put both kinds in one round with content assertions. Cold analyze unchanged: 71.3s, 15 rounds, 51,286 nodes / 163,092 edges. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
225 lines
9 KiB
TypeScript
225 lines
9 KiB
TypeScript
/**
|
||
* Dispatch rounds — batching cache packs into one pool round.
|
||
*
|
||
* `WorkerPool.dispatch` is a barrier, so one round-trip per parse-cache pack
|
||
* strands the pool whenever a pack is smaller than it. Packs are keyed by
|
||
* `(language, hash(path) % 128)`, so on a real repo most of them are: this
|
||
* repository produces 1285 packs where the byte budget alone needs 16, and 549
|
||
* of those hold a single file. Chunks now accumulate into a round bounded by
|
||
* `GITNEXUS_PARSE_ROUND_BYTES` and go out in one `dispatchGroups` call.
|
||
*
|
||
* Batching must be invisible to the graph. These tests pin the two ways it
|
||
* could stop being invisible:
|
||
* 1. Ordering — deferred aggregation runs in `chunkIdx` order, so the graph
|
||
* must not depend on how chunks were grouped into rounds.
|
||
* 2. Attribution — a round returns one result array per pack, so a pack's
|
||
* parse-cache entry must hold ITS OWN worker output. Mis-attribution would
|
||
* survive a cold run and only surface as a corrupted warm replay, which is
|
||
* what the second test exercises.
|
||
*/
|
||
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
|
||
import fs from 'node:fs';
|
||
import os from 'node:os';
|
||
import path from 'node:path';
|
||
|
||
import { runChunkedParseAndResolve } from '../../src/core/ingestion/pipeline-phases/parse-impl.js';
|
||
import { createKnowledgeGraph } from '../../src/core/graph/graph.js';
|
||
import { PARSE_CACHE_VERSION, packParseCacheChunks } from '../../src/storage/parse-cache.js';
|
||
import type { ParseWorkerResult } from '../../src/core/ingestion/workers/parse-worker.js';
|
||
|
||
const ORIGINAL_ROUND_BYTES = process.env.GITNEXUS_PARSE_ROUND_BYTES;
|
||
|
||
/**
|
||
* Enough files, across enough languages, that `(language, bucket)` packing
|
||
* yields many more packs than the byte budget would — the shape that makes
|
||
* per-pack dispatch a barrier problem in the first place.
|
||
*/
|
||
const FIXTURE: ReadonlyArray<[string, string]> = [
|
||
...Array.from({ length: 12 }, (_, i): [string, string] => [
|
||
`src/mod${i}.ts`,
|
||
`export function ts${i}() { return ${i}; }\n`,
|
||
]),
|
||
...Array.from({ length: 8 }, (_, i): [string, string] => [
|
||
`src/mod${i}.py`,
|
||
`def py${i}():\n return ${i}\n`,
|
||
]),
|
||
...Array.from({ length: 6 }, (_, i): [string, string] => [
|
||
`src/Mod${i}.java`,
|
||
`public class Mod${i} { public int go() { return ${i}; } }\n`,
|
||
]),
|
||
...Array.from({ length: 6 }, (_, i): [string, string] => [
|
||
`src/mod${i}.go`,
|
||
`package main\n\nfunc Go${i}() int { return ${i} }\n`,
|
||
]),
|
||
];
|
||
|
||
describe('parse-impl dispatch rounds', () => {
|
||
let repoPath = '';
|
||
let storageDir = '';
|
||
|
||
beforeEach(() => {
|
||
repoPath = fs.mkdtempSync(path.join(os.tmpdir(), 'parse-impl-dispatch-rounds-'));
|
||
storageDir = fs.mkdtempSync(path.join(os.tmpdir(), 'parse-impl-rounds-storage-'));
|
||
for (const [rel, content] of FIXTURE) {
|
||
const full = path.join(repoPath, rel);
|
||
fs.mkdirSync(path.dirname(full), { recursive: true });
|
||
fs.writeFileSync(full, content);
|
||
}
|
||
});
|
||
|
||
afterEach(() => {
|
||
for (const dir of [repoPath, storageDir]) {
|
||
if (dir && fs.existsSync(dir)) fs.rmSync(dir, { recursive: true, force: true });
|
||
}
|
||
if (ORIGINAL_ROUND_BYTES === undefined) delete process.env.GITNEXUS_PARSE_ROUND_BYTES;
|
||
else process.env.GITNEXUS_PARSE_ROUND_BYTES = ORIGINAL_ROUND_BYTES;
|
||
});
|
||
|
||
const files = () =>
|
||
FIXTURE.map(([rel]) => ({ path: rel, size: fs.statSync(path.join(repoPath, rel)).size }));
|
||
|
||
/**
|
||
* Order-independent fingerprint of the graph. Counts alone would let a
|
||
* mis-attributed chunk (right totals, wrong contents) pass.
|
||
*/
|
||
const fingerprint = (graph: ReturnType<typeof createKnowledgeGraph>): string =>
|
||
Array.from(graph.nodes.values())
|
||
.map((node) => {
|
||
const props = node.properties as { name?: string; filePath?: string } | undefined;
|
||
return `${node.label}|${props?.name ?? ''}|${props?.filePath ?? ''}`;
|
||
})
|
||
.sort()
|
||
.join('\n');
|
||
|
||
const run = async (parseCache?: {
|
||
version: string;
|
||
entries: Map<string, ParseWorkerResult[]>;
|
||
usedKeys: Set<string>;
|
||
storagePath: string;
|
||
onDiskKeys: Set<string>;
|
||
}) => {
|
||
const scan = files();
|
||
const rels = scan.map((f) => f.path);
|
||
const graph = createKnowledgeGraph();
|
||
await runChunkedParseAndResolve(
|
||
graph,
|
||
scan,
|
||
rels,
|
||
scan.length,
|
||
repoPath,
|
||
Date.now(),
|
||
() => {},
|
||
parseCache ? { parseCache } : {},
|
||
);
|
||
return graph;
|
||
};
|
||
|
||
it('the fixture really does split into more packs than the byte budget needs', () => {
|
||
// Guards the premise: if packing ever stopped over-splitting, the tests
|
||
// below would still pass while measuring nothing.
|
||
const packs = packParseCacheChunks(
|
||
files().map((f) => ({
|
||
path: f.path,
|
||
size: f.size,
|
||
language: f.path.slice(f.path.lastIndexOf('.') + 1),
|
||
})),
|
||
2 * 1024 * 1024,
|
||
);
|
||
const totalBytes = files().reduce((sum, f) => sum + f.size, 0);
|
||
expect(totalBytes).toBeLessThan(2 * 1024 * 1024);
|
||
expect(packs.length).toBeGreaterThan(1);
|
||
});
|
||
|
||
it('produces the same graph whether chunks are batched into rounds or dispatched one by one', async () => {
|
||
// 1 byte closes a round after every cache-missing chunk — the pre-round
|
||
// behaviour, and the control arm for the batched default.
|
||
process.env.GITNEXUS_PARSE_ROUND_BYTES = '1';
|
||
const perChunk = await run();
|
||
|
||
delete process.env.GITNEXUS_PARSE_ROUND_BYTES;
|
||
const batched = await run();
|
||
|
||
expect(batched.nodeCount).toBe(perChunk.nodeCount);
|
||
expect(batched.relationshipCount).toBe(perChunk.relationshipCount);
|
||
expect(fingerprint(batched)).toBe(fingerprint(perChunk));
|
||
// Pin real symbols so an empty-graph regression cannot satisfy the above.
|
||
const names = fingerprint(batched);
|
||
expect(names).toContain('ts0');
|
||
expect(names).toContain('py0');
|
||
expect(names).toContain('Mod0');
|
||
expect(names).toContain('Go0');
|
||
});
|
||
|
||
it('keeps hit and miss chunks attributed to their own files inside one round', async () => {
|
||
// The realistic incremental shape: some packs warm, some cold, batched into
|
||
// the SAME round. `drainRound` walks the round's entries in `chunkIdx`
|
||
// order but pulls worker output with a separate `missIdx` cursor, so a
|
||
// hit sitting between two misses is exactly where that cursor can slip.
|
||
// Cold-then-warm alone never exercises it -- every entry is the same kind.
|
||
const cache = {
|
||
version: PARSE_CACHE_VERSION,
|
||
entries: new Map<string, ParseWorkerResult[]>(),
|
||
usedKeys: new Set<string>(),
|
||
storagePath: storageDir,
|
||
onDiskKeys: new Set<string>(),
|
||
};
|
||
|
||
const cold = await run(cache);
|
||
const cachedPacks = cache.onDiskKeys.size + cache.entries.size;
|
||
expect(cachedPacks).toBeGreaterThan(1);
|
||
|
||
// Edit ONE file. Its pack now misses; every other pack still hits, so the
|
||
// next run's rounds carry both kinds together.
|
||
fs.writeFileSync(
|
||
path.join(repoPath, 'src/mod0.ts'),
|
||
'export function ts0() { return 999; }\nexport function ts0Extra() { return 1; }\n',
|
||
);
|
||
|
||
const mixed = await run(cache);
|
||
|
||
// The edited file's NEW symbol must be present, proving the miss chunk's
|
||
// fresh worker output landed under its own file...
|
||
const mixedPrint = fingerprint(mixed);
|
||
expect(mixedPrint).toContain('ts0Extra');
|
||
// ...and every untouched file's symbols must still be present and attached
|
||
// to their own paths, proving no hit chunk was overwritten by, or swapped
|
||
// with, a neighbouring miss chunk's results.
|
||
const coldPrint = fingerprint(cold);
|
||
const untouched = coldPrint
|
||
.split('\n')
|
||
.filter((entry) => !entry.endsWith('|src/mod0.ts'))
|
||
.sort();
|
||
const mixedUntouched = mixedPrint
|
||
.split('\n')
|
||
.filter((entry) => !entry.endsWith('|src/mod0.ts'))
|
||
.sort();
|
||
expect(mixedUntouched).toEqual(untouched);
|
||
});
|
||
|
||
it('stores each pack’s own worker output, so a warm replay reproduces the cold graph', async () => {
|
||
const cache = {
|
||
version: PARSE_CACHE_VERSION,
|
||
entries: new Map<string, ParseWorkerResult[]>(),
|
||
usedKeys: new Set<string>(),
|
||
storagePath: storageDir,
|
||
onDiskKeys: new Set<string>(),
|
||
};
|
||
|
||
// Cold: every pack misses, and the round writes each pack's results under
|
||
// that pack's own hash.
|
||
const cold = await run(cache);
|
||
// With a storagePath the chunk bodies land on disk and the hash is tracked
|
||
// in `onDiskKeys`; without one they stay in `entries`. Count both so the
|
||
// assertion pins "more than one pack was cached", not the storage route.
|
||
expect(cache.onDiskKeys.size + cache.entries.size).toBeGreaterThan(1);
|
||
expect(cache.usedKeys.size).toBe(cache.onDiskKeys.size + cache.entries.size);
|
||
|
||
// Warm: every pack replays from its cache entry with no worker dispatch.
|
||
// If a round had attributed pack A's results to pack B's key, the replayed
|
||
// graph would differ here even though the cold run looked correct.
|
||
const warm = await run(cache);
|
||
expect(fingerprint(warm)).toBe(fingerprint(cold));
|
||
expect(warm.nodeCount).toBe(cold.nodeCount);
|
||
expect(warm.relationshipCount).toBe(cold.relationshipCount);
|
||
});
|
||
});
|