mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-09 03:17:54 +00:00
perf(2680): zero-allocation field scan brings iteration back to parity
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
This commit is contained in:
parent
a66b63897f
commit
9fa18384ac
7 changed files with 111 additions and 81 deletions
|
|
@ -162,6 +162,11 @@ export const createKnowledgeGraph = (): KnowledgeGraph => {
|
||||||
forEachRelationship(fn: (rel: GraphRelationship) => void) {
|
forEachRelationship(fn: (rel: GraphRelationship) => void) {
|
||||||
relationshipMap.forEach(fn);
|
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),
|
getNode: (id: string) => nodeMap.get(id),
|
||||||
|
|
||||||
// O(1) count getters - avoid creating arrays just for length
|
// O(1) count getters - avoid creating arrays just for length
|
||||||
|
|
|
||||||
|
|
@ -27,6 +27,19 @@ export interface KnowledgeGraph {
|
||||||
iterRelationshipsByType: (type: RelationshipType) => IterableIterator<GraphRelationship>;
|
iterRelationshipsByType: (type: RelationshipType) => IterableIterator<GraphRelationship>;
|
||||||
forEachNode: (fn: (node: GraphNode) => void) => void;
|
forEachNode: (fn: (node: GraphNode) => void) => void;
|
||||||
forEachRelationship: (fn: (rel: GraphRelationship) => 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;
|
getNode: (id: string) => GraphNode | undefined;
|
||||||
nodeCount: number;
|
nodeCount: number;
|
||||||
relationshipCount: number;
|
relationshipCount: number;
|
||||||
|
|
|
||||||
|
|
@ -290,14 +290,16 @@ export const buildCommunityProjection = (knowledgeGraph: KnowledgeGraph): Commun
|
||||||
const connectedNodes = new Set<string>();
|
const connectedNodes = new Set<string>();
|
||||||
const nodeDegree = new Map<string, number>();
|
const nodeDegree = new Map<string, number>();
|
||||||
|
|
||||||
knowledgeGraph.forEachRelationship((rel) => {
|
// Field-wise scan (#2680): this walks every edge and reads only these four,
|
||||||
if (!isClusteringRelationship(rel.type) || rel.sourceId === rel.targetId) return;
|
// so taking objects would allocate one per edge for nothing.
|
||||||
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;
|
||||||
|
|
||||||
connectedNodes.add(rel.sourceId);
|
connectedNodes.add(sourceId);
|
||||||
connectedNodes.add(rel.targetId);
|
connectedNodes.add(targetId);
|
||||||
nodeDegree.set(rel.sourceId, (nodeDegree.get(rel.sourceId) || 0) + 1);
|
nodeDegree.set(sourceId, (nodeDegree.get(sourceId) || 0) + 1);
|
||||||
nodeDegree.set(rel.targetId, (nodeDegree.get(rel.targetId) || 0) + 1);
|
nodeDegree.set(targetId, (nodeDegree.get(targetId) || 0) + 1);
|
||||||
});
|
});
|
||||||
|
|
||||||
const nodes: CommunityProjectionNode[] = [];
|
const nodes: CommunityProjectionNode[] = [];
|
||||||
|
|
@ -328,12 +330,12 @@ export const buildCommunityProjection = (knowledgeGraph: KnowledgeGraph): Commun
|
||||||
const seenEdges = new Set<string>();
|
const seenEdges = new Set<string>();
|
||||||
const edges: Array<readonly [number, number]> = [];
|
const edges: Array<readonly [number, number]> = [];
|
||||||
|
|
||||||
knowledgeGraph.forEachRelationship((rel) => {
|
knowledgeGraph.forEachRelationshipFields((sourceId, targetId, type, confidence) => {
|
||||||
if (!isClusteringRelationship(rel.type) || rel.sourceId === rel.targetId) return;
|
if (!isClusteringRelationship(type) || sourceId === targetId) return;
|
||||||
if (isLarge && rel.confidence < MIN_CONFIDENCE_LARGE) return;
|
if (isLarge && confidence < MIN_CONFIDENCE_LARGE) return;
|
||||||
|
|
||||||
const sourceIndex = nodeIndexById.get(rel.sourceId);
|
const sourceIndex = nodeIndexById.get(sourceId);
|
||||||
const targetIndex = nodeIndexById.get(rel.targetId);
|
const targetIndex = nodeIndexById.get(targetId);
|
||||||
if (sourceIndex === undefined || targetIndex === undefined || sourceIndex === targetIndex)
|
if (sourceIndex === undefined || targetIndex === undefined || sourceIndex === targetIndex)
|
||||||
return;
|
return;
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -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 type { KnowledgeGraph } from '../graph/types.js';
|
||||||
import { parseTruthyEnv } from './utils/env.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
|
// 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
|
// guard on the candidate already being the edge target, so only the source label
|
||||||
// needs checking here.
|
// needs checking here.
|
||||||
const isFileDefinesEdge = (graph: KnowledgeGraph, rel: GraphRelationship): boolean => {
|
const isFileDefinesEdge = (
|
||||||
if (rel.type !== 'DEFINES') return false;
|
graph: KnowledgeGraph,
|
||||||
return graph.getNode(rel.sourceId)?.label === 'File';
|
type: RelationshipType,
|
||||||
|
sourceId: string,
|
||||||
|
): boolean => {
|
||||||
|
if (type !== 'DEFINES') return false;
|
||||||
|
return graph.getNode(sourceId)?.label === 'File';
|
||||||
};
|
};
|
||||||
|
|
||||||
export const pruneLocalValueSymbols = (
|
export const pruneLocalValueSymbols = (
|
||||||
graph: KnowledgeGraph,
|
graph: KnowledgeGraph,
|
||||||
options: {
|
options: { keepLocalValueSymbols?: boolean } = {},
|
||||||
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;
|
|
||||||
} = {},
|
|
||||||
): LocalSymbolPruneStats => {
|
): LocalSymbolPruneStats => {
|
||||||
if (options.keepLocalValueSymbols ?? shouldKeepLocalValueSymbols()) {
|
if (options.keepLocalValueSymbols ?? shouldKeepLocalValueSymbols()) {
|
||||||
return emptyStats(true);
|
return emptyStats(true);
|
||||||
|
|
@ -65,28 +55,21 @@ export const pruneLocalValueSymbols = (
|
||||||
if (candidateIds.size === 0) return emptyStats(false);
|
if (candidateIds.size === 0) return emptyStats(false);
|
||||||
|
|
||||||
const candidatesWithSemanticEdges = new Set<string>();
|
const candidatesWithSemanticEdges = new Set<string>();
|
||||||
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
|
// 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
|
// edge a block-local value symbol carries is the incoming File -> DEFINES, on
|
||||||
// which the candidate is the target, never the source.
|
// which the candidate is the target, never the source.
|
||||||
if (candidateIds.has(rel.sourceId)) {
|
if (candidateIds.has(sourceId)) {
|
||||||
candidatesWithSemanticEdges.add(rel.sourceId);
|
candidatesWithSemanticEdges.add(sourceId);
|
||||||
}
|
}
|
||||||
|
|
||||||
// An incoming edge is semantic unless it is the structural File -> DEFINES.
|
// An incoming edge is semantic unless it is the structural File -> DEFINES.
|
||||||
if (candidateIds.has(rel.targetId)) {
|
if (candidateIds.has(targetId) && !isFileDefinesEdge(graph, type, sourceId)) {
|
||||||
if (!isFileDefinesEdge(graph, rel)) {
|
candidatesWithSemanticEdges.add(targetId);
|
||||||
candidatesWithSemanticEdges.add(rel.targetId);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
});
|
||||||
|
|
||||||
const hasStreamedSemanticEdge = options.hasStreamedSemanticEdge;
|
|
||||||
if (hasStreamedSemanticEdge !== undefined) {
|
|
||||||
for (const candidateId of candidateIds) {
|
|
||||||
if (hasStreamedSemanticEdge(candidateId)) candidatesWithSemanticEdges.add(candidateId);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
let prunedNodes = 0;
|
let prunedNodes = 0;
|
||||||
for (const candidateId of candidateIds) {
|
for (const candidateId of candidateIds) {
|
||||||
|
|
|
||||||
|
|
@ -230,14 +230,13 @@ const MIN_TRACE_CONFIDENCE = 0.5;
|
||||||
const buildCallsGraph = (graph: KnowledgeGraph): AdjacencyList => {
|
const buildCallsGraph = (graph: KnowledgeGraph): AdjacencyList => {
|
||||||
const adj = new Map<string, string[]>();
|
const adj = new Map<string, string[]>();
|
||||||
|
|
||||||
for (const rel of graph.iterRelationships()) {
|
// Field-wise scan (#2680) — whole-graph walk, four fields, no object needed.
|
||||||
if (rel.type === 'CALLS' && rel.confidence >= MIN_TRACE_CONFIDENCE) {
|
graph.forEachRelationshipFields((sourceId, targetId, type, confidence) => {
|
||||||
if (!adj.has(rel.sourceId)) {
|
if (type !== 'CALLS' || confidence < MIN_TRACE_CONFIDENCE) return;
|
||||||
adj.set(rel.sourceId, []);
|
const existing = adj.get(sourceId);
|
||||||
}
|
if (existing === undefined) adj.set(sourceId, [targetId]);
|
||||||
adj.get(rel.sourceId)!.push(rel.targetId);
|
else existing.push(targetId);
|
||||||
}
|
});
|
||||||
}
|
|
||||||
|
|
||||||
return adj;
|
return adj;
|
||||||
};
|
};
|
||||||
|
|
@ -245,14 +244,12 @@ const buildCallsGraph = (graph: KnowledgeGraph): AdjacencyList => {
|
||||||
const buildReverseCallsGraph = (graph: KnowledgeGraph): AdjacencyList => {
|
const buildReverseCallsGraph = (graph: KnowledgeGraph): AdjacencyList => {
|
||||||
const adj = new Map<string, string[]>();
|
const adj = new Map<string, string[]>();
|
||||||
|
|
||||||
for (const rel of graph.iterRelationships()) {
|
graph.forEachRelationshipFields((sourceId, targetId, type, confidence) => {
|
||||||
if (rel.type === 'CALLS' && rel.confidence >= MIN_TRACE_CONFIDENCE) {
|
if (type !== 'CALLS' || confidence < MIN_TRACE_CONFIDENCE) return;
|
||||||
if (!adj.has(rel.targetId)) {
|
const existing = adj.get(targetId);
|
||||||
adj.set(rel.targetId, []);
|
if (existing === undefined) adj.set(targetId, [sourceId]);
|
||||||
}
|
else existing.push(sourceId);
|
||||||
adj.get(rel.targetId)!.push(rel.sourceId);
|
});
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
return adj;
|
return adj;
|
||||||
};
|
};
|
||||||
|
|
|
||||||
|
|
@ -38,29 +38,35 @@
|
||||||
*
|
*
|
||||||
* ## What it costs — measured, not assumed
|
* ## What it costs — measured, not assumed
|
||||||
*
|
*
|
||||||
* Reads allocate: iteration rebuilds objects instead of handing back stored
|
* Measured on the same 400k-node / 1.08M-edge graph, all edges streamable, each
|
||||||
* ones, and a real analyze performs SIX full relationship scans (the pruner,
|
* arm running what its own consumers actually call:
|
||||||
* 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:
|
|
||||||
*
|
*
|
||||||
* heap 820 MB -> 623 MB (1.32x better)
|
* 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:
|
* Getting there took three measured steps, because the naive version was 6.8x
|
||||||
* building the ~150-character synthesized `id` eagerly cost 436 ms of that, so
|
* WORSE (651 ms) — reads rebuild objects, and a real analyze performs SIX full
|
||||||
* it moved to a lazy prototype getter ({@link StreamedRelationship}); and
|
* relationship scans (the pruner, community detection x2, process extraction x2,
|
||||||
* {@link forEachRelationship} now loops the columns directly while
|
* and the taint fixpoint's CALLS pass):
|
||||||
* {@link iterRelationships} reuses one iterator-result record, since a
|
|
||||||
* fresh-result hand-rolled iterator measured WORSE (252 ms) than the generator
|
|
||||||
* it replaced.
|
|
||||||
*
|
*
|
||||||
* The residual ~88 ms is object allocation — 6.5M instances across the six
|
* 1. The ~150-character synthesized `id` was built eagerly on every read — 6.5M
|
||||||
* scans — and it is irreducible while the read API returns objects at all. The
|
* concatenations for a field no in-pipeline consumer reads. Isolating it
|
||||||
* fix, if iteration ever needs true parity, is a field-wise callback
|
* showed 436 ms of the regression. It is now a lazy prototype getter on
|
||||||
* (`sourceId, targetId, type, confidence` as primitives) for the hot consumers,
|
* {@link StreamedRelationship}.
|
||||||
* all four of which read only those. That is an interface change across the
|
* 2. Generator and iterator-protocol overhead: {@link forEachRelationship} loops
|
||||||
* graph and its consumers, so it wants its own measurement and its own change.
|
* 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
|
* 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
|
* 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 {
|
forEachNode(fn: (node: GraphNode) => void): void {
|
||||||
this.real.forEachNode(fn);
|
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
|
/** Direct loop rather than delegating to {@link iterRelationships}: this is
|
||||||
* the form community detection uses (twice), and skipping the generator and
|
* the form community detection uses (twice), and skipping the generator and
|
||||||
* iterator protocol is measurably cheaper on a million-edge scan. */
|
* iterator protocol is measurably cheaper on a million-edge scan. */
|
||||||
|
|
|
||||||
|
|
@ -281,6 +281,11 @@ export class PdgEmitSink implements KnowledgeGraph {
|
||||||
forEachRelationship(fn: (rel: GraphRelationship) => void): void {
|
forEachRelationship(fn: (rel: GraphRelationship) => void): void {
|
||||||
this.real.forEachRelationship(fn);
|
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 {
|
getNode(id: string): GraphNode | undefined {
|
||||||
return this.real.getNode(id);
|
return this.real.getNode(id);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue