From d4056da3892aaa527172f0e647c9017b0fe0a710 Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Sat, 25 Jul 2026 18:53:47 +0000 Subject: [PATCH] perf(scope-resolution): pre-filter value bindings in the callable target index (#2693) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .github/workflows/ci-tests.yml | 11 +++ .../bench/callable-value-flow/baselines.json | 8 ++ .../bench/callable-value-flow/measure.mjs | Bin 0 -> 8826 bytes .../passes/callable-value-flow.ts | 74 +++++++++++++----- 4 files changed, 72 insertions(+), 21 deletions(-) create mode 100644 gitnexus/bench/callable-value-flow/baselines.json create mode 100644 gitnexus/bench/callable-value-flow/measure.mjs diff --git a/.github/workflows/ci-tests.yml b/.github/workflows/ci-tests.yml index dd6eed93c..782b51f8d 100644 --- a/.github/workflows/ci-tests.yml +++ b/.github/workflows/ci-tests.yml @@ -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 diff --git a/gitnexus/bench/callable-value-flow/baselines.json b/gitnexus/bench/callable-value-flow/baselines.json new file mode 100644 index 000000000..d42c566d9 --- /dev/null +++ b/gitnexus/bench/callable-value-flow/baselines.json @@ -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." +} diff --git a/gitnexus/bench/callable-value-flow/measure.mjs b/gitnexus/bench/callable-value-flow/measure.mjs new file mode 100644 index 0000000000000000000000000000000000000000..70d27b5f97c16be682383b078c06eb3598163ea4 GIT binary patch literal 8826 zcmbta>uwvz74C06#R=kq%ZOY`G90^99YB_4Td5=qmYw`i1aY}L6xZGhvokA-Q5f`B zAD}4E2kM*TN&1~LGs~qV8%|pUwmCa<=3KvXnGU}BW`n+=XLXt-{Yj-1nQ2wlSJPQ- zXp_=J6(%(c8ml6n(xj|tH0Hf8t7tZT8&y}z92SYX8TD{bF{wDpvS^&C{&keqsz1rf zIT_hVDet5p)w)*n_0G3{dDsncgipyYn)7`s0jX5eq%LBUmPJR?RbFk$2BdE5=@2Ug;W(^}V1?}7=ve)sm|-SM-7 zHw^&|1yiM>jC5I7vFhP(wu)6NBqbPC%gVGgl$GTSx{XV1m|j_cm%N6R!SIz5_0g=H z)ftq5kH7!@U)C$GV_u& z`~+%A4LF{amC-|Hf~Zd_i)LCSm$|;wIi#gQdr6q1U=nGAyQrMdn}gFchzi#oxz$k@ zu8XU21xsDA0G2w@C@_~oV_+=#?jShYd-L*OaCW?RbkrRU>9_50i*Qm!RgZpq7~as& zb(Aobc-J#I@@{^~uE@L!Q5y!N+4N4{d1|Jj^V2s6ee33MOwZ0r!Hu=pAaX#Wfdz#VsZQaTa4b)2gQrQ`i#bDd4S4Y*J=SBrRY%*o(?p z^fO$nsPnO^8u_rjtPnAMj@g;QAB7=3jdmV=OB0Y$)vN*u;+sg}j|Bw~>jchwn9%S4 z_^0*e!=&rEn?)(}f%p{^Kuna`wbDI~3|@x1Dis@ekd60{_SSBSA;Og)47`ML6&3cJ zR^kQT9u{^S;%LgjAovmAh35fQBIscnU8e{U&zdFOqk;v&2A}|tCXb33>M4|W=oLr~ zDb2-ch4WEp^a##xW>7XWB>+lB-|z*&NFuu5w?}vx8~{xR>nsaW4j>ofNSMFZBZR+2 zB9a@Jn7?*jRv&i3sVQgu3|wb4isli5$!-X`BN_tQ6V~h4qo&zn-l&J~zcC=E&DB6ZD@6v+998#SW_?YM;#XX@RXqqpTLlUlYn*Hwm{{;FGwVKbxth7^@42qH6m7qBskIM2xJ^K_D4rR^wKF~3;UthwqmIb+>Kdw@6GGM** zkugB(lee|Xf&byU8M~0aFDUGV8=S-qU0x<>C8_l(u`NRPGgwn|VC@>_%0wZw-0kkV z^UwCq4vr2_4lYmk-o7G7!+}xI5bsgPJE{)fYZM_M;3!C*z{#CQTb`&S5G*~}YL+-F zVP*S~-}B?%o8xz zfsgM~gf44iNIP&gFv^t^gD~Rli7!rET@=!5$y3;Z8s|xm zUY{IT1Z}DvyAyC|)+-%P2`IyHDCL^?69=i_p&;?dmvAEr)D6-cs=7H>5>o!y3d!;& z2vH*4#lVjhGT5l+@{k$hq=3|k;wu5#06yOWK>#t($75fnFk+PR!ZFCKc%?2-{n%1y ztWZ>Zf3W}d;JHg3Y9YA8T&yxSSY^fBldm~VduuQckB8fUDaO^Iux3jVX4z6iCPlg{ z7`-!x_QL)DrR+dgum7Jh-f<@3Vm?NZ#EA8PdUfhqF?EzAS%xIz8^2dlXT<0>>vn(}1C+u`X zI7QI?v=p?omn}t61Ya<91#>6N;c}%sFbT`Dg3MwDHF(+XQ05GJM>XusD858J{^as& zs0y2x!>v#0hbGgXWwfzw>Y`c4ha+rTk zjz4h72{IT+ws3u|M|uk~@R?rIcZ$38=;U<^Q~@t-UEHsERP;%vs&$pv7zjGjldlVT z?|!!0=B9HC@cft?m$X2kx(9#p1N!Hzcs&CkKxhvl!dXMK&VO zqW|e<1s#tp9Y0Z6t=NKo_1j7L#Uw7Qf8OUeEhfWE6<22ZiFmLw3G6|d*0J-{kzaYS z1;-W;jb#64Jnf55EFrYGnnS}|i_`#ru0#cY8JZY}$BV;cXy-XgV5vBqt!ptAYQ|9< z!RXgt`FQJ- z3-G%i#Kr7Ltm7$@s8mqjG2$|lHA<7AZbj+d1gPp8jdC?|e0ajPFeto|G=i3%q1dur zBl>Ji!%T`VJeA@>15#@&H7wVfc4T1ZR8Rp^Q&j~EpjeYy4lt0K^jb*)rqO0{eyBi1 zseH!$NsnIcqc+6-sa`D3g&Hxzf6V=*}lD=_b){X?qiqCod6)nGF>8lzw3(EvXB zI*hAYo+)udlygZ5)4(B^7$%KnVR#IZNWJB2MyRI(lOV$tn-t;;Y+%3PiX4-hszkM& zM=|{XGV=|JI_#RSlZ07S*h}4lj;T^o3}if-nr-X9@QvPTBhodD!H_vyB@)3pSP@ zvc`{5l@r=zH)F<8=sf(O_}IT8kTtqM;JS5ZYTu1qRmPt6NCtl-Vg?Ps$Hv)s!EH^ zU|Gy?Bj{bZ>6cZybq&j2;~e7!2Ao@e=4{=hl)0^_=BTJ_&$>%?ly zLI~q)nCt2P%P#Q{mst%lw4Mfc?!utjYzb&GWHu=*N51kt>jsx!{WFydxuYtjRb9YY^uaA1tkp z%7b<=BQH67-|1jip66o^1IaJIh1EG9r&2+*54S(8mjP|w+96=yJVYrM;Fx8D*8_;z zY4Dfm$QF+v zLZVrgU%brQz{I80Cqp`sR~~@2tcuD*Q@%)VR1!SwifKIG%_CBC%75_^eSETI^L0kT z)|cJ}Y`?*k?N_cK4Dk+e9i~N`)d@hY;~o`&H%;^-uZCH974QQ^i1%Gcluj0b)eE8x z)3F}y@Q{~$nHlm6K)?vypp>8JLXkoUMVeLtYHm#3lNX4;vqcdHYXf4;fon7AKLIm- zjys9c3@ZAU;BfV$mtT`Kfw^D%rV4r91)4|AXS;_3ri03Yun!N3_W%pa`7o<5hc8eeHu#GFXdAjrQ z^3&W|3;7eP+PX22|J2-cKf`ynxjQj^^6He3|8jq4X&Y~{T|b1}S`QuRtb)<`x(nF< yYle%q3wp74c=S`j5AYbiIUjG5KQDK>qT!DsY+IDhIOUn{eEy!(y|Xhe9{vmKh<0TF literal 0 HcmV?d00001 diff --git a/gitnexus/src/core/ingestion/scope-resolution/passes/callable-value-flow.ts b/gitnexus/src/core/ingestion/scope-resolution/passes/callable-value-flow.ts index f771206fc..a7c52e454 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/passes/callable-value-flow.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/passes/callable-value-flow.ts @@ -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 { const out = new Map(); - 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 { - const out = new Map(); +interface GraphCallableIndexes { + /** Callable graph nodes by `file\0label\0line\0name` — the definition anchor. */ + readonly byAnchor: ReadonlyMap; + /** + * `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; +} + +function buildGraphCallableIndexes(graph: KnowledgeGraph): GraphCallableIndexes { + const byAnchor = new Map(); + const callableNameKeys = new Set(); 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 {