perf(gfql): cardinality-aware lean combine for seeded chains (#1755)#1758
Merged
Conversation
Contributor
Author
review-skill pass (findings mode) — no correctness blockersRan the repo review skill. No correctness defects survived verification — the Low-severity, non-blocking (deferred):
Verdict: landable incremental parity-safe perf fix. Leaving open for human review — not self-merging. |
The two-pass chain executor (_chain_impl backward pass + combine_steps) fires full-node-frame safe_merge/pandas.merge calls to reconcile wavefront boundaries even when a seeded result is a single row (root cause #1755), so a seeded 1-hop pays a whole-frame join. Three hot merges scan the full node/edge frame regardless of frontier size: (a) backward pass: safe_merge(g._nodes, wavefront[[id]], how='inner') (2x/op) (b) combine tail: safe_merge(out_df, g._nodes/_edges, how='left') When the small side is >=4x smaller than the full frame, the byte-identical result is obtained by an isin membership filter (no join-indexer / block- consolidation machinery): inner-merge of a unique full frame against a unique single-key wavefront == isin in full-frame order; a how='left' merge discards unmatched right rows anyway, so pre-shrinking the right side is byte-identical. Gated narrowly on cardinality + right-side column-set + id uniqueness so the bulk/multi-source path is untouched; pandas-only (cuDF's GPU hash-join is already sub-ms, polars takes its own engine path). GFQL_LEAN_COMBINE=0 restores the legacy merges (used by the differential parity harness). Parity: byte-identical node+edge frames lean-on vs lean-off across seeded node lookup, seeded 1-hop expand (creator / both endpoints), and the native typed chain (test_chain_lean_combine.py, incl. a non-vacuity guard that the lean path actually engages). mypy clean. Measured dgx pandas (50k/200k, resident #1658): IS5 cypher ~29->18ms, native typed chain ~21->10ms, parity-exact. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL
#1755) Review follow-up: - _lean_intersect_full now declines when the (small) key_frame carries a null id. isin() matches nulls but merge's null-key semantics are version-dependent; checking only the small side keeps the isin==inner-merge equivalence version-independent at O(small) cost (a null key_frame is enough to force the legacy merge; nulls in the full frame are dropped identically by both paths). - non-vacuity engagement test now also asserts the combine-tail _lean_prefilter_right actually shrinks the right frame on a seeded chain, so neither lean path is silently inert (parity tests stay meaningful). 11/11 pass, mypy clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL
lmeyerov
force-pushed
the
perf/gfql-lean-combine-1755
branch
from
July 21, 2026 19:08
dd8d349 to
267676f
Compare
#1755) Review feedback: the lean-combine helpers (_lean_combine_enabled, _is_unique_ids, _lean_engine_ok, _lean_intersect_full, _lean_prefilter_right) are reusable, engine-agnostic DataFrameT helpers — move them out of chain.py into a dedicated graphistry/compute/chain_lean_combine.py (mirrors the gfql_fast_paths.py #1731 convention: one-directional import, no back-edge, imports only leaf modules). chain.py imports them; call sites + monkeypatch-based engagement tests unchanged. 11 lean-combine tests pass; mypy clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL
This was referenced Jul 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Cardinality-aware lean combine for seeded chains (#1755, "abstraction tax" lever 1).
The two-pass chain executor (
_chain_implbackward pass +combine_steps) reconciles wavefront boundaries with full-node-framesafe_merge/pandas.mergecalls even when the seeded result is a single row, so a seeded 1-hop pays a whole-frame join. Three hot merges scan the full node/edge frame regardless of frontier size:safe_merge(g._nodes, wavefront[[id]], how='inner')(2×/op)safe_merge(out_df, g._nodes/_edges, how='left')When the small side is ≥
4×smaller than the full frame, the byte-identical result is obtained by anisinmembership filter (no join-indexer / block-consolidation machinery):isinin full-frame order;how='left'merge discards unmatched right rows anyway, so pre-shrinking the right side is byte-identical.Gating (narrow by design)
_lean_engine_ok): cuDF's GPU hash-join is already sub-ms so anisinpre-pass is net-negative there; polars takes its own engine path.[id]and id-uniqueness, so the bulk / multi-source / fan-out paths are untouched.GFQL_LEAN_COMBINE=0restores the legacy full-frame merges (used by the differential parity harness).Evidence
Parity (local, pandas):
test_chain_lean_combine.py— 11 tests: gate units + lean-on vs lean-off byte-identical node+edge frames across seeded node lookup, seeded 1-hop expand (creator / both endpoints), native typed chain, plus a non-vacuity guard asserting the lean path actually engages. mypy clean; typedSeriesT/DataFrameT.A/B (dgx-spark,
26.02-gfql-polars, 50k persons / 200k messages, seeded 1-hop, warm-median warmup3/repeat7):cuDF is untouched (empirically confirms the pandas-only gate → no cross-engine regression).
Note on magnitude: the prototype measured ~1.6× with the resident #1658 index (#1676, not yet on
master) making the hop cheap so combine dominated total time. Onmasteras-is the seeded hop still scans edges, so combine is a smaller fraction and the win is 1.15–1.25×; it grows once the resident index lands. This PR is a correctness-safe, parity-exact increment — not the full ≤1ms fix (which additionally needs the typed-filter lever + a seeded single-hop fast-path that bypasses the two-pass combine entirely).🤖 Generated with Claude Code