perf(gfql): kill per-call O(E) terms in seeded Cypher/chain queries (#1755 slice 1)

[lmeyerov]

What

Extracts the foundational slice of the seeded-Cypher abstraction-tax fix (#1755) as ONE clean, review-gated PR off master — the [EXPERIMENTAL] prototype #1738's ee510f78, isolated from the still-in-design "lean gather-combine" slices (those never land as-is).

Eliminates the two per-call O(E) terms that made g.gfql("MATCH…") seeded point/short-query latency GROW with edge count (LDBC SF0.1→SF1 grew 4–5× on 11× data while Kuzu stays flat), even with the #1658 CSR index resident:

1. Typed keep-mask cache (gfql/index/traverse.py) — the feat(gfql): physical adjacency indexes for O(degree) seeded traversal #1658 index path rebuilt its simple-equality edge keep-mask via a full column compare (eq_str, ~2.8ms/1M rows) on every seeded hop. Now cached per (id(frame), engine, scalar edge_match).
2. Augmented-frame cache (lazy/engine/polars/chain.py) — the polars chain rebuilt with_row_index per call: an O(E) copy whose fresh object identity also defeated every downstream id-keyed cache (including Replace NaNs with nulls since node cannot parse JSON with NaNs #1). Now one augmentation per resident frame; stable identity restored (which is what lets cache Replace NaNs with nulls since node cannot parse JSON with NaNs #1 actually hit).
3. Typed-edge slice cache (lazy/engine/polars/row_pipeline.py) — the bindings-row path re-filtered the full edge type column O(E) per call; now a cached eager slice per (id(frame), scalar edge_match).

All three use the merged #1735 clean-cache pattern (weakref.finalize on the source frame → a recycled id() can never serve a stale entry; a rebound/new frame re-computes). Non-scalar / predicate / membership / null edge-match forms decline to the general per-call path (never mis-cache).

Why this is safe

• Not a data-structure/kernel gap — native g.hop() already hits sub-ms via feat(gfql): physical adjacency indexes for O(degree) seeded traversal #1658 (0.20ms floor). This is purely the compile+chain-executor abstraction tax (plan criterion 5(c)).
• Caches are pure functions of an immutable resident frame; polars frames aren't mutated in place, matching the perf(gfql): identity-stable _pl_nan_to_null + clean-cache (kills O(E) per-hop NaN re-scan on polars) #1735 precedent already on master.

Tests

New TestSeededChainTaxCaches (in test_engine_polars_chain.py, already in the polars lane): cache-identity + weakref eviction-on-GC + every decline branch for each of the three caches, plus an end-to-end run-twice parity guard (repeated seeded typed query is stable + native + matches the pandas oracle). Local: 6/6 new + 1009 chain-parity + 82 non-GPU index-parity pass; mypy clean on all 3 files.

Pending before merge

• Independent 4-engine A/B on dgx-spark (byte-identical results + the seeded warm-median tax-reduction number) — re-measuring the same wall as perf(gfql): Cypher seeded-lookup abstraction tax -- 10-800x over native g.hop() (chain-executor combine_steps, not index-miss) #1755's root-cause probe.
• .agents/skills/review pass.

Extracted from #1738 (ee510f78); base stack #1734/#1735/#1736/#1737 all merged. Fixes part of #1755.

🤖 Generated with Claude Code

[lmeyerov]

A/B finding (honest, pre-merge): slice 1's caches don't engage on a naive seeded query

Ran a same-container baseline(master)-vs-this-branch A/B (Message→Person HAS_CREATOR, seeded 1-hop typed cypher, warm median, 4 engines, at 200k and 4M edges). Results were parity-identical (result_rows=1 everywhere; no corruption) but showed no speedup (4M: pandas 496→518, polars 111→114, cudf 79→83, polars-gpu 95→95 ms).

Root-caused locally (not inferred): after repeated polars gfql() calls, all three cache dicts are size 0 and gfql_explain reports used_index=False, resident=[]. The keep-mask cache lives inside the #1658 index path (no resident index here → never runs); the query also doesn't route through the polars augmentation branch or binding_rows_polars. So my probe measured a path where these caches are inert — it did not reproduce #1755's conditions (its root-cause probe explicitly makes the #1658 index resident).

Implication: this PR is a correctness-verified, typed, green refactor that removes O(E) terms on the index path, but the headline points-flip is not yet demonstrated. Before merge I'll redo the A/B on the resident-#1658-index LDBC IS1/IS5 harness (where #1738 measured the 67–79% cut), confirm cache engagement, and determine whether slice 1 alone suffices or slice 2 (e4cc21b9, combine-deferral) must stack on top. Not to be merged as "the tax fix" until that number is real.

[lmeyerov]

DEFINITIVE A/B via scratch_tax_probe.py (the exact #1755 root-cause probe): slices 1+2 give ZERO tax reduction

Ran the original root-cause probe (same synthetic Message→Person graph, resident #1658 index confirmed) baseline(master) vs this branch + slice 2:

metric	baseline	fixed (1+2)
IS5 bare_hop (native ceiling)	0.131ms	0.133ms
IS5 native_chain_notype	13.0	15.3
IS5 native_chain_typed	21.1	23.0
IS5 cypher	30.1	31.6
IS5 tax (cypher/bare_hop)	229×	238×

Fixed is identical-to-slightly-slower. The decomposition shows the tax is bare_hop 0.13ms → +13ms chain machinery (combine_steps two-pass re-merge) → +8ms full-column type filters → +9ms cypher projection — and slices 1/2 touch none of those. #1738's "slices 1-2 cut points 67-79%" does not reproduce on its own probe.

Disposition: this PR is confirmed correctness-safe perf-hygiene (removes O(E) eq_str/with_row_index terms on the index path) but is not the seeded-points lever. Recovering the tax needs a combine_steps lean-path redesign (clean-authored, not a #1738 cherry-pick). Leaving #1757 open as optional hygiene — not to be merged as 'the tax fix'.

[lmeyerov]

Restacked and measured (locked-step 5 report)

Restacked onto master cleanly. SF1 polars, both reps reported, perf lock held, rows_returned unchanged throughout.

query	master	r1	r2	change	Kuzu
seed-lookup (IS1)	103.6	77.4	74.5	1.34–1.39x	1.35
message-creator (IS5)	92.0	70.3	72.3	1.27–1.31x	1.68
expand-order-limit (IS2)	107.8	89.3	90.9	1.19–1.21x	21.13
message-content (IS4)	4.7	4.1	4.2	1.12–1.15x	0.71
message-replies	14.7	15.1	14.8	0.97–0.99x	—
new-topics	203.0	194.7	190.8	1.04–1.06x	—
tag-cooccurrence	42.4	43.9	41.7	0.97–1.02x	—
recent-replies	52.5	54.0	56.6	0.93–0.97x	—

The four queries that move are exactly the four cells we lose to Kuzu. The four that don't move are exactly the ones we win. Non-targets sit at 0.93–1.06x while targets move 1.12–1.39x consistently across both reps — so this is signal, not host noise, and it confirms the PR is aimed at the right layer (per-call overhead, #1755).

It does not flip any cell: 74.5 ms against Kuzu's 1.35 ms is a 55x loss, down from 77x.

Blocked on three cache properties, not rejected

The caches are three module-level dicts holding data-plane objects (an O(E) boolean mask, a full with_row_index edge frame, a full eager typed slice), with no bound, no lock, keyed by id(). The sharpest issue is correctness, not memory: "a recycled id() can never serve a stale entry" holds under prompt refcount collection, but a frame in a reference cycle is collected later, leaving a window where a new frame reuses a freed id() and is served another frame's data.

Empirical follow-up (polars 1.35.2, python 3.13.12) says every objection has a cheap remedy:

property	finding	remedy
pl.DataFrame hashable	False	(rules out WeakKeyDictionary — that part of the original reasoning holds)
weakref-able	True	—
accepts attributes	True	per-frame UUID token instead of id() — a token cannot be recycled, so the stale-entry window closes entirely
GIL released during collect	yes (2.95x on 4 threads vs 4x serial)	real concurrency, so a lock is warranted
bound	none	cap entries, or bind lifetime to the existing _gfql_index_registry rather than a module global

A check-then-act race probe (8 threads, 1 key) produced 1 build, but that is timing under the GIL, not a guarantee — and the threading result above shows real parallelism is available. Weak evidence; the structural argument stands.

Measured memory, stated precisely

25 live indexed frames took RSS 443 → 998 MB (22.2 MB/frame), but retained cache entries read 0 at inspection — so that growth is the adjacency indexes, not this PR's caches. The unbounded-growth concern is therefore structural (no cap, no lock, id() keys), not demonstrated. Worth saying plainly so the revision is argued against the real risk.

Suggested next revision: UUID token key + explicit lock + bounded size (or registry-scoped lifetime), then re-measure. The speedup above is real and on-target; only the cache mechanics are in the way.

[lmeyerov]

Evidence from today that bears directly on this PR

This PR's goal is killing per-call O(E) terms in seeded Cypher/chain queries, using caches. Today I found and fixed one such term outright, with no cache and no global state — worth weighing before this lands.

#1782 (perf/gfql-indexed-edge-match-lazy): index_seeded_hop built the edge_match mask over all E edges and then read it only at the handful of CSR-matched positions. That is a per-call O(E) term inside an O(degree) traversal. Evaluating the predicate on the gathered candidate rows removes it structurally — no memoization, nothing to bound, no identity-safety question, no cross-query lifetime.

Measured on the real LDBC SF1 polars lane, position-balanced ABBA/BAAB with rows validated: IS1 −29.1%, IS5 −25.4%, IS2 −19.0%, IS4 −10.5%, IS7 −2.3%. 4 faster, 0 slower.

Where the remaining IS1/IS5 cost actually is (measured today, correctly-configured polars: polars-engine indexes + engine='polars'):

• seeded point lookup: 0.92 ms — already competitive with Kuzu
• full seeded pattern: ~135 ms, of which collect_all is ~110 ms across 2 collects
• the CSR index does engage on the polars hop (1.40× vs index_policy='off') — 132 ms remains with it engaged

So the residue is the polars executor building and collecting a combine plan over the full 3.18M-node / 14M-edge frames regardless of how narrow the traversal already is. That is not a repeated-work problem a cache would fix — it is one collect over graph-sized inputs per query. Caching it would memoize a graph-sized intermediate, which is the memory-blowup risk already raised on this PR.

Suggestion: re-measure this PR on top of #1782 before deciding. If the O(E) term it was chasing is the one #1782 already removed, this can close as a quarry with the analysis, and the remaining work is making the polars combine proportional to the traversal result — a planner change, not a cache.

I have not measured this PR myself (speedup / peak_rss / repeated-query growth loop all still outstanding), so this is input to that decision, not a verdict on it.

[lmeyerov]

Closing with the analysis, per the "restack on the current stack and measure; no movement →
close" test. The result is more clear-cut than a measurement: the code this PR optimizes has
largely been deleted by the stack underneath it.

I restacked it on top of #1784 (perf/gfql-polars-semi-no-dedup). The cherry-pick conflicts,
and the conflict itself is the finding.

Term 1 — typed keep-mask cache: the target function no longer exists

This PR adds _EDGE_KEEP_MASK_CACHE keyed on (id(edge frame), engine, scalar items) to
memoize _build_edge_keep_mask, described here as "a full O(E) column compare on EVERY seeded
hop (~2.8 ms per call at 1M rows)".

#1782 did not memoize that compare — it removed it. The seeded index path now evaluates the
predicate on the gathered candidate rows (_EdgeMatchRowFilter.mask_for), so there is no
full-column mask to cache. Grep for _build_edge_keep_mask across the stack returns 0 hits;
the function is gone. Caching it is not a smaller win, it is a no-op against a deleted code
path.

(#1782 does retain a whole-column mask, but only as a cost guard for fixed-point walks that
gather past E/8 — it is built at most once per query and never on the seeded path this PR
targets.)

Term 2 — augmented-frame cache: real, but now ~1 ms

The with_row_index term is genuinely still there, but #1783 removed one of the two passes by
reusing the synthetic edge id as the stable row order. Profiled on the current stack at
N=2M / E=2M: with_row_index is 1 call per query, 1.0 ms of a 21.84 ms query — 4.6%. It was
~16 ms/query (2 × 8 ms) before the stack.

Why that is not worth the price

The remaining 4.6% would be bought with a module-level mutable data-plane cache keyed on
id(frame) — the property that has been treated as disqualifying until it is bounded in size,
locked for concurrent use, and keyed on something stronger than id() (CPython recycles ids;
the PR's own mitigation is a weakref.finalize on the source frame). That is a substantial
correctness and concurrency surface to take on for 4.6% of a query, when the O(E) term it was
designed to remove is already gone.

What survives from it

The insight was right and it did land, just not as a cache. The two things this PR identified —
a per-call O(E) edge-predicate compare, and a per-call O(E) frame augmentation whose fresh
object identity defeated downstream caches — were both fixed by deleting the work rather
than memoizing it (#1782 and #1783 respectively). #1784 then removed the O(N) semi-join key
dedup that dominated what remained. Net on the real LDBC SF1 polars lane, position-balanced with
rows validated: IS1 −50.5%, IS5 −52.3%, IS2 −42.6%, IS7 −17.3%, 4 faster / 0 slower.

Reopen if a future profile shows the augmentation term growing back into something worth a
cache — but it should then be fixed by removing the duplication, as #1783 did, before reaching
for global mutable state.