mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-11-06 07:47:46 +00:00
fix(scope): reduce memory for terminal import misses (#3741)
* fix(scope): reduce memory for terminal import misses * fix(review): cover external imports and cache isolation --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
This commit is contained in:
parent
374b10de86
commit
b2c72997ea
5 changed files with 212 additions and 21 deletions
|
|
@ -91,6 +91,8 @@ export interface FinalizeInput {
|
|||
readonly files: readonly FinalizeFile[];
|
||||
/** Opaque workspace context forwarded to provider hooks. */
|
||||
readonly workspaceIndex: WorkspaceIndex;
|
||||
/** Optional phase-boundary diagnostics; receives no source or symbol data. */
|
||||
readonly onProgress?: (phase: string) => void;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -258,6 +260,7 @@ export interface FinalizeOutput {
|
|||
// ─── Entry point ───────────────────────────────────────────────────────────
|
||||
|
||||
export function finalize(input: FinalizeInput, hooks: FinalizeHooks): FinalizeOutput {
|
||||
input.onProgress?.('imports-start');
|
||||
const byFilePath = new Map<string, FinalizeFile>();
|
||||
for (const f of input.files) byFilePath.set(f.filePath, f);
|
||||
|
||||
|
|
@ -277,6 +280,7 @@ export function finalize(input: FinalizeInput, hooks: FinalizeHooks): FinalizeOu
|
|||
}
|
||||
edgeIndex.set(file.filePath, drafts);
|
||||
}
|
||||
input.onProgress?.('imports-end');
|
||||
|
||||
// ── Phase 1: build file-level import graph (only resolvable edges form
|
||||
// graph edges; unresolvable ones are terminal and contribute no
|
||||
|
|
@ -297,6 +301,7 @@ export function finalize(input: FinalizeInput, hooks: FinalizeHooks): FinalizeOu
|
|||
|
||||
// ── Phase 2: Tarjan SCC → reverse-topological list of SCCs.
|
||||
const sccs = tarjanSccs(graph);
|
||||
input.onProgress?.('scc-end');
|
||||
|
||||
// ── Phase 2.5: prepare the per-file re-export closure (iterative,
|
||||
// SCC-condensed). Eliminates the recursive crawl that the per-edge
|
||||
|
|
@ -304,11 +309,22 @@ export function finalize(input: FinalizeInput, hooks: FinalizeHooks): FinalizeOu
|
|||
// See `buildReexportClosures` for the algorithm.
|
||||
// A local import is a dependency of its file, not a re-export of that file.
|
||||
const moduleEdgeIndex = new Map<string, ImportEdgeDraft[]>();
|
||||
const reexportingFiles = new Set<string>();
|
||||
for (const file of input.files) {
|
||||
moduleEdgeIndex.set(
|
||||
file.filePath,
|
||||
(edgeIndex.get(file.filePath) ?? []).filter((d) => d.fromScope === file.moduleScope),
|
||||
const drafts = (edgeIndex.get(file.filePath) ?? []).filter(
|
||||
(d) => d.fromScope === file.moduleScope,
|
||||
);
|
||||
moduleEdgeIndex.set(file.filePath, drafts);
|
||||
if (
|
||||
drafts.some(
|
||||
(draft) =>
|
||||
draft.targetFile !== null &&
|
||||
byFilePath.has(draft.targetFile) &&
|
||||
contributesReexportEdge(draft),
|
||||
)
|
||||
) {
|
||||
reexportingFiles.add(file.filePath);
|
||||
}
|
||||
}
|
||||
const ambiguityByFile = collectAmbiguityByFile(
|
||||
input.files,
|
||||
|
|
@ -322,20 +338,31 @@ export function finalize(input: FinalizeInput, hooks: FinalizeHooks): FinalizeOu
|
|||
ambiguousByFile.set(filePath, new Set(byName.keys()));
|
||||
}
|
||||
const topLevelOnly = hooks.namedImportsBindTopLevelOnly === true;
|
||||
input.onProgress?.('ambiguity-end');
|
||||
// Direct imports and unfiltered wildcards never read these closures. Build
|
||||
// them once, on demand, so those workspaces do not allocate transitive tables
|
||||
// (and their provenance paths) that are discarded without a single lookup.
|
||||
let reexportClosures: ReadonlyMap<string, FileReexportClosure> | undefined;
|
||||
const getReexportClosures = () =>
|
||||
(reexportClosures ??= buildReexportClosures(
|
||||
input.files,
|
||||
byFilePath,
|
||||
moduleEdgeIndex,
|
||||
ambiguousByFile,
|
||||
topLevelOnly,
|
||||
hooks.filterWildcardNames,
|
||||
input.workspaceIndex,
|
||||
));
|
||||
const getReexportClosures = (targetFile: string) => {
|
||||
// #3695: a terminal miss must not allocate unrelated wildcard closures.
|
||||
if (!reexportingFiles.has(targetFile)) {
|
||||
return EMPTY_REEXPORT_CLOSURES;
|
||||
}
|
||||
if (reexportClosures === undefined) {
|
||||
input.onProgress?.('reexports-start');
|
||||
reexportClosures = buildReexportClosures(
|
||||
input.files,
|
||||
byFilePath,
|
||||
moduleEdgeIndex,
|
||||
ambiguousByFile,
|
||||
topLevelOnly,
|
||||
hooks.filterWildcardNames,
|
||||
input.workspaceIndex,
|
||||
);
|
||||
input.onProgress?.('reexports-end');
|
||||
}
|
||||
return reexportClosures;
|
||||
};
|
||||
|
||||
// ── Phase 3: process SCCs in reverse-topological order (leaves first).
|
||||
// Within each SCC, run a bounded fixpoint that resolves intra-SCC edges.
|
||||
|
|
@ -397,6 +424,7 @@ export function finalize(input: FinalizeInput, hooks: FinalizeHooks): FinalizeOu
|
|||
}
|
||||
}
|
||||
|
||||
input.onProgress?.('link-end');
|
||||
// ── Phase 4: collect finalized `ImportEdge[]` per binding scope, preserving
|
||||
// input order within each file, and wildcard-expand where applicable.
|
||||
for (const file of input.files) {
|
||||
|
|
@ -447,9 +475,11 @@ export function finalize(input: FinalizeInput, hooks: FinalizeHooks): FinalizeOu
|
|||
linkedByScope.set(scopeId, Object.freeze(edges));
|
||||
}
|
||||
|
||||
input.onProgress?.('wildcards-end');
|
||||
// ── Phase 5: materialize bindings (local + scoped imports + wildcards),
|
||||
// delegating precedence to `provider.mergeBindings`.
|
||||
const bindingsByScope = materializeBindings(input.files, linkedByScope, hooks);
|
||||
input.onProgress?.('bindings-end');
|
||||
|
||||
// ── Stats.
|
||||
const sccCount = sccs.length;
|
||||
|
|
@ -697,7 +727,7 @@ function extractExportedName(parsed: ParsedImport): string {
|
|||
function tryFinalize(
|
||||
draft: ImportEdgeDraft,
|
||||
byFilePath: Map<string, FinalizeFile>,
|
||||
getReexportClosures: () => ReadonlyMap<string, FileReexportClosure>,
|
||||
getReexportClosures: (targetFile: string) => ReadonlyMap<string, FileReexportClosure>,
|
||||
topLevelOnly: boolean,
|
||||
): ImportEdge | null {
|
||||
const targetFile = draft.targetFile;
|
||||
|
|
@ -759,7 +789,7 @@ function tryFinalize(
|
|||
// already encodes every name reachable through that file's named and
|
||||
// wildcard re-exports — including transitively through cyclic SCCs —
|
||||
// so the lookup is O(1) and never recurses.
|
||||
const followed = lookupReexportedName(getReexportClosures(), targetFile, importedName);
|
||||
const followed = lookupReexportedName(getReexportClosures(targetFile), targetFile, importedName);
|
||||
if (followed === null) {
|
||||
// Target resolvable but the name isn't exported — keep trying in case a
|
||||
// re-export inside the target's SCC surfaces it in a later iteration.
|
||||
|
|
@ -793,6 +823,7 @@ function tryFinalize(
|
|||
*/
|
||||
type ReexportClosureEntry = { readonly def: SymbolDefinition; readonly via: readonly string[] };
|
||||
type FileReexportClosure = ReadonlyMap<string, ReexportClosureEntry>;
|
||||
const EMPTY_REEXPORT_CLOSURES: ReadonlyMap<string, FileReexportClosure> = new Map();
|
||||
|
||||
/**
|
||||
* Build per-file re-export closures.
|
||||
|
|
@ -1610,7 +1641,7 @@ function expandWildcard(
|
|||
byFilePath: Map<string, FinalizeFile>,
|
||||
hooks: FinalizeHooks,
|
||||
workspace: WorkspaceIndex,
|
||||
getReexportClosures: () => ReadonlyMap<string, FileReexportClosure>,
|
||||
getReexportClosures: (targetFile: string) => ReadonlyMap<string, FileReexportClosure>,
|
||||
): readonly ImportEdge[] {
|
||||
if (edge.targetModuleScope === undefined || edge.targetFile === null) {
|
||||
return [edge]; // unresolvable wildcard survives as a single unlinked edge
|
||||
|
|
@ -1621,7 +1652,7 @@ function expandWildcard(
|
|||
const targetClosure =
|
||||
hooks.filterWildcardNames === undefined
|
||||
? undefined
|
||||
: getReexportClosures().get(edge.targetFile);
|
||||
: getReexportClosures(edge.targetFile).get(edge.targetFile);
|
||||
const names =
|
||||
hooks.filterWildcardNames?.(
|
||||
edge.targetModuleScope,
|
||||
|
|
|
|||
|
|
@ -51,3 +51,28 @@ node --import tsx bench/scope-resolution/measure.mts --language java --sizes 10,
|
|||
To establish baselines after reviewing a fixture, semantic, or allocation change,
|
||||
run the default all-language workload with `--record` instead of `--check` and inspect
|
||||
`baselines.json`. CI never records or accepts a new baseline automatically.
|
||||
|
||||
## Finalization import-miss probe
|
||||
|
||||
To reproduce the allocation trigger isolated while investigating #3695:
|
||||
|
||||
```sh
|
||||
node --expose-gc --import tsx bench/scope-resolution/finalize-miss.mts 500
|
||||
```
|
||||
|
||||
Run in fresh processes on each revision after building. This constructs 500
|
||||
files with chained wildcard imports, a terminal module with an unresolved external
|
||||
wildcard import, and a consumer importing a missing name from that terminal module.
|
||||
The missing name cannot be supplied by re-exports, so it should not trigger
|
||||
construction of the unrelated chain's
|
||||
transitive export tables. The JSON reports finalization time, endpoint heap
|
||||
growth, process peak RSS, and a fingerprint of all finalized imports, bindings,
|
||||
SCCs and statistics. It is a synthetic finalizer workload, not a full Java
|
||||
indexing run or a reproduction of the reporter's private repository.
|
||||
|
||||
For a real indexing run, the existing `GITNEXUS_DEBUG_HEAP=1` and
|
||||
`GITNEXUS_HEAP_PROBE_FILE` settings now include `sr-finalize-*` markers for input
|
||||
preparation, import-target resolution, SCCs, ambiguity checks, transitive exports
|
||||
(only if requested), linking, wildcard expansion, bindings and workspace indexes.
|
||||
`sr-extract-start` precedes `sr-post-nodeLookup`; a last `sr-post-nodeLookup`
|
||||
marker therefore points into finalization after extraction.
|
||||
|
|
|
|||
71
gitnexus/bench/scope-resolution/finalize-miss.mts
Normal file
71
gitnexus/bench/scope-resolution/finalize-miss.mts
Normal file
|
|
@ -0,0 +1,71 @@
|
|||
/** Isolate a terminal import miss beside an unrelated wildcard chain (#3695). */
|
||||
import assert from 'node:assert/strict';
|
||||
import { createHash } from 'node:crypto';
|
||||
import { performance } from 'node:perf_hooks';
|
||||
import { finalize, type FinalizeFile, type FinalizeHooks } from 'gitnexus-shared';
|
||||
|
||||
const count = Number(process.argv[2] ?? 500);
|
||||
assert.ok(Number.isSafeInteger(count) && count >= 2, 'Expected a file count >= 2');
|
||||
const files: FinalizeFile[] = Array.from({ length: count }, (_, i) => ({
|
||||
filePath: `chain/${i}.java`,
|
||||
moduleScope: `scope:${i}`,
|
||||
localDefs: Array.from({ length: 4 }, (_, j) => ({
|
||||
nodeId: `def:${i}_${j}`,
|
||||
filePath: `chain/${i}.java`,
|
||||
type: 'Class',
|
||||
qualifiedName: `p.C${i}_${j}`,
|
||||
})),
|
||||
parsedImports: i + 1 < count ? [{ kind: 'wildcard', targetRaw: `chain/${i + 1}.java` }] : [],
|
||||
}));
|
||||
files.push({
|
||||
filePath: 'leaf.java',
|
||||
moduleScope: 'scope:leaf',
|
||||
localDefs: [{ nodeId: 'def:Leaf', filePath: 'leaf.java', type: 'Class', qualifiedName: 'Leaf' }],
|
||||
parsedImports: [{ kind: 'wildcard', targetRaw: 'java/util' }],
|
||||
});
|
||||
files.push({
|
||||
filePath: 'consumer.java',
|
||||
moduleScope: 'scope:consumer',
|
||||
localDefs: [],
|
||||
parsedImports: [
|
||||
{ kind: 'named', localName: 'absent', importedName: 'absent', targetRaw: 'leaf.java' },
|
||||
],
|
||||
});
|
||||
const paths = new Set(files.map((file) => file.filePath));
|
||||
const hooks: FinalizeHooks = {
|
||||
resolveImportTarget: (target) => (paths.has(target) ? target : null),
|
||||
expandsWildcardTo: () => [],
|
||||
mergeBindings: (existing, incoming) => [...existing, ...incoming],
|
||||
};
|
||||
global.gc?.();
|
||||
const heapBefore = process.memoryUsage().heapUsed;
|
||||
const start = performance.now();
|
||||
const result = finalize({ files, workspaceIndex: undefined }, hooks);
|
||||
const elapsedMs = performance.now() - start;
|
||||
const heapAfter = process.memoryUsage().heapUsed;
|
||||
const peakRss = process.resourceUsage().maxRSS * 1024;
|
||||
const edges = result.imports.get('scope:consumer');
|
||||
assert.equal(edges?.length, 1);
|
||||
assert.equal(edges[0].linkStatus, 'unresolved');
|
||||
assert.equal(result.stats.totalFiles, count + 2);
|
||||
assert.equal(result.stats.totalEdges, count + 1);
|
||||
assert.equal(result.bindings.size, count + 2);
|
||||
const serialized = JSON.stringify({
|
||||
imports: [...result.imports],
|
||||
bindings: [...result.bindings].map(([scope, bindings]) => [scope, [...bindings]]),
|
||||
sccs: result.sccs,
|
||||
stats: result.stats,
|
||||
});
|
||||
console.log(
|
||||
JSON.stringify({
|
||||
count,
|
||||
elapsedMs,
|
||||
heapBefore,
|
||||
heapAfter,
|
||||
heapDelta: heapAfter - heapBefore,
|
||||
peakRss,
|
||||
files: result.stats.totalFiles,
|
||||
edges: result.stats.totalEdges,
|
||||
fingerprint: createHash('sha256').update(serialized).digest('hex'),
|
||||
}),
|
||||
);
|
||||
|
|
@ -52,6 +52,7 @@ import {
|
|||
} from 'gitnexus-shared';
|
||||
import type { ScopeResolutionIndexes } from './model/scope-resolution-indexes.js';
|
||||
import { parseTruthyEnv } from './utils/env.js';
|
||||
import { isDebugHeapEnabled, logHeapProbe } from './utils/heap-probe.js';
|
||||
import { TransitionalScopeTree } from '../../storage/scope-index-store.js';
|
||||
|
||||
// ─── Public entry point ─────────────────────────────────────────────────────
|
||||
|
|
@ -82,9 +83,9 @@ export interface FinalizeOrchestratorOptions {
|
|||
* Produce a fully materialized `ScopeResolutionIndexes` from the
|
||||
* workspace's per-file artifacts.
|
||||
*
|
||||
* Pure function (given pure hooks). No I/O, no globals consulted. The
|
||||
* pipeline calls this once per ingestion run and hands the result to
|
||||
* The pipeline calls this once per ingestion run and hands the result to
|
||||
* `MutableSemanticModel.attachScopeIndexes`.
|
||||
* Optional heap diagnostics use the pipeline's existing debug flags.
|
||||
*/
|
||||
export function finalizeScopeModel(
|
||||
parsedFiles: readonly ParsedFile[],
|
||||
|
|
@ -92,15 +93,21 @@ export function finalizeScopeModel(
|
|||
): ScopeResolutionIndexes {
|
||||
const hooks = withDefaultHooks(options.hooks ?? {});
|
||||
const workspaceIndex: WorkspaceIndex = options.workspaceIndex ?? undefined;
|
||||
const onProgress = isDebugHeapEnabled()
|
||||
? (phase: string) => logHeapProbe(`sr-finalize-${phase}`)
|
||||
: undefined;
|
||||
|
||||
// ── Step 1: Shared finalize — runs SCC-aware cross-file link + binding
|
||||
// materialization. Returns linked imports + merged bindings per module
|
||||
// scope + SCC condensation + stats.
|
||||
onProgress?.('input-start');
|
||||
const finalizeInput = {
|
||||
files: parsedFiles.map((file) => toFinalizeFile(file, options.moduleExports?.(file))),
|
||||
workspaceIndex,
|
||||
onProgress,
|
||||
};
|
||||
const finalizeOut = finalize(finalizeInput, hooks);
|
||||
onProgress?.('indexes-start');
|
||||
|
||||
// ── Step 2: Workspace-wide indexes built from the per-file unions.
|
||||
// These are pure aggregations — no algorithm beyond what the builders
|
||||
|
|
@ -125,9 +132,13 @@ export function finalizeScopeModel(
|
|||
const scopeTree = parseTruthyEnv(process.env.GITNEXUS_DISK_SCOPE_INDEX)
|
||||
? new TransitionalScopeTree(allScopes)
|
||||
: buildScopeTree(allScopes);
|
||||
onProgress?.('scopeTree-end');
|
||||
const defs = buildDefIndex(allDefs);
|
||||
onProgress?.('defs-end');
|
||||
const qualifiedNames = buildQualifiedNameIndex(allDefs);
|
||||
onProgress?.('qualifiedNames-end');
|
||||
const moduleScopes = buildModuleScopeIndex(moduleEntries);
|
||||
onProgress?.('moduleScopes-end');
|
||||
|
||||
// ── Step 3: MethodDispatchIndex. Today we lack per-language MRO
|
||||
// strategies wired into this orchestrator (that belongs with the
|
||||
|
|
|
|||
|
|
@ -316,6 +316,52 @@ describe('finalize', () => {
|
|||
});
|
||||
|
||||
describe('wildcard expansion', () => {
|
||||
it.each(
|
||||
[
|
||||
{ importKind: 'none', parsedImports: [] },
|
||||
{ importKind: 'ordinary', parsedImports: [named('Leaf', 'Leaf', 'leaf')] },
|
||||
{ importKind: 'unresolved wildcard', parsedImports: [wildcard('missing')] },
|
||||
{ importKind: 'external wildcard', parsedImports: [wildcard('external')] },
|
||||
{ importKind: 'unresolved reexport', parsedImports: [reexport('X', 'X', 'missing')] },
|
||||
{ importKind: 'external reexport', parsedImports: [reexport('X', 'X', 'external')] },
|
||||
].flatMap((testCase) =>
|
||||
[false, true].map((filteredWildcards) => ({ ...testCase, filteredWildcards })),
|
||||
),
|
||||
)(
|
||||
'does not inspect unrelated exports after a terminal miss ($importKind, filtered=$filteredWildcards)',
|
||||
({ parsedImports, filteredWildcards }) => {
|
||||
const leaf = file('leaf', [def('def:Leaf', 'Class', 'Leaf')]);
|
||||
const target = file('target', [], parsedImports);
|
||||
let exportSurfaceReads = 0;
|
||||
const unrelated: FinalizeFile = {
|
||||
...file('unrelated', [def('def:Unrelated', 'Class', 'Unrelated')]),
|
||||
get localExports() {
|
||||
exportSurfaceReads++;
|
||||
return new Map([['Unrelated', def('def:Unrelated', 'Class', 'Unrelated')]]);
|
||||
},
|
||||
};
|
||||
const wildcardImporter = file('wildcard-importer', [], [wildcard('unrelated')]);
|
||||
const consumer = file('consumer', [], [named('Missing', 'Missing', 'target')]);
|
||||
const files = [consumer, target, leaf, wildcardImporter, unrelated];
|
||||
const hooks = defaultHooks(files);
|
||||
const out = finalize(
|
||||
{ files, workspaceIndex: undefined },
|
||||
{
|
||||
...hooks,
|
||||
resolveImportTarget: (targetRaw, fromFile, workspace) =>
|
||||
targetRaw === 'external'
|
||||
? targetRaw
|
||||
: hooks.resolveImportTarget(targetRaw, fromFile, workspace),
|
||||
...(filteredWildcards ? { filterWildcardNames: (_scope, names) => names } : {}),
|
||||
},
|
||||
);
|
||||
|
||||
expect(firstImport(out, consumer.moduleScope)?.linkStatus).toBe('unresolved');
|
||||
expect(firstImport(out, wildcardImporter.moduleScope)?.targetDefId).toBe('def:Unrelated');
|
||||
expect(exportSurfaceReads).toBe(filteredWildcards ? 2 : 1);
|
||||
},
|
||||
);
|
||||
|
||||
it('expands `wildcard` into one ImportEdge per exported name', () => {
|
||||
const b = file('b', [
|
||||
def('def:b.X', 'Class', 'b.X'),
|
||||
|
|
@ -356,12 +402,19 @@ describe('finalize', () => {
|
|||
});
|
||||
|
||||
describe('re-export flattening', () => {
|
||||
it('sets transitiveVia on reexport edges', () => {
|
||||
it('resolves a reexport and its consumer after a terminal miss', () => {
|
||||
const c = file('c', [def('def:c.X', 'Class', 'c.X')]);
|
||||
const b = file('b', [], [reexport('X', 'X', 'c')]);
|
||||
const a = file('a', [], [named('X', 'X', 'b')]);
|
||||
const a = file('a', [], [named('Absent', 'Absent', 'c'), named('X', 'X', 'b')]);
|
||||
const files = [a, b, c];
|
||||
const out = finalize({ files, workspaceIndex: undefined }, defaultHooks(files));
|
||||
const imports = out.imports.get(a.moduleScope);
|
||||
expect(imports).toHaveLength(2);
|
||||
expect(imports?.[0].linkStatus).toBe('unresolved');
|
||||
expect(imports?.[1]).toMatchObject({
|
||||
targetDefId: 'def:c.X',
|
||||
transitiveVia: ['b', 'c'],
|
||||
});
|
||||
const reexportEdge = firstImport(out, b.moduleScope)!;
|
||||
expect(reexportEdge.kind).toBe('reexport');
|
||||
expect(reexportEdge.transitiveVia).toEqual(['c']);
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue