From 9fa18384acc377f9083042579fd0a06b11954491 Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Sat, 25 Jul 2026 05:36:46 +0000 Subject: [PATCH] perf(2680): zero-allocation field scan brings iteration back to parity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third and final step on the iteration cost. The memory win had come with a 6.8x iteration regression; the previous commit cut that to 1.8x by making the synthesized id lazy and removing generator overhead. The residual was object allocation itself — 6.5M instances across the six full relationship scans an analyze performs — which no amount of tuning removes while the read API hands back objects. So the hot consumers stop asking for objects. Adds `KnowledgeGraph.forEachRelationshipFields`, which passes (sourceId, targetId, type, confidence) as primitives — exactly and only what every whole-graph scan reads. On the sink those come straight out of the columns, allocating nothing; on the object-based graph they are read off the stored relationship, so the flag-off path is unaffected. Converted the five whole-graph scans: community detection (x2), process extraction (x2), and the local-symbol pruner. `isFileDefinesEdge` now takes (type, sourceId) rather than a relationship. The taint fixpoint's by-type pass is left alone — one scan of six, and converting it would turn an indexed bucket lookup into a full scan on the object-based graph. heap 820 MB -> 623 MB (1.32x better) scans ~82 ms -> ~90 ms (was 651 ms; now parity within noise) Also deletes the pruner's `hasStreamedSemanticEdge` option, which has had no caller since the sink's reads became complete — a dead knob is worse than no knob. Verified: 104 tests across the eight affected suites, including the pruner's pipeline integration test (which needs the raised worker-ready timeout on this host; it passes cleanly with it and its failures are the known 5s handshake). Refs #2680 --- gitnexus/src/core/graph/graph.ts | 5 ++ gitnexus/src/core/graph/types.ts | 13 ++++ .../src/core/ingestion/community-processor.ts | 26 ++++---- .../src/core/ingestion/local-symbol-pruner.ts | 51 +++++---------- .../src/core/ingestion/process-processor.ts | 29 ++++----- gitnexus/src/core/lbug/graph-emit-sink.ts | 63 +++++++++++++------ gitnexus/src/core/lbug/pdg-emit-sink.ts | 5 ++ 7 files changed, 111 insertions(+), 81 deletions(-) diff --git a/gitnexus/src/core/graph/graph.ts b/gitnexus/src/core/graph/graph.ts index c906e1b10..1c708a4af 100644 --- a/gitnexus/src/core/graph/graph.ts +++ b/gitnexus/src/core/graph/graph.ts @@ -162,6 +162,11 @@ export const createKnowledgeGraph = (): KnowledgeGraph => { forEachRelationship(fn: (rel: GraphRelationship) => void) { relationshipMap.forEach(fn); }, + forEachRelationshipFields( + fn: (sourceId: string, targetId: string, type: RelationshipType, confidence: number) => void, + ) { + relationshipMap.forEach((rel) => fn(rel.sourceId, rel.targetId, rel.type, rel.confidence)); + }, getNode: (id: string) => nodeMap.get(id), // O(1) count getters - avoid creating arrays just for length diff --git a/gitnexus/src/core/graph/types.ts b/gitnexus/src/core/graph/types.ts index 539f77987..c051a77e5 100644 --- a/gitnexus/src/core/graph/types.ts +++ b/gitnexus/src/core/graph/types.ts @@ -27,6 +27,19 @@ export interface KnowledgeGraph { iterRelationshipsByType: (type: RelationshipType) => IterableIterator; forEachNode: (fn: (node: GraphNode) => void) => void; forEachRelationship: (fn: (rel: GraphRelationship) => void) => void; + /** + * Zero-allocation relationship scan: fields, not objects (#2680). + * + * The whole-graph scans (the local-symbol pruner, community detection, + * process extraction) read only these four fields, and materializing a + * `GraphRelationship` per edge just to read them dominates iteration cost once + * relationships are held columnar — measured at ~90 ms per analyze on a + * million-edge graph. Prefer this over `forEachRelationship` in any pass that + * walks every edge and needs no other field. + */ + forEachRelationshipFields: ( + fn: (sourceId: string, targetId: string, type: RelationshipType, confidence: number) => void, + ) => void; getNode: (id: string) => GraphNode | undefined; nodeCount: number; relationshipCount: number; diff --git a/gitnexus/src/core/ingestion/community-processor.ts b/gitnexus/src/core/ingestion/community-processor.ts index ff892ae73..7a91eb574 100644 --- a/gitnexus/src/core/ingestion/community-processor.ts +++ b/gitnexus/src/core/ingestion/community-processor.ts @@ -290,14 +290,16 @@ export const buildCommunityProjection = (knowledgeGraph: KnowledgeGraph): Commun const connectedNodes = new Set(); const nodeDegree = new Map(); - knowledgeGraph.forEachRelationship((rel) => { - if (!isClusteringRelationship(rel.type) || rel.sourceId === rel.targetId) return; - if (isLarge && rel.confidence < MIN_CONFIDENCE_LARGE) return; + // Field-wise scan (#2680): this walks every edge and reads only these four, + // so taking objects would allocate one per edge for nothing. + knowledgeGraph.forEachRelationshipFields((sourceId, targetId, type, confidence) => { + if (!isClusteringRelationship(type) || sourceId === targetId) return; + if (isLarge && confidence < MIN_CONFIDENCE_LARGE) return; - connectedNodes.add(rel.sourceId); - connectedNodes.add(rel.targetId); - nodeDegree.set(rel.sourceId, (nodeDegree.get(rel.sourceId) || 0) + 1); - nodeDegree.set(rel.targetId, (nodeDegree.get(rel.targetId) || 0) + 1); + connectedNodes.add(sourceId); + connectedNodes.add(targetId); + nodeDegree.set(sourceId, (nodeDegree.get(sourceId) || 0) + 1); + nodeDegree.set(targetId, (nodeDegree.get(targetId) || 0) + 1); }); const nodes: CommunityProjectionNode[] = []; @@ -328,12 +330,12 @@ export const buildCommunityProjection = (knowledgeGraph: KnowledgeGraph): Commun const seenEdges = new Set(); const edges: Array = []; - knowledgeGraph.forEachRelationship((rel) => { - if (!isClusteringRelationship(rel.type) || rel.sourceId === rel.targetId) return; - if (isLarge && rel.confidence < MIN_CONFIDENCE_LARGE) return; + knowledgeGraph.forEachRelationshipFields((sourceId, targetId, type, confidence) => { + if (!isClusteringRelationship(type) || sourceId === targetId) return; + if (isLarge && confidence < MIN_CONFIDENCE_LARGE) return; - const sourceIndex = nodeIndexById.get(rel.sourceId); - const targetIndex = nodeIndexById.get(rel.targetId); + const sourceIndex = nodeIndexById.get(sourceId); + const targetIndex = nodeIndexById.get(targetId); if (sourceIndex === undefined || targetIndex === undefined || sourceIndex === targetIndex) return; diff --git a/gitnexus/src/core/ingestion/local-symbol-pruner.ts b/gitnexus/src/core/ingestion/local-symbol-pruner.ts index caa13d505..24e9733ff 100644 --- a/gitnexus/src/core/ingestion/local-symbol-pruner.ts +++ b/gitnexus/src/core/ingestion/local-symbol-pruner.ts @@ -1,4 +1,4 @@ -import type { GraphNode, GraphRelationship, NodeLabel } from 'gitnexus-shared'; +import type { GraphNode, NodeLabel, RelationshipType } from 'gitnexus-shared'; import type { KnowledgeGraph } from '../graph/types.js'; import { parseTruthyEnv } from './utils/env.js'; @@ -30,28 +30,18 @@ const isLocalValueCandidate = (node: GraphNode): boolean => { // True when `rel` is the structural `File -> DEFINES -> candidate` edge. Callers // guard on the candidate already being the edge target, so only the source label // needs checking here. -const isFileDefinesEdge = (graph: KnowledgeGraph, rel: GraphRelationship): boolean => { - if (rel.type !== 'DEFINES') return false; - return graph.getNode(rel.sourceId)?.label === 'File'; +const isFileDefinesEdge = ( + graph: KnowledgeGraph, + type: RelationshipType, + sourceId: string, +): boolean => { + if (type !== 'DEFINES') return false; + return graph.getNode(sourceId)?.label === 'File'; }; export const pruneLocalValueSymbols = ( graph: KnowledgeGraph, - options: { - keepLocalValueSymbols?: boolean; - /** - * Reports whether a node is an endpoint of a relationship that has already - * been streamed to CSV and is therefore NOT visible to the - * `iterRelationships()` scan below (#2680). - * - * Without this, a block-local symbol whose only reference is a streamed - * edge looks unreferenced, gets pruned, and leaves the streamed CSV row - * pointing at a node with no row — a dangling edge at COPY time, not just - * a semantic mistake. Absent (the default) the scan alone decides, so - * behaviour is unchanged when streaming is off. - */ - hasStreamedSemanticEdge?: (nodeId: string) => boolean; - } = {}, + options: { keepLocalValueSymbols?: boolean } = {}, ): LocalSymbolPruneStats => { if (options.keepLocalValueSymbols ?? shouldKeepLocalValueSymbols()) { return emptyStats(true); @@ -65,28 +55,21 @@ export const pruneLocalValueSymbols = ( if (candidateIds.size === 0) return emptyStats(false); const candidatesWithSemanticEdges = new Set(); - for (const rel of graph.iterRelationships()) { + // Field-wise scan (#2680): a whole-graph walk that reads only these three, so + // materializing a relationship object per edge would be pure overhead. + graph.forEachRelationshipFields((sourceId, targetId, type) => { // Any outgoing edge from a candidate is a semantic edge: the only structural // edge a block-local value symbol carries is the incoming File -> DEFINES, on // which the candidate is the target, never the source. - if (candidateIds.has(rel.sourceId)) { - candidatesWithSemanticEdges.add(rel.sourceId); + if (candidateIds.has(sourceId)) { + candidatesWithSemanticEdges.add(sourceId); } // An incoming edge is semantic unless it is the structural File -> DEFINES. - if (candidateIds.has(rel.targetId)) { - if (!isFileDefinesEdge(graph, rel)) { - candidatesWithSemanticEdges.add(rel.targetId); - } + if (candidateIds.has(targetId) && !isFileDefinesEdge(graph, type, sourceId)) { + candidatesWithSemanticEdges.add(targetId); } - } - - const hasStreamedSemanticEdge = options.hasStreamedSemanticEdge; - if (hasStreamedSemanticEdge !== undefined) { - for (const candidateId of candidateIds) { - if (hasStreamedSemanticEdge(candidateId)) candidatesWithSemanticEdges.add(candidateId); - } - } + }); let prunedNodes = 0; for (const candidateId of candidateIds) { diff --git a/gitnexus/src/core/ingestion/process-processor.ts b/gitnexus/src/core/ingestion/process-processor.ts index aa744e54d..dbec25474 100644 --- a/gitnexus/src/core/ingestion/process-processor.ts +++ b/gitnexus/src/core/ingestion/process-processor.ts @@ -230,14 +230,13 @@ const MIN_TRACE_CONFIDENCE = 0.5; const buildCallsGraph = (graph: KnowledgeGraph): AdjacencyList => { const adj = new Map(); - for (const rel of graph.iterRelationships()) { - if (rel.type === 'CALLS' && rel.confidence >= MIN_TRACE_CONFIDENCE) { - if (!adj.has(rel.sourceId)) { - adj.set(rel.sourceId, []); - } - adj.get(rel.sourceId)!.push(rel.targetId); - } - } + // Field-wise scan (#2680) — whole-graph walk, four fields, no object needed. + graph.forEachRelationshipFields((sourceId, targetId, type, confidence) => { + if (type !== 'CALLS' || confidence < MIN_TRACE_CONFIDENCE) return; + const existing = adj.get(sourceId); + if (existing === undefined) adj.set(sourceId, [targetId]); + else existing.push(targetId); + }); return adj; }; @@ -245,14 +244,12 @@ const buildCallsGraph = (graph: KnowledgeGraph): AdjacencyList => { const buildReverseCallsGraph = (graph: KnowledgeGraph): AdjacencyList => { const adj = new Map(); - for (const rel of graph.iterRelationships()) { - if (rel.type === 'CALLS' && rel.confidence >= MIN_TRACE_CONFIDENCE) { - if (!adj.has(rel.targetId)) { - adj.set(rel.targetId, []); - } - adj.get(rel.targetId)!.push(rel.sourceId); - } - } + graph.forEachRelationshipFields((sourceId, targetId, type, confidence) => { + if (type !== 'CALLS' || confidence < MIN_TRACE_CONFIDENCE) return; + const existing = adj.get(targetId); + if (existing === undefined) adj.set(targetId, [sourceId]); + else existing.push(sourceId); + }); return adj; }; diff --git a/gitnexus/src/core/lbug/graph-emit-sink.ts b/gitnexus/src/core/lbug/graph-emit-sink.ts index 46dde2028..0b9765155 100644 --- a/gitnexus/src/core/lbug/graph-emit-sink.ts +++ b/gitnexus/src/core/lbug/graph-emit-sink.ts @@ -38,29 +38,35 @@ * * ## What it costs — measured, not assumed * - * Reads allocate: iteration rebuilds objects instead of handing back stored - * ones, and a real analyze performs SIX full relationship scans (the pruner, - * community detection x2, process extraction x2, and the taint fixpoint's CALLS - * pass). Measured on the same 400k-node / 1.08M-edge graph, all edges - * streamable: + * Measured on the same 400k-node / 1.08M-edge graph, all edges streamable, each + * arm running what its own consumers actually call: * * heap 820 MB -> 623 MB (1.32x better) - * scans 107 ms -> 195 ms (1.8x worse) + * scans ~82 ms -> ~90 ms (parity, within run-to-run noise) * - * The first cut of this was 6.8x worse (651 ms). Two fixes, both measured: - * building the ~150-character synthesized `id` eagerly cost 436 ms of that, so - * it moved to a lazy prototype getter ({@link StreamedRelationship}); and - * {@link forEachRelationship} now loops the columns directly while - * {@link iterRelationships} reuses one iterator-result record, since a - * fresh-result hand-rolled iterator measured WORSE (252 ms) than the generator - * it replaced. + * Getting there took three measured steps, because the naive version was 6.8x + * WORSE (651 ms) — reads rebuild objects, and a real analyze performs SIX full + * relationship scans (the pruner, community detection x2, process extraction x2, + * and the taint fixpoint's CALLS pass): * - * The residual ~88 ms is object allocation — 6.5M instances across the six - * scans — and it is irreducible while the read API returns objects at all. The - * fix, if iteration ever needs true parity, is a field-wise callback - * (`sourceId, targetId, type, confidence` as primitives) for the hot consumers, - * all four of which read only those. That is an interface change across the - * graph and its consumers, so it wants its own measurement and its own change. + * 1. The ~150-character synthesized `id` was built eagerly on every read — 6.5M + * concatenations for a field no in-pipeline consumer reads. Isolating it + * showed 436 ms of the regression. It is now a lazy prototype getter on + * {@link StreamedRelationship}. + * 2. Generator and iterator-protocol overhead: {@link forEachRelationship} loops + * the columns directly, and {@link iterRelationships} reuses one + * iterator-result record. Note a hand-rolled iterator allocating a fresh + * `{value, done}` per edge measured WORSE (252 ms) than the generator it + * replaced, so the obvious rewrite is not the one that shipped. + * 3. The remaining ~90 ms was object allocation itself, irreducible while the + * read API returns objects — so the five whole-graph scans moved to + * `forEachRelationshipFields`, which passes the four fields they actually + * read as primitives and allocates nothing. See + * {@link GraphEmitSink.forEachRelationshipFields}. + * + * The last allocating scan is the taint fixpoint's `iterRelationshipsByType` + * pass; it is one scan of six and accounts for the small residual. Give it a + * by-type field variant only if a measurement says it matters. * * It is in any case NOT O(chunk) — node identity and the resolution registries * stay O(repo). True O(chunk) needs DB-side resolution and Leiden (#2337), at @@ -481,6 +487,25 @@ export class GraphEmitSink implements KnowledgeGraph, GraphEmitControl { forEachNode(fn: (node: GraphNode) => void): void { this.real.forEachNode(fn); } + /** + * The fast path: streamed edges are read straight out of the columns, so a + * whole-graph scan allocates NOTHING. This is what keeps iteration at parity + * with the object-based graph despite holding relationships columnar. + */ + forEachRelationshipFields( + fn: (sourceId: string, targetId: string, type: RelationshipType, confidence: number) => void, + ): void { + this.real.forEachRelationshipFields(fn); + for (let ix = 0; ix < this.srcIx.length; ix++) { + fn( + this.nodeIdByIx[this.srcIx[ix]], + this.nodeIdByIx[this.tgtIx[ix]], + this.relTypes[ix], + this.confidences[ix], + ); + } + } + /** Direct loop rather than delegating to {@link iterRelationships}: this is * the form community detection uses (twice), and skipping the generator and * iterator protocol is measurably cheaper on a million-edge scan. */ diff --git a/gitnexus/src/core/lbug/pdg-emit-sink.ts b/gitnexus/src/core/lbug/pdg-emit-sink.ts index 6dc9597d0..79f8b899f 100644 --- a/gitnexus/src/core/lbug/pdg-emit-sink.ts +++ b/gitnexus/src/core/lbug/pdg-emit-sink.ts @@ -281,6 +281,11 @@ export class PdgEmitSink implements KnowledgeGraph { forEachRelationship(fn: (rel: GraphRelationship) => void): void { this.real.forEachRelationship(fn); } + forEachRelationshipFields( + fn: (sourceId: string, targetId: string, type: RelationshipType, confidence: number) => void, + ): void { + this.real.forEachRelationshipFields(fn); + } getNode(id: string): GraphNode | undefined { return this.real.getNode(id); }