From b2c72997ea347ceff682f26f260a534975525563 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Magyar?= Date: Tue, 27 Oct 2026 15:41:48 +0000 Subject: [PATCH] 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 --- .../scope-resolution/finalize-algorithm.ts | 65 ++++++++++++----- gitnexus/bench/scope-resolution/README.md | 25 +++++++ .../bench/scope-resolution/finalize-miss.mts | 71 +++++++++++++++++++ .../core/ingestion/finalize-orchestrator.ts | 15 +++- .../finalize-algorithm.test.ts | 57 ++++++++++++++- 5 files changed, 212 insertions(+), 21 deletions(-) create mode 100644 gitnexus/bench/scope-resolution/finalize-miss.mts diff --git a/gitnexus-shared/src/scope-resolution/finalize-algorithm.ts b/gitnexus-shared/src/scope-resolution/finalize-algorithm.ts index 2ee32c3a7..14f884555 100644 --- a/gitnexus-shared/src/scope-resolution/finalize-algorithm.ts +++ b/gitnexus-shared/src/scope-resolution/finalize-algorithm.ts @@ -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(); 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(); + const reexportingFiles = new Set(); 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 | 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, - getReexportClosures: () => ReadonlyMap, + getReexportClosures: (targetFile: string) => ReadonlyMap, 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; +const EMPTY_REEXPORT_CLOSURES: ReadonlyMap = new Map(); /** * Build per-file re-export closures. @@ -1610,7 +1641,7 @@ function expandWildcard( byFilePath: Map, hooks: FinalizeHooks, workspace: WorkspaceIndex, - getReexportClosures: () => ReadonlyMap, + getReexportClosures: (targetFile: string) => ReadonlyMap, ): 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, diff --git a/gitnexus/bench/scope-resolution/README.md b/gitnexus/bench/scope-resolution/README.md index 7f853847a..862636b0f 100644 --- a/gitnexus/bench/scope-resolution/README.md +++ b/gitnexus/bench/scope-resolution/README.md @@ -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. diff --git a/gitnexus/bench/scope-resolution/finalize-miss.mts b/gitnexus/bench/scope-resolution/finalize-miss.mts new file mode 100644 index 000000000..d6bd0a700 --- /dev/null +++ b/gitnexus/bench/scope-resolution/finalize-miss.mts @@ -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'), + }), +); diff --git a/gitnexus/src/core/ingestion/finalize-orchestrator.ts b/gitnexus/src/core/ingestion/finalize-orchestrator.ts index e72182acd..cc35dcb30 100644 --- a/gitnexus/src/core/ingestion/finalize-orchestrator.ts +++ b/gitnexus/src/core/ingestion/finalize-orchestrator.ts @@ -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 diff --git a/gitnexus/test/unit/scope-resolution/finalize-algorithm.test.ts b/gitnexus/test/unit/scope-resolution/finalize-algorithm.test.ts index 45450ddd6..0a133cf1a 100644 --- a/gitnexus/test/unit/scope-resolution/finalize-algorithm.test.ts +++ b/gitnexus/test/unit/scope-resolution/finalize-algorithm.test.ts @@ -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']);