mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-09 03:17:54 +00:00
perf(scope-resolution): pre-filter value bindings in the callable target index (#2693)
Widening the `buildGraphTargetIndex` gate to consider VALUE bindings put the hot loop on a much larger def population — value bindings outnumber callables in real source — and the naive version paid full price per binding. Measured on a synthetic 800-file corpus (8 value bindings per file, 1 of them a closure binding), the widening cost 2.50-2.82x the pre-#2693 callable-only build. Two wastes, both provable rather than guessed: 1. `definitionAnchorKey` ran for every def, including value bindings. The anchor index is keyed by callable LABEL and the key is built from `def.type`, so a value def can never hit it — and the key costs a regex per def. 2. Every value binding paid the whole `resolveDefGraphId` key chain only to be rejected. It need not: every qualified key that function tries embeds `def.type`, so for a VALUE def those can only ever reach a value-labelled node. Its one route to a callable is the label-agnostic `simpleKey(filePath, simpleName)` fallback, which by construction requires a callable node with the SAME file and simple name. So a value binding with no such node cannot resolve to a callable, and one Set lookup decides it. That set is derived in the graph walk the anchor index already performs, so it costs no extra pass. large_ms 7.79-8.37 -> 4.90-5.02 (1.61x faster) widening_overhead 2.50-2.82 -> 1.45-1.50 The resolved target-set fingerprint is byte-identical across both, which is the point: this is a cost change, not a behaviour change. Adds bench/callable-value-flow/ (fingerprint + scaling + widening-overhead gates) and wires it into ci-tests.yml beside the other build-free benches. The overhead budget of 1.9 sits between the measured with-filter and without-filter bands, so it cannot be met if the pre-filter is removed. Timings use the MIN of 15 warmed reps, not the median: the same build reported 1.65 idle and 2.03 under load, and a median-based gate would have to be loosened past the point of detecting the regression it exists to catch. `buildGraphTargetIndex` is exported for the bench; it is pure and not part of the pass's public contract.
This commit is contained in:
parent
5fee2016d2
commit
d4056da389
4 changed files with 72 additions and 21 deletions
11
.github/workflows/ci-tests.yml
vendored
11
.github/workflows/ci-tests.yml
vendored
|
|
@ -488,6 +488,17 @@ jobs:
|
|||
run: node --import tsx bench/scope-capture/measure.mjs --check
|
||||
working-directory: gitnexus
|
||||
|
||||
- name: Callable-value-flow target-index guards (#2693)
|
||||
# Build-free: asserts buildGraphTargetIndex resolves an unchanged target
|
||||
# set (fingerprint), stays linear in def count, and that the #2693
|
||||
# widened gate — which now considers VALUE bindings, a population that
|
||||
# outnumbers callables in real source — stays within its measured
|
||||
# overhead of the pre-#2693 callable-only cost. Catches a regression in
|
||||
# the value-binding pre-filter, without which every value binding pays
|
||||
# the full resolveDefGraphId key chain just to be rejected.
|
||||
run: node --import tsx bench/callable-value-flow/measure.mjs --check
|
||||
working-directory: gitnexus
|
||||
|
||||
- name: CFG construction time / disk / memory guards (#2081 M1)
|
||||
# Build-free: asserts collectFunctionCfgs output is unchanged
|
||||
# (fingerprint) and that wall-time, cfgSideChannel disk bytes, AND
|
||||
|
|
|
|||
8
gitnexus/bench/callable-value-flow/baselines.json
Normal file
8
gitnexus/bench/callable-value-flow/baselines.json
Normal file
|
|
@ -0,0 +1,8 @@
|
|||
{
|
||||
"_comment": "Baselines for bench/callable-value-flow/measure.mjs --check (#2693). `fingerprint` is an order-independent sha256 over every (defNodeId -> graphId) pair buildGraphTargetIndex resolves on the synthetic corpus; it is a CORRECTNESS gate, so drift means the callable-value target set moved and must be explained, never re-baselined to make CI green. The two budgets are timing gates and carry deliberate headroom for shared CI runners.",
|
||||
"fingerprint": "70bebf6a26ff6fc9f231a0933678274b44c4883ddab5e719a61a9c77d6223e51",
|
||||
"scaling_budget": 1.6,
|
||||
"_scaling_note": "(t_large/t_small)/(800/250). ~1.0 is linear; measured 1.16-1.31. The index build is one pass over defs plus map lookups, so a jump toward 3.x means someone made the per-def work depend on corpus size (e.g. a scan inside the loop).",
|
||||
"widening_overhead_budget": 1.9,
|
||||
"_widening_overhead_note": "large_ms / callable_only_ms — how much more the #2693 widened gate costs than the pre-#2693 callable-only population on the SAME corpus. Measured 1.45-1.50 WITH the value-binding pre-filter and 2.50-2.82 WITHOUT it, so this budget sits between the two bands: it cannot be met if the pre-filter is removed or defeated. Value bindings outnumber callables in real source, and without the filter every one of them pays the full resolveDefGraphId key chain only to be rejected."
|
||||
}
|
||||
BIN
gitnexus/bench/callable-value-flow/measure.mjs
Normal file
BIN
gitnexus/bench/callable-value-flow/measure.mjs
Normal file
Binary file not shown.
|
|
@ -20,7 +20,11 @@ import type { ScopeResolutionIndexes } from '../../model/scope-resolution-indexe
|
|||
import type { GraphNodeLookup } from '../graph-bridge/node-lookup.js';
|
||||
import type { CalleeIdAccumulator } from '../graph-bridge/callee-id-sink.js';
|
||||
import { tryEmitEdgeWithExplicitTargetId } from '../graph-bridge/edges.js';
|
||||
import { resolveCallerGraphId, resolveDefGraphId } from '../graph-bridge/ids.js';
|
||||
import {
|
||||
resolveCallerGraphId,
|
||||
resolveDefGraphId,
|
||||
simpleQualifiedName,
|
||||
} from '../graph-bridge/ids.js';
|
||||
import { resolveInheritanceBaseInScope } from '../scope/walkers.js';
|
||||
import { narrowOverloadCandidates } from './overload-narrowing.js';
|
||||
|
||||
|
|
@ -739,24 +743,46 @@ export function emitCallableValueFlow(input: EmitCallableValueFlowInput): Callab
|
|||
return { emitted, resolvedInvokes, ambiguousInvokes, unmatchedInvokes, iterations };
|
||||
}
|
||||
|
||||
function buildGraphTargetIndex(
|
||||
/**
|
||||
* Pure — exported for `bench/callable-value-flow/measure.mjs`, which guards its
|
||||
* scaling ratio and result fingerprint. Not part of the pass's public contract.
|
||||
*/
|
||||
export function buildGraphTargetIndex(
|
||||
scopes: ScopeResolutionIndexes,
|
||||
nodeLookup: GraphNodeLookup,
|
||||
providerTarget: ((def: SymbolDefinition) => boolean) | undefined,
|
||||
graph: KnowledgeGraph,
|
||||
): ReadonlyMap<string, Target> {
|
||||
const out = new Map<string, Target>();
|
||||
const byAnchor = buildGraphCallableAnchorIndex(graph);
|
||||
const { byAnchor, callableNameKeys } = buildGraphCallableIndexes(graph);
|
||||
for (const def of scopes.defs.byId.values()) {
|
||||
const callableDef = isCallable(def) || providerTarget?.(def) === true;
|
||||
// #2693: a closure bound to a name (`val f = { }`) is a callable the def
|
||||
// type cannot see. The scope-resolution layer declares such a binding with
|
||||
// its VALUE label (Kotlin/Swift `Property`, Dart `Variable`) while #2687
|
||||
// makes the graph emit a single `Function` node for it. Only the graph
|
||||
// knows, so value bindings are resolved first and admitted below on the
|
||||
// label of the node they actually reach.
|
||||
if (!callableDef && !VALUE_BINDING_DEF_TYPES.has(def.type)) continue;
|
||||
const anchorKey = definitionAnchorKey(def);
|
||||
// knows, so value bindings are resolved below on the label of the node they
|
||||
// actually reach.
|
||||
if (!callableDef) {
|
||||
if (!VALUE_BINDING_DEF_TYPES.has(def.type)) continue;
|
||||
// Exact pre-filter, not a heuristic. Every qualified key
|
||||
// `resolveDefGraphId` tries embeds `def.type`, so for a VALUE def those
|
||||
// can only ever hit a value-labelled node; its one route to a callable is
|
||||
// the label-agnostic `simpleKey(filePath, simpleName)` fallback, which by
|
||||
// construction requires a callable node with the SAME file and simple
|
||||
// name. So a value binding with no such node cannot possibly resolve to a
|
||||
// callable, and skipping it here is equivalent to running the full
|
||||
// resolve and rejecting the result — at one Set lookup instead of the
|
||||
// whole key chain. Value bindings outnumber callables in real source, so
|
||||
// this is the difference between paying resolve cost per binding and
|
||||
// paying it per binding that can actually match.
|
||||
const simple = simpleQualifiedName(def);
|
||||
if (simple === undefined || !callableNameKeys.has(`${def.filePath}\0${simple}`)) continue;
|
||||
}
|
||||
// Only callable defs can match the anchor index — it is keyed by callable
|
||||
// LABEL, and `definitionAnchorKey` builds its key from `def.type`. Running
|
||||
// it for a value def costs a regex per def and can never hit.
|
||||
const anchorKey = callableDef ? definitionAnchorKey(def) : undefined;
|
||||
const anchored = anchorKey === undefined ? undefined : byAnchor.get(anchorKey);
|
||||
const id =
|
||||
anchored?.length === 1 ? anchored[0] : resolveDefGraphId(def.filePath, def, nodeLookup);
|
||||
|
|
@ -905,30 +931,36 @@ function declarationSignatureCompatible(
|
|||
return typeof declarationConst !== 'boolean' || declarationConst === definitionConst;
|
||||
}
|
||||
|
||||
function buildGraphCallableAnchorIndex(
|
||||
graph: KnowledgeGraph,
|
||||
): ReadonlyMap<string, readonly string[]> {
|
||||
const out = new Map<string, string[]>();
|
||||
interface GraphCallableIndexes {
|
||||
/** Callable graph nodes by `file\0label\0line\0name` — the definition anchor. */
|
||||
readonly byAnchor: ReadonlyMap<string, readonly string[]>;
|
||||
/**
|
||||
* `file\0name` for every callable graph node. The value-binding pre-filter in
|
||||
* `buildGraphTargetIndex` needs only existence, and deriving it here keeps the
|
||||
* graph to ONE walk rather than a second pass for the same nodes.
|
||||
*/
|
||||
readonly callableNameKeys: ReadonlySet<string>;
|
||||
}
|
||||
|
||||
function buildGraphCallableIndexes(graph: KnowledgeGraph): GraphCallableIndexes {
|
||||
const byAnchor = new Map<string, string[]>();
|
||||
const callableNameKeys = new Set<string>();
|
||||
for (const node of graph.iterNodes()) {
|
||||
if (node.label !== 'Function' && node.label !== 'Method' && node.label !== 'Constructor') {
|
||||
continue;
|
||||
}
|
||||
const filePath = node.properties.filePath;
|
||||
const name = node.properties.name;
|
||||
if (typeof filePath !== 'string' || typeof name !== 'string') continue;
|
||||
callableNameKeys.add(`${filePath}\0${name}`);
|
||||
const zeroBasedLine = node.properties.startLine;
|
||||
if (
|
||||
typeof filePath !== 'string' ||
|
||||
typeof name !== 'string' ||
|
||||
typeof zeroBasedLine !== 'number'
|
||||
) {
|
||||
continue;
|
||||
}
|
||||
if (typeof zeroBasedLine !== 'number') continue;
|
||||
const key = `${filePath}\0${node.label}\0${zeroBasedLine + 1}\0${name}`;
|
||||
const bucket = out.get(key);
|
||||
if (bucket === undefined) out.set(key, [node.id]);
|
||||
const bucket = byAnchor.get(key);
|
||||
if (bucket === undefined) byAnchor.set(key, [node.id]);
|
||||
else bucket.push(node.id);
|
||||
}
|
||||
return out;
|
||||
return { byAnchor, callableNameKeys };
|
||||
}
|
||||
|
||||
function definitionAnchorKey(def: SymbolDefinition): string | undefined {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue