Skip to content

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

Open
lmeyerov wants to merge 2 commits into
masterfrom
perf/gfql-seeded-chain-tax-1755
Open

perf(gfql): kill per-call O(E) terms in seeded Cypher/chain queries (#1755 slice 1)#1757
lmeyerov wants to merge 2 commits into
masterfrom
perf/gfql-seeded-chain-tax-1755

Conversation

@lmeyerov

Copy link
Copy Markdown
Contributor

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

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

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

🤖 Generated with Claude Code

lmeyerov and others added 2 commits July 21, 2026 01:56
…gmentation caches)

IS1 profiling found two linear-with-E costs per seeded Cypher call on a
resident graph: the index path's simple-equality keep-mask rebuilt via a full
column compare (eq_str), and the polars chain's per-call with_row_index O(E)
copy whose fresh identity also defeated downstream id-keyed caches. Both now
recycle-safe caches keyed on the resident frame (weakref.finalize eviction,
the _pl_nan_to_null pattern). Profiled: both terms eliminated; 274 tests pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL
… typed-edge slice)

White-box identity/eviction/decline-branch tests for the three recycle-safe
per-frame caches in #1755, plus an end-to-end run-twice parity guard proving the
caches never serve a stale/corrupt frame across repeated seeded queries.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL
@lmeyerov

Copy link
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown
Contributor Author

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'.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant