perf(gfql): fuse the polars single-hop grouped aggregate into one lazy plan - #1823
Conversation
…y plan
`_execute_single_hop_grouped_aggregate_fast_path` serves the cypher shape
MATCH (a {..})-[{..}]->(b {..}) [WHERE ..]
RETURN <alias>.<prop> AS k, <agg> AS v ORDER BY .. [LIMIT n]
which is three of the nine matched graph-benchmark cells (q1, q3, q4). Its
polars branch chained ~7 EAGER ops -- two semi-joins, two property joins, a
group_by, a sort and a head, each its own lazy().collect(_eager=True) -- so
every intermediate materialized, the select([src, dst]) projection could not
be pushed into the semi-joins, and the head() could not reach into the plan
at all. 74-96% of each of those queries' wall time sat inside those collects.
The same op sequence is now built as ONE lazy plan collected once. The algebra
is character-identical to the eager twin (same .unique() id frames, same
semi-joins, same UN-deduplicated property lookups, same
group_by(maintain_order=True).agg(..), same per-key nulls_last sort, same
head), so the value is identical, row ORDER included.
STRICTLY ADDITIVE: the eager code is untouched and is the fallback on every
decline, so the blast radius is a decline away from zero.
DECLINES (never a different answer, only a forgone speedup):
* a non-eager-polars input frame;
* a property column missing from its alias' node frame -- the eager twin
discovers this MID-CHAIN and declines the WHOLE fast path, so the guard
is HOISTED ahead of plan construction; otherwise the fused lane would
answer a query that is supposed to raise;
* source and destination bound to the same edge column;
* a projected column colliding with an endpoint column or the lookup key;
* an untranslatable aggregate;
* a result row order that ORDER BY does not fully determine.
That last one is the correctness crux and it is MEASURED, not assumed:
without every group key in the sort, the twin's order falls back to
maintain_order=True group first-appearance order over an EAGER join output,
which a lazy plan is free to change. An ungated variant of the same plan
diverged from the twin in 47 of 64 comparisons (4 graph sizes x 4 seeds x 4
order-undetermined shapes), and under LIMIT the divergence is a different ROW
SET, not merely a different row order.
MEASURED, dgx-spark, matched q1-q9 lane, canonical query text (gb_queries.py,
md5 6e7ae268a5a41742587fcb87854b6e27 @ pyg-bench 7e5ce3b), one perf lock per
experiment, master tree vs PR tree position-balanced M P P M P M M P, per-slot
medians, replicated in a second independent run:
20k q1 13.31 -> 8.96 (-32.7%) q3 8.54 -> 5.53 (-35.3%)
q4 6.99 -> 5.05 (-27.8%) all non-overlapping in both runs
100k q1 42.59 -> 31.45 (-26.2%) q4 12.19 -> 10.43 (-14.4%)
q3 -9.1%..-13.0%
vs SAME-SESSION embedded Kuzu at 20k: q1 widens 1.12x -> 1.66x; q3 moves from
a 1.37x LOSS to a TIE (slot ranges overlap -- a tie, NOT a win); q4 narrows
from 2.10x to 1.51x and REMAINS A LOSS. q5 and q8 are ties at both scales in
both runs and q8 stays a win -- this lane is not called for them.
Value identity: one canonical row set per query across every slot of every
run. Differential over 19 shapes x 10 graphs, compared row-order AND
column-order sensitively against both the eager code and the pandas oracle:
zero divergences from the eager code.
DISCLOSED, pre-existing, deliberately NOT changed here: the polars property
lookup is not deduplicated by node id while the pandas one is, so a node table
carrying the same id twice multiplies matched rows on polars only. The fused
lane reproduces the eager polars answer exactly and a test pins both sides.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015YsqAZQLbqjSDrYSFz2GoB
|
✅ Validation result: with #1814 in, the lane fits — measured, not argued
The decisive comparison, same tests, same commit content, one variable: So the cancel was a scheduling-budget problem with an already-open fix, exactly as described — 593s Merge order confirmed by measurement: land #1814, then this. No change to this PR is required, and The validation branch has served its purpose and is deleted. |
Generality, stated independently of the benchmark — including the part that cuts against this PRThe body above justifies the lane as "three of the nine matched graph-benchmark cells — q1, q3 and q4." That is a benchmark justification, not a shape justification, and on its own it is not sufficient. Measured answer, on non-benchmark shapes: Where the lane generalizes. dgx-spark,
So the lane is not q1/q3/q4-shaped. It serves the grouped-aggregate shape, and 7 of the 10 grouped-aggregate variants probed. Where it does NOT — and this is the honest limitation. The order-totality gate declines the form an analyst actually writes:
q4's benchmark text happens to name every group key, so its sort is total and the lane engages. Drop the tiebreaker and it declines and stays 87–97% eager. That asymmetry is worth naming in the PR rather than leaving for a reviewer to find. Sizing the prize behind that gate, gate removed in a scratch monkeypatch (same plan), 8 interleaved slots:
13–23%, most of it inside the noise band at n=4 slots. Values were identical on this dataset, but that is not safety evidence — one dataset, one seed, and the earlier 47-of-64 row-order divergence sweep stands unrefuted. Conclusion: the gate stays, and this PR is landable as-isThe order-totality gate is a measured correctness boundary, not a shape gate reverse-engineered from the benchmark — removing it changes which rows come back under a non-total Widening it is a cross-engine row-order policy decision, not an engineering one, and belongs in its own change: when Two corrections to the body above
Unrelated pre-existing bug this work surfacedThese lanes bare-call |
Only CHANGELOG conflicted, and both entries are kept -- #1823's fused single-hop grouped aggregate and this PR's constant folding are disjoint lanes (#1823 serves q1-q4, this serves q5-q7), and gfql_fast_paths.py auto-merged with no overlap. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YYZRXegrALuXd3NHH5evqx
… tests actually run The entry claimed a "288-cell pandas x polars x cuDF sweep over 8 aggregates x 9 dtypes ... zero divergences (24 before)". Nothing in the repo produces 288: the shipped matrix is `_MATRIX_AGGS` (6: count/sum/avg/min/max/collect) x `_MATRIX_COLS` (10: 5 numeric + 4 non-numeric + 1 all-null) x 3 engines (polars/cudf/polars-gpu), i.e. 180 cells, each compared against the pandas oracle on BOTH the value and the error class. 288 = 8x9x4, which matches neither the agg list, the column list, nor the engine list, and the prose names three engines while 4 would be needed to reach it. The "(24 before)" baseline has no artifact behind it either, so it is dropped rather than restated -- an unverifiable number is worse than no number. Also: "the aggregate kernels are written three times" was true when this branch was cut and is not now. #1823 added a fused lazy lane in front of the OLAP single-hop grouped fast path, making four -- which is exactly why this branch had to add a decline to that lane. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YYZRXegrALuXd3NHH5evqx
My previous merge of this branch was built by a helper that assumes a branch ADDS a CHANGELOG entry and re-inserts it under its section. This branch does not add one -- it REWRITES #1823's existing entry -- so the helper produced the entry TWICE: master's uncorrected line plus the corrected one, which left the withdrawn claim still present in the file the PR was meant to fix. Rebuilt from master with the correction applied as an in-place replacement, asserting both that the anchor occurs exactly once on master before the edit and that the entry occurs exactly once after it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YYZRXegrALuXd3NHH5evqx
Docs sit at the TOP of the stack so a change-request here cannot block the code PRs underneath it. CHANGELOG hand-resolved: this branch REPLACES #1823's entry rather than adding one, so neither a textual 3-way nor the add-only fallback applies. Built as the parent file with the q8 withdrawal applied in place, asserting the anchor occurs exactly once before the edit and the entry exactly once after. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YYZRXegrALuXd3NHH5evqx
Docs sits at the TOP so a change-request here cannot block the code underneath. CHANGELOG hand-resolved: this branch REPLACES #1823's entry rather than adding one, so neither a textual 3-way nor the add-only fallback applies. Built as the parent file with the q8 withdrawal applied in place, asserting the anchor occurs exactly once before the edit and the entry exactly once after. Rebuilt from this branch's clean pre-stack head: an earlier stacking attempt uploaded child blobs over already-merged ones, which propagated a dropped POLARS_TEST_FILES entry up the whole stack and turned test-gfql-core red. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YYZRXegrALuXd3NHH5evqx
…istry#1825) graphistry#1823's entry closed with "q5 and q8 are ties at both scales in both runs, and q8 stays a win over Kuzu". The first half is a real measurement of what that lane does. The second half is a board-position claim that does not survive re-measurement, and it is now merged on master where it round-trips as fact. q8's headline number is a CROSS-CALL MEMOIZATION artifact. The two-hop equal-domain degree counts are cached onto the CALLER's Plottable keyed by id(), so the figure quoted on the board is a warm repeat call: warm 2.02 ms @20k 5.09 ms @100k cold (memo delattr'd) 13.18 49.68 fresh Plottable 14.60 53.02 bind_only control 0.02 0.02 <- rules out re-binding A ONE-SHOT q8 therefore loses to embedded Kuzu by 3.1-5.2x at 20k and 2.8-6.0x at 100k. The memo itself is filed as graphistry#1825, because keying a cache by id() on the caller's object also returns a stale answer after an in-place edge or node mutation -- so the cell is not merely optimistic, the mechanism behind it is a bug. Reconciled rather than deleted: the withdrawn sentence is quoted in place so the correction is traceable from the claim, and the measurement graphistry#1823 actually reports -- that its lane leaves q8 unchanged -- is explicitly preserved. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YYZRXegrALuXd3NHH5evqx
_execute_single_hop_grouped_aggregate_fast_pathserves the cypher shapewhich is three of the nine matched graph-benchmark cells — q1, q3 and q4. Its polars branch
chained ~7 eager ops — two semi-joins, two property joins, a
group_by, asortand ahead,each its own
lazy().collect(_eager=True)— so every intermediate materialized, theselect([src, dst])projection could not be pushed into the semi-joins, and thehead()could notreach into the plan at all. 74–96% of each of those queries' wall time sat inside those collects.
The same op sequence is now built as one lazy plan collected once. The algebra is
character-identical to the eager twin (same
.unique()id frames, same semi-joins, sameun-deduplicated property lookups, same
group_by(maintain_order=True).agg(..), same per-keynulls_lastsort, samehead), so the value is identical — row order included.Strictly additive: the eager code is untouched and is the fallback on every decline, so the
blast radius is a decline away from zero.
Sizing was rebuilt before anything was built, and it reproduced
Query text loaded from
benchmarks/graphbench/matched_q1_q9/gb_queries.py, md56e7ae268a5a41742587fcb87854b6e27, verified against bench commit7e5ce3band unchanged atpyg-bench
9311202. Nothing hand-written.Baseline reconciliation vs the published 20k board — MATCHES. Clean master
49db91cc9, theboard's own runner
gb_gfql.py, 4 slots:In-path paired control (eager op chain vs one lazy plan, same process, 9 paired invocations per
query, values identical every time): 20k q1 −42.5%/−41.6%, q3 −42.4%/−36.7%, q4 −15.8%/−15.7%;
100k q1 −36.3%/−33.5%, q3 −29.8%/−23.3%, q4 −37.1%/−25.9%. The prior hand-built −35% figure for q3
reproduces from canonical text, so the build was worth doing.
Result: one win widened, one loss converted to a TIE, one loss narrowed. Zero flips to WIN.
dgx-spark, matched q1–q9 lane, one perf lock per experiment, master tree vs PR tree
position-balanced
M P P M P M M P, per-slot medians (never best-of), and replicated end to endin a second independent run with 4 Kuzu slots instead of 2. Verdict rule fixed before the
replication ran: per-slot medians; overlapping ranges are a TIE, never a win.
20k (run 1 / run 2):
100k (run 1 / run 2): q1 42.96/42.59 → 31.66/31.45 (−26.3% / −26.2%, disjoint both);
q4 12.29/12.19 → 9.57/10.43 (−22.1% / −14.4%, disjoint both); q3 15.41/15.26 → 13.41/13.88
(−13.0% disjoint / −9.1% overlapping); q5 and q8 overlapping in both. 100k was already a win
column: q1 3.47× → 4.70×, q3 1.98× → 2.17×, q4 1.09× → 1.28×, q8 unchanged.
Explicit verdicts
serves it — and it is faster, by 32% at 20k and 26% at 100k, non-overlapping in all four runs,
with its win over Kuzu widening. There is no trade to surface.
(5.20–5.95 ms) overlaps Kuzu's (5.46–6.98 ms). The nominal median ratio is 1.14× in GFQL's favour
and I am not calling that a win. This is thinner than the ~1.10× flip that was projected; the
projection is superseded by the measurement.
Narrows as projected.
Value identity: one canonical row set per query across every slot of every run — 0 mismatches.
Shape enumeration, and what is declined
26 cypher shapes were probed end to end, with engagement instrumented rather than assumed. The
fused lane serves 14, declines 5, and is never reached by 7 — those seven are declined by
the fast path's own guard first (undirected, reverse, two-hop, named edge, no aggregate, aggregate
without a group key,
SKIP+LIMIT), and the tests assert the fast path was called and declined,which is what makes the empty fused-call list meaningful. Three further shapes (
min/maxover analias property,
count(DISTINCT ..), a repeated MATCH alias) are declined by the cypher loweringand never reach the fast path at all;
min/maxare still translated and unit-tested, because thefast path is reachable from a hand-built chain even though cypher cannot get there today.
Declines, each with its reason and each pinned by a test:
polars
.lazy());mid-chain and returns
Nonefrom the whole fast path, so the query then raises the polarsNotImplementedError. A fused plan built before that check would have silently answered instead —so the guard is hoisted ahead of plan construction, and the test pins the raise;
ORDER BYdoes not fully determine — see below.The order-totality gate is a measurement, not a precaution
Without every group key in the sort, the eager twin's row order falls back to
maintain_order=Truegroup first-appearance order over an eager join output — a property alazy plan is free to change by re-ordering or re-siding joins, and one that
LIMITturns from acosmetic difference into a different row set.
I built an ungated variant of the same lazy plan and compared it against the eager twin over 4
graph sizes × 4 seeds × 4 order-undetermined shapes: 64 comparisons, 47 divergent, and every
LIMIT-bearing divergence differed in row set, not merely row order. The positive side is pinnedtoo: with a total sort, 128/128 comparisons at up to 60k nodes / 137 groups are identical to both
the eager twin and pandas.
Verification
cases, each comparing against both the eager code and the pandas oracle, row-order and
column-order sensitively. Fixtures: base, duplicate node rows on the end arm, duplicate node
rows on the start arm, self-loops + parallel edges, string ids, dangling endpoints, empty
edges, no matching nodes, null properties, all-null group key. 0 divergences from the eager
code. (A scratch harness over the wider 19-shape × 9-graph corpus gave the same answer: 342
comparisons, 252 with the lane engaged, 0 divergences from the eager code — every divergence it
reported was against pandas, and the eager code reproduced each one, which is how the pre-existing
multiplicity asymmetry below was identified as pre-existing rather than introduced.)
The first round killed 19/25 and the survivor analysis found two real gaps, both now closed:
a composite mutant (start-arm semi-join → non-deduplicated inner join) was invisible because every
duplicate-node fixture put its duplicates on the end arm, and a group-key-reordering mutant was
invisible because dict equality ignores key order. Fixtures and the comparison were strengthened
and both mutants now die. The 7 remaining survivors are provably equivalent rewrites, not gaps:
semi→inneron either arm (the right side is a.unique()'d single column, so an inner joinemits each left row at most once and
right_oncoalesces the key — and the composite with thededup removed is killed); dropping
.unique()on a semi key side (duplicates on a semi key sidecan neither change nor multiply rows — and the composite with
semi→inneris killed);inner→lefton the property join (the preceding semi-joins already guarantee every endpoint ispresent — and the composite that also drops that semi-join is killed);
maintain_order=True→False(provably redundant under the totality gate, since the sort istotal; left in place so the plan stays character-identical to the twin);
pl.len()→pl.first().len()inside an agg (a synonym — the real count mutants, off-by-one andn_unique, both die); and a shared lookup-key name across the two aliases (each join consumes thekey before the next lookup frame is built).
--gpus all,graphistry/test-rapids-official:26.02-gfql-polars(cudf 26.02.01, cudf_polars present, polars 1.35.2 — a version-drift check against the local
1.42): 373 passed, 0 skipped. Every
polars-gpuandcudfparameter actually executed. LocalCPU run: 197 passed, 176 skipped (those are exactly the GPU parameters).
LOCK-FREE-CONFIRMED, no runnerprocesses left.
bin/lint.shandbin/typecheck.shclean. NoAny, nocastat the call boundary, nogetattr.Structural lock-in
pyg-bench graphistry/pyg-bench#115 — structural, not a timing gate (the win is a constant-factor
plan fusion, so a growth-ratio gate would keep passing after the lane stopped engaging). On the
board's own query text it asserts q1/q3/q4 are served, q5–q9 never enter the lane, fused ==
forced-eager == pandas with row and column order, and that an order-undetermined variant of q4
declines. Verified to fail against a build without the lane.
Disclosed, pre-existing, deliberately not changed here
The polars property lookup is not deduplicated by node id while the pandas one is
(
drop_duplicates(subset=[node_col])), so a node table carrying the same id twice multiplies matchedrows on polars only. That asymmetry predates this change; the fused lane reproduces the eager polars
answer exactly rather than quietly "fixing" it inside a performance change, and a test pins both
sides on both arms of the hop.
Rebase note (three PRs touch this file)
The diff is deliberately mechanically separate from the other two in-flight changes to
gfql_fast_paths.py. In master coordinates: #1800 edits_RESIDUAL_TOLOWER_EQaround line 617;#1816 inserts at line 1546 and edits
_execute_two_hop_count_fast_pathat line 1904; this PRinserts at line 1574 and edits one line at 1713 inside
_execute_single_hop_grouped_aggregate_fast_path. Every pair is far more than three lines apart, soall three merge cleanly in any order. Tests go in a new file rather than into
test_lowering.py(#1816) or
test_residual_polars_native.py(#1800); the only shared file isCHANGELOG.md.🤖 Generated with Claude Code
https://claude.ai/code/session_015YsqAZQLbqjSDrYSFz2GoB