perf(gfql/polars): make the chain combine proportional to the traversal result by lmeyerov · Pull Request #1785 · graphistry/pygraphistry

lmeyerov · 2026-07-27T00:45:27Z

perf(gfql/polars): make the chain combine proportional to the traversal result

Stacked on #1784. Draft — needs a pyg-bench lane and an SF1 lane A/B before it can land.

The recorded mechanism was wrong — for the third time in this area

The plan recorded the IS5 residual as UNIQUE BY [key] over a UNION of the per-step
full frames. Instrumenting the step frames says otherwise: they are 1 row each, and
LazyFrame.profile() puts that unique() at 0.008 ms. The union inputs were never the
graph.

The real one: an empty-probe semi-join against the whole node table

_combine_edges runs the prev/next endpoint gates for every step — including node
steps, whose edge frame is g._edges.clear(), i.e. zero rows. The eager combine had a
skip for exactly that case. The collect-once rewrite lazified the step frames, and .lazy()
erases the height, so if not is_lazy(df) and df.height == 0 went permanently false and the
skip died silently.

For step 0 the gate's key side is prev_nodes = g._nodes — the whole node table — and polars
builds the hash table on the right before discovering the left probe has no rows.
Isolated in raw polars: 6.99 ms for one empty-probe join at N=2M, versus 0.22 ms against
a one-row key side. A chain pays one per node step.

This is the same class as #1783 (empty left ⇒ don't materialize the right), the semi-join
case rather than the left-merge case.

Second term found while profiling: node rows were materialized in two passes over the
node table (step ids, then the endpoints the first pass missed), 0.90 + 0.86 ms at N=2M.

The change

_LazyShim records the pre-lazy row count (tri-state); _combine_edges drops
known-empty steps before the gates. A frame that arrives already lazy reports None
and is planned normally — this keys on known empty, never on a guess.
_combine_nodes → _combine_node_ids (ids only, undeduped — it is a semi key side)
plus _materialize_node_rows, which unions both id sides first and reads the node table
once. The row-level unique(subset=[node], maintain_order=True) is kept verbatim:
those rows feed how="left" alias joins where duplicate keys multiply.

Both preconditions are structural — frame cardinality and semi-join algebra. No query,
schema, label or constant is referenced anywhere.

Scaling, one dimension at a time (min of 9, local CPU)

	before	after
NODES 250k → 4M @ E=2M	10.17 → 45.48 ms	7.60 → 17.17 ms (2.65× at 4M)
node slope over 16× N	+35.3 ms	+9.6 ms (3.7× flatter)
EDGES 500k → 8M @ N=1M	13.47 → 26.02 ms	7.08 → 17.57 ms
edge slope	+12.6 ms	+10.5 ms (unchanged)

The edge slope being unchanged is the point: this is a node-side fix and should not move it.
Same 2–3× on n-e-n, n-e-n-e-n and undirected.

Parity

280 shape × graph combos (clean / self-loop / dangling / dup-keys / multi-edge /
back-edge / isolated) and 400 dup-node-id combos × 5 repeats, compared as full frames
including row order: 0 diffs.
graphistry/tests/compute/ failure set byte-identical — 119 pre-existing before and after,
0 new.
1003-case polars chain differential suite + perf(gfql/polars): stop deduplicating semi-join key sides #1784's semi-dedup suite pass. lint + typecheck
clean.

Mutation results (reintroduce the bug → the new tests must fail)

mutation	result
drop empty-step skip	2 fail
restore two-pass materialization	1 fail
drop row-level node dedup	18 fail
drop endpoint fold	2 fail
drop node order restore	34 fail
skip steps of unknown height	1 fail
drop edge order restore	0 fail — not caught
drop `maintain_order` on node dedup	0 fail — not caught

What is NOT verified — disclosed rather than papered over

The edge order restore (sort(EORD)) is unpinned. Probed 5 shapes × {5k, 60k, 300k}
with the sort removed: the frame comes back in EORD order every time, so it is
unobservable. Pre-existing defensive code, untouched.
maintain_order=True on the node dedup is unobservable — the semi-join feeding it does
not preserve left order, so "the first row in node-frame order" is not a property this
engine guarantees. A/B confirms which duplicate survives is unchanged (400 dup-id combos,
identical frames), but the docstring claim was removed rather than asserted. An initial
version of that unit test asserted the unguaranteed order and was intermittently failing —
caught before commit.
The endpoint fold could not be shown load-bearing end to end. 6300 generated chain
executions found no case where the step ids miss an edge endpoint, so the first version of
that test was vacuous (it passed with the endpoint side removed entirely).
_materialize_node_rows was extracted so the case can be constructed at the helper, where
the mutation does fail. The fold is kept as a safety net, not deleted.
Remaining O(N)/O(E): one full-frame semi probe per output frame (~0.9 ms per 2M rows,
nodes and edges). That is the floor for "select rows by id" without an index-backed gather;
a NODE_ID-index gather would make it O(result) but needs an eager id collect and breaks
the collect-once design. Separate, larger change.
Measured on local CPU only. No SF1 lane A/B yet — that plus a pyg-bench lock-in lane are
what this needs before landing.

lmeyerov · 2026-07-27T01:44:39Z

`[1]` cleared — the lock-in lane was run, per-probe, per-build

pyg-bench #101 is merged (a672a46) and the lane was then run from merged main against
five frozen read-only git archive exports, perf lock held externally for the whole sweep on
a quiet host.

probe	pins	master	#1782	#1783	#1784	#1785
`indexed-typed-edge-match-hop[polars]`	#1782	regression	pass	pass	pass	pass
`empty-left-typed-edge-miss[pandas]`	#1783	regression	regression	pass	pass	pass
`polars-edge-order-materializations`	#1783	regression	regression	pass	pass	pass
`semi-join-key-dedup-node-ladder[polars]`	#1784	regression	regression	regression	pass	pass
`indexed-typed-edge-match-hop[pandas]`	#1782	pass	pass	pass	pass	pass

A monotone staircase: each PR flips exactly its own probes, nothing regresses, and #1785
holds all five. Growth over an 8× ladder:

indexed-typed-edge-match-hop[polars]: master 2.97× (2.55 → 7.59 ms) → perf(gfql): evaluate indexed edge_match on candidate rows, not the whole edge frame #1782 0.93×
(0.28 → 0.26 ms), i.e. flat.
empty-left-typed-edge-miss: master 2.55× (9.20 → 23.45 ms) → perf(gfql): empty-left merge shrink + polars EORD/EID row-index dedup #1783 0.998×
(6.67 → 6.66 ms), i.e. flat.

Note the config-level exit code is cumulative — it is the stack gate, so on its own it
reads as "master/#1782/#1783 fail". The per-probe verdicts above are what [1] actually asks
for.

One probe does not discriminate — flagging rather than burying

indexed-typed-edge-match-hop[pandas] reports pass on master. It is not blind — it
measures 5.294× growth there. It passes because the verdict ANDs the ratio gate with a
5.0 ms min_absolute_delta_ms floor, and that variant's absolute delta at this ladder size is
only 2.11 ms. So the floor, which exists to stop flaky sub-noise failures, is suppressing a
real 5.3× growth.

The [polars] variant is what actually pins #1782; the pandas one currently rides along.
It should be fixed by raising base_edges for that probe until the delta clears the floor, or
by giving it a smaller per-probe floor — not by deleting it. It is one config change away from
being load-bearing, and it is a live instance of exactly the failure mode the lane's own README
warns about.

Two run traps worth recording

uv is not on the non-interactive ssh PATH (exit 127 — use $HOME/.local/bin/uv), and the
pyg-bench uv env has no polars (--with polars). Both produced silent-looking failures first.

…claim; kill the twin guard Review skill found three items worth fixing; all three were claim-scope or follow-up-hazard rather than defects in the shipped behaviour. 1. `sort(EORD)` is NOT dead code — the probe that called it unpinned was engine blind. It only exercised the in-memory collect, where the frame comes back EORD-ordered even with the sort deleted. Under `GFQL_POLARS_CPU_STREAMING=1` it does not, and a trailing rows(limit=)/skip would then slice the wrong rows. The existing 60k order test is now parametrized over the collect engine. Mutation-checked: deleting the sort fails [streaming] and PASSES [in-memory], which is exactly why the original probe saw nothing. 2. The `maintain_order=True` docstring claimed the surviving duplicate is decided "the same way it was before", citing a 400-combo A/B. That A/B ran on the default engine only; under streaming the survivor genuinely differs. Scoped the claim to the in-memory collect and said plainly that the survivor is a stable property there, not a guaranteed one. 3. `_apply_node_names` still carried the ORIGINAL spelling of the bug this branch exists to fix: `is_lazy(df) or df.height > 0`, where the function is always called with lazified steps, so `is_lazy` short-circuits True and the height test is unreachable. Restated against `edges_empty`, which survives lazification. Unlike the edges combine this guard is SEMANTIC, not a cost guard — an empty next edge step must not empty `named` through the gate below it. I could not make the dead version observable, so this is hardening, not a bug fix; but leaving a known-dead cardinality guard beside a freshly-revived one is precisely how the original defect recurred. lint + mypy clean; 1159 polars chain/combine/semi-dedup tests pass.

…al result Two graph-sized terms sat inside a combine whose answer is a handful of rows. 1. `_combine_edges` ran the prev/next endpoint gates for EVERY step, including the node steps whose edge frame is `g._edges.clear()` -- zero rows. The eager combine skipped those; the collect-once (Track B) rewrite lazified the step frames and `.lazy()` erases the height, so the skip silently went dead. The cost lands on the side that is NOT empty: for the first step the gate's key side is the whole node table, and polars builds the hash table on that side before discovering the probe side has no rows. Isolated in raw polars: 6.99 ms for one such join at N=2M vs 0.22 ms against a one-row key side -- and a chain pays one per node step. `_LazyShim.step` now records the row count while the frame is still eager and an empty step is dropped from the id union. It can contribute no ids, so the result is unchanged by construction. The skip keys on KNOWN-empty only: a frame that arrives already lazy reports no height and is planned normally. 2. The output node rows were materialized in TWO passes over the node table -- one for the ids the steps kept, one more for the surviving edges' endpoints the first pass missed -- then concatenated. The output node set is the UNION of those two id sides, so `_materialize_node_rows` unions the ids first and reads the node table ONCE. The row-level `unique(subset=[node])` is preserved verbatim: those rows go on to feed `how="left"` alias joins where a node table carrying the same id twice multiplies rows. The id sides themselves are NOT deduplicated -- they are semi key sides. Note for the record: the recorded description of this residual ("UNIQUE over a UNION of the per-step FULL frames") named the wrong operator. The union inputs are 1-row step frames; the `unique` costs 0.008 ms. It was the union BRANCHES -- empty-probe joins against graph-sized build sides -- exactly as the earlier semi-dedup finding. MEASURED, one dimension at a time, synthetic LDBC-IS5-shaped graphs (one-row answer, polars-engine resident indexes, local CPU, min of 9): vary NODES at E=2M 250k -> 4M: 10.17 -> 45.48 ms before 7.60 -> 17.17 ms after (2.65x at 4M) node-count slope 3.7x flatter (+35.3 ms -> +9.6 ms over 16x N) vary EDGES at N=1M 500k -> 8M: 13.47 -> 26.02 ms before 7.08 -> 17.57 ms after edge slope essentially unchanged, as expected for a node-side fix; constant ~6 ms lower same 2-3x holds for n-e-n, n-e-n-e-n and undirected shapes PARITY: 280 shape x graph combinations (clean / self-loop / dangling / duplicate keys / multi-edge / back-edge / isolated) and 400 duplicate-node-id combinations x 5 repeats compared as FULL frames including row order -- 0 diffs vs the pre-change tree, and stable across repeated runs. `graphistry/tests/compute/` failure set is byte-identical before and after (119 pre-existing failures, 0 new, +119 passed = the new file). TESTS pin the boundary, not a wall clock. Mutation-checked; each row is a deliberate reintroduction of a bug, run against the new file: drop the empty-step skip -> 2 fail (both structural tests) restore the two-pass materialization -> 1 fail (node table read 2x) drop the row-level node dedup -> 18 fail (parity + dedup + order) drop the endpoint fold -> 2 fail drop the node order restore -> 34 fail skip steps whose height is UNKNOWN -> 1 fail NOT caught, and reported as such: dropping the edge order restore, and dropping `maintain_order` on the node dedup. Neither is observable -- the edge semi-join returns EORD order naturally at 5k/60k/300k across 5 shapes, and the node semi-join feeding the dedup is unordered either way. Both are pre-existing defensive code, untouched here. Also honest about coverage: the endpoint fold could not be shown load-bearing end-to-end (6300 generated chain executions found no graph/shape where the step ids miss an edge endpoint), so it is tested at the helper, where the case can be constructed -- an end-to-end test of it would have passed with the endpoint side removed entirely. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VARwnAqmvczhz5EmP7TYyZ

…claim; kill the twin guard Review skill found three items worth fixing; all three were claim-scope or follow-up-hazard rather than defects in the shipped behaviour. 1. `sort(EORD)` is NOT dead code — the probe that called it unpinned was engine blind. It only exercised the in-memory collect, where the frame comes back EORD-ordered even with the sort deleted. Under `GFQL_POLARS_CPU_STREAMING=1` it does not, and a trailing rows(limit=)/skip would then slice the wrong rows. The existing 60k order test is now parametrized over the collect engine. Mutation-checked: deleting the sort fails [streaming] and PASSES [in-memory], which is exactly why the original probe saw nothing. 2. The `maintain_order=True` docstring claimed the surviving duplicate is decided "the same way it was before", citing a 400-combo A/B. That A/B ran on the default engine only; under streaming the survivor genuinely differs. Scoped the claim to the in-memory collect and said plainly that the survivor is a stable property there, not a guaranteed one. 3. `_apply_node_names` still carried the ORIGINAL spelling of the bug this branch exists to fix: `is_lazy(df) or df.height > 0`, where the function is always called with lazified steps, so `is_lazy` short-circuits True and the height test is unreachable. Restated against `edges_empty`, which survives lazification. Unlike the edges combine this guard is SEMANTIC, not a cost guard — an empty next edge step must not empty `named` through the gate below it. I could not make the dead version observable, so this is hardening, not a bug fix; but leaving a known-dead cardinality guard beside a freshly-revived one is precisely how the original defect recurred. lint + mypy clean; 1159 polars chain/combine/semi-dedup tests pass.

lmeyerov · 2026-07-27T07:48:55Z

SF1 A/B against master — 3 faster, 0 over the gate

Master now contains #1782 + #1783 + #1784, so this measures #1785's marginal contribution,
not the stack's combined effect. Pooled 8 samples per build across two position-balanced
rounds, rows identical on every query.

query	master	+#1785	delta	per-slot ranges
IS5 message-creator	33.32 ms	22.45 ms	−32.6%	separated
IS1 seed-lookup	39.24 ms	29.19 ms	−25.6%	separated
IS2 expand-order-limit	53.24 ms	43.09 ms	−19.1%	separated
IS4 message-content	3.76 ms	4.09 ms	+8.6%	overlap
IS7 message-replies	12.86 ms	13.76 ms	+7.0%	overlap

The two flags, and what I am not claiming

At 4 samples these read +14.6% and +10.8% — over the gate. At 8 they are +8.6% and
+7.0% with fully overlapping distributions, the same decay the OLAP sweep's three flags showed
(one of which flipped sign entirely).

But I am not calling them noise. Both tilt consistently positive and B's distribution
sits above A's on each — IS4 A[3.1…4.9] vs B[3.0…5.0], IS7 A[11.2…14.4] vs
B[11.5…15.4]. That reads as a small real constant cost, roughly +0.3 ms and +0.9 ms,
showing up only where the query is already tiny. It is mechanistically plausible: this PR adds
a tri-state height check and restructures node materialization, i.e. slightly more per-query
plan work. Below the project's 10% gate, disclosed rather than laundered.

What this unblocks

This PR is what moves IS5 across the line vs Neo4j. On master today IS5 is 33.32 ms
against Neo4j's 27.28 — a LOSE 1.22×. With #1785 it is 22.45 ms, a WIN 1.22×. I had
earlier reported that flip as landed; it was measured on the full-stack tree and belongs to
this PR, not to master. IS2's flip is on merged code and is unaffected.

Bar status

[1] lock-in lane: passes all five probes (it was the only build to do so alongside perf(gfql/polars): stop deduplicating semi-join key sides #1784).
[2] pos+neg tests either side of the boundary, mutation-checked — including one that
caught an engine-blind probe calling sort(EORD) dead code when it is load-bearing
under streaming collect.
[3] CI green; GPU suite clean on device.

…m inferred The helpers this PR added or reshaped were untyped, which violates the repo's engine-agnostic typing convention. Annotate them with the aliases rather than bare Any: `_LazyShim.__init__` / `.step`, `_known_empty`, `_combine_node_ids`, `_materialize_node_rows`, plus class-level slot annotations on `_LazyShim` (bare annotations only — a class-level value would collide with __slots__). The slots are `Optional[pl.LazyFrame]`, NOT the `PolarsFrame` union: every construction site calls `.lazy()` first, which is the point of the shim, and `pl.concat`'s TypeVar rejects a `DataFrame | LazyFrame` argument outright. The union spelling type-checks as 7 errors; the LazyFrame spelling as none. `_combine_node_ids` takes the `_LazyShim`, not a `Plottable` — the call site passes `g_lz`, a duck-typed stand-in that is not a Plottable subclass. Two consequences worth naming: * `_known_empty` needs an explicit cast for `.height`. `is_lazy` is a plain bool predicate, so mypy cannot narrow its else-branch. Widening it to a `TypeIs` would narrow this for every caller in the engine, but that is a dtypes.py-wide change and not this PR's. * `collect_all`'s results are rebound to new names. `final_nodes` is statically a LazyFrame down that block and `collect_all` returns eager frames, so reusing the name is a type error — and silencing it would cost exactly the lazy/eager distinction the shim exists to preserve. Sibling helpers in the same file (`_semi`, `_align_seed_dtype`, ...) are also untyped but pre-existing, and are left alone. Verified in-container on dgx-spark, differentially: mypy reports 213 errors with 1 in chain.py both before and after this commit — the same pre-existing `maybe_index_hop` arg-type — so this adds none. (The container's pandas-stubs drift from the CI lockfile makes the absolute count meaningless; only the differential is evidence.) The gfql suite with `--gpus all` is byte-identical to the pre-commit tree: 5570 passed, 25 skipped, 15 xfailed, 0 failed on both. ruff clean under the repo config. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015YsqAZQLbqjSDrYSFz2GoB

…surfaces (#1788) The named-middle rewrite turns `[...named ops..., rows()]` into `rows(binding_ops=<middle>)` so a Cypher multi-alias RETURN lowers to a bindings table. It excluded calls already carrying `binding_ops`, `source` or `alias_endpoints` — but not a non-default `table`. So merely NAMING an op in the middle changed which table came back: * odd-length named middle -> the rewrite fired and the caller silently got the BINDINGS table instead of the edges table. No error; `table=` ignored. * even-length named middle -> a path ending on an EDGE, so the rewritten op list is not an alternating node/edge path and it hard-errored with "require ... a single connected alternating node/edge path". * UNNAMED middle -> correct. Which is what made this look shape-specific rather than naming-specific. Found while chasing why LDBC IS3 is the only interactive-short cell with a competitor number and no GFQL score: its adapter runs a two-pass workaround whose edge half is exactly `(person)-[r:KNOWS]-` + `rows(table="edges")`. Both surfaces need the guard — `compute/chain.py` and the native polars `gfql/lazy/engine/polars/chain.py` carry the rewrite independently, and the mutation checks below show fixing one leaves the other wrong. THE GUARD KEYS ON A NON-DEFAULT TABLE, NOT ON `is None`. `rows()` declares `table: str = "nodes"` and always emits it, so `params.get("table") is None` is never true — my first attempt used that and disabled the rewrite outright, breaking the IS6 bindings path (5 regressions). Those were caught only by re-baselining against current master rather than against the stale baseline I had from an earlier branch, which no longer matched after #1781/#1785 landed. The residual cost is that an EXPLICIT `rows(table="nodes")` is byte-identical to a bare `rows()` at the params level and still rewrites; that limitation is now pinned by a test rather than left to be rediscovered. Distinguishing them would need `rows()` to default `table=None`, a wire-format change and out of scope. Tests: 10 cases across both engines — the even-length (IS3) case, the SILENT odd-length case, named-vs-unnamed equivalence, the documented `table="nodes"` limitation, and the NEGATIVE side: a bare `rows()` after a named middle must STILL get the bindings table, without which "never rewrite" would pass and break every Cypher multi-alias RETURN. Mutation-checked PER SURFACE: removing the pandas guard fails 3 pandas cases with polars green; removing the polars guard fails 3 polars cases with pandas green. Verified in-container on dgx-spark: `graphistry/tests/compute` with `--gpus all` gives 7030 passed against master 3f2128e's 7020 (+10, the new file) and an IDENTICAL 9-failure set (pre-existing dask/cuDF coercion). Claude-Session: https://claude.ai/code/session_015YsqAZQLbqjSDrYSFz2GoB Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

This was referenced Jul 27, 2026

perf(gfql): evaluate indexed edge_match on candidate rows, not the whole edge frame #1782

Merged

perf(gfql): empty-left merge shrink + polars EORD/EID row-index dedup #1783

Merged

lmeyerov mentioned this pull request Jul 27, 2026

perf(gfql/polars): stop deduplicating semi-join key sides #1784

Merged

lmeyerov force-pushed the perf/gfql-polars-semi-no-dedup branch from 7d508ab to 29e8c41 Compare July 27, 2026 02:49

lmeyerov force-pushed the perf/gfql-polars-narrow-combine branch from 8d8b9d9 to 72ae3c3 Compare July 27, 2026 02:50

lmeyerov mentioned this pull request Jul 27, 2026

feat(gfql): native polars unbounded directed var-length rows(binding_ops) — unblocks LDBC IS6 on polars (#1709) #1781

Merged

lmeyerov force-pushed the perf/gfql-polars-semi-no-dedup branch 2 times, most recently from 57f739e to b883011 Compare July 27, 2026 06:37

lmeyerov changed the base branch from perf/gfql-polars-semi-no-dedup to master July 27, 2026 06:56

lmeyerov and others added 2 commits July 26, 2026 23:56

lmeyerov force-pushed the perf/gfql-polars-narrow-combine branch from 8742dd2 to 35d93bd Compare July 27, 2026 06:58

lmeyerov marked this pull request as ready for review July 27, 2026 07:48

lmeyerov merged commit 674443e into master Jul 27, 2026
57 checks passed

lmeyerov mentioned this pull request Jul 27, 2026

fix(gfql): rows(table=...) must survive a named middle on both chain surfaces #1788

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

perf(gfql/polars): make the chain combine proportional to the traversal result - #1785

perf(gfql/polars): make the chain combine proportional to the traversal result#1785
lmeyerov merged 3 commits into
masterfrom
perf/gfql-polars-narrow-combine

lmeyerov commented Jul 27, 2026

Uh oh!

lmeyerov commented Jul 27, 2026

Uh oh!

lmeyerov commented Jul 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

lmeyerov commented Jul 27, 2026

perf(gfql/polars): make the chain combine proportional to the traversal result

The recorded mechanism was wrong — for the third time in this area

The real one: an empty-probe semi-join against the whole node table

The change

Scaling, one dimension at a time (min of 9, local CPU)

Parity

Mutation results (reintroduce the bug → the new tests must fail)

What is NOT verified — disclosed rather than papered over

Uh oh!

lmeyerov commented Jul 27, 2026

[1] cleared — the lock-in lane was run, per-probe, per-build

One probe does not discriminate — flagging rather than burying

Two run traps worth recording

Uh oh!

lmeyerov commented Jul 27, 2026

SF1 A/B against master — 3 faster, 0 over the gate

The two flags, and what I am not claiming

What this unblocks

Bar status

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

`[1]` cleared — the lock-in lane was run, per-probe, per-build