perf(gfql): elide the interval shift in the dense-domain two-hop kernel (q8@100k, round 2) - #1847
Open
lmeyerov wants to merge 4 commits into
Open
perf(gfql): elide the interval shift in the dense-domain two-hop kernel (q8@100k, round 2)#1847lmeyerov wants to merge 4 commits into
lmeyerov wants to merge 4 commits into
Conversation
Round 2 on the q8@100k residual. Round 1's kernel report blamed the ~31.5ms local floor on "memory-bound single-threaded bincount over 2.4M edges" -- line-level profiling says otherwise. On the 100k graph-benchmark (2.42M FOLLOWS edges, Person ids 1..100000 so lo=1), the 26.3ms kernel splits: 16.2ms materializing the two shifted id arrays (src-lo, dst-lo: two fresh ~19MB allocations per call, page-fault bound in-context -- the same subtract standalone is 0.9ms), 3.2ms in the two per-Series bounds-proof chains, and only 6.2ms in the bincounts themselves. The bincount was never the floor; the shift around it was. Three changes, all inside the proof-gated kernel, values identical by construction: - Shift ELISION: when lo >= 0 and hi+1 fits the SAME table budget the domain guard already enforces (4E+1024), count the raw arrays with tables of size hi+1. The bounds proof guarantees nothing lands below lo, so both tables stay aligned and the [0, lo) prefix is provably all-zero -- contributing nothing to the product-sum. Counting body 23.6ms -> 6.9ms local. - Distant (lo large) or negative intervals still shift, but through ONE reused scratch buffer (xp.subtract(..., out=)) instead of two fresh E-length arrays; mixed src/dst dtypes keep the plain shift rather than impose a cross-dtype cast policy. - The polars bounds proof fuses all six reductions (min/max/null_count per endpoint column) into one parallel select: 2.1ms -> 1.1ms. pandas/cudf keep per-column reductions (already single C/device calls); the helper becomes _edge_cols_bounds_within. Measured rejections (2.42M edges, 24-core local, diagnosis-only): polars group_by degree counting 18.7ms vs bincount 6.9ms (the engine's parallel group_by is NOT the honest lever at this scale); np.add.at 7.9ms; one-pass concat bincount 25.7ms; chunked threaded bincount 4.8ms best-case -- real but marginal against pool overhead, dropped. Local diagnosis-only effect (same harness both sides, cold binding, answers byte-identical): polars q8@100k 33.9ms -> 17.3ms (1.96x); pandas q8@100k 213.8ms -> 201.4ms (its floor is the object-dtype rel filter, not this kernel); 20k unchanged within noise on both engines (shift there was ~0.3ms). The authoritative numbers must come from the locked DGX rerun of the receipted matched q1-q9 lane. Tests: lane pins via an array-namespace spy (shift-elision lane counts raw arrays with hi+1 tables and never allocates; distant-interval lane routes both columns through one scratch buffer; mixed-dtype lane keeps the plain shift), both sides of the table-budget boundary, non-integer edge-column declines on both engines, the helper's standalone empty-frame guard, and a CPU emulation of the cudf null-mask series shape. Existing T6 value-parity, offset/negative-interval, and decline pins all still pass unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011AB4RZpph3uSFUpzKnZJcr
…erformance Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011AB4RZpph3uSFUpzKnZJcr
# Conflicts: # CHANGELOG.md
Stack sync after the #1844 review follow-ups (typed per-engine dense-proof helpers + T6B review-sufficiency suite). Union resolution: round 2's fused _edge_cols_bounds_within is kept unchanged; the per-column _int_col_bounds_within dispatcher over the typed arms is retained as the seam the T6B tests pin directly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011AB4RZpph3uSFUpzKnZJcr
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
Round 2 on the q8@100k residual, stacked on #1844 (
perf/gfql-q8-class-fix; this PR's base is that branch, so the diff here is only the round-2 commit plus its changelog entry — GitHub will retarget to master when #1844 lands).Round 1's kernel report blamed the ~31.5 ms local floor on "memory-bound single-threaded bincount over 2.4M edges" — line-level profiling says otherwise. On the 100k graph-benchmark (2.42M FOLLOWS edges, Person ids 1..100000 so lo=1), the 26.3 ms local kernel split: 16.2 ms materializing the two shifted id arrays (src-lo, dst-lo: two fresh ~19 MB allocations per call, page-fault bound in-context — the same subtract standalone is 0.9 ms), 3.2 ms in the two per-Series bounds-proof chains, and only 6.2 ms in the bincounts. The bincount was never the floor; the shift around it was.
Three changes, all inside the proof-gated kernel, values identical by construction:
lo >= 0andhi+1fits the SAME table budget the domain guard already enforces (4E+1024), count the raw arrays with tables of sizehi+1. The bounds proof guarantees nothing lands belowlo, so both tables stay aligned and the[0, lo)prefix is provably all-zero — contributing nothing to the product-sum.xp.subtract(..., out=)) instead of two fresh E-length arrays; mixed src/dst dtypes keep the plain shift.Measured rejections (2.42M edges, 24-core local, diagnosis-only): polars group_by degree counting 18.7 ms vs bincount 6.9 ms;
np.add.at7.9 ms; one-pass concat bincount 25.7 ms; chunked threaded bincount 4.8 ms best-case — real but marginal against pool overhead, dropped.Honest transfer note
The local ~2x (counting body 23.6 → 6.9 ms; polars q8@100k 33.9 → 17.3 ms local) did NOT transfer to the DGX board host: the shift cost there was local page-fault behavior absent on DGX. On the receipted DGX candidate lane the q8@100k polars cell moves 15.94 ms (round-1 kernel alone) → 14.72 ms, and the comparator verdict stays
LOSE 1.52xagainst warmed Kuzu 9.70 ms. The change is kept for the elision's correctness-neutral simplification plus the small real gain. Round-3 target: the Kuzu warmed goalpost is ~9.7 ms.Measured cell (attributed: q8@100k, jointly with #1844)
Measurements come from the COMBINED candidate build
938f22851e68918799da2bee49c88db2e2c103ee(masterf875724ce+perf/gfql-q8-class-fix+perf/gfql-singlehop-subsumed-semijoin+perf/gfql-q7-two-star-minimal-join+ this branch), with per-fix attribution by disjoint cell sets: the q8 branches own q8 (this one the 100k residual), the single-hop PR owns q1/q3/q4, the two-star PR owns q5/q6/q7.LOSE 2.44xLOSE 1.52xLOSE 2.94xWIN 1.27x (WEAK: slot ranges overlap)(attributed to #1844; 20k was unchanged within noise by this round locally)Untouched cells did not move: cells outside the q8 attribution changed only where attributed to the sibling branches; the unattributed cells (q2, q9) kept their WIN verdicts at both scales.
Receipts
results/graphbench-board-20k-cand-20260803/andresults/graphbench-board-100k-cand-20260803/(compare.txt, SOURCE_COMMITS, lock-receipt, 1 Hz load receipts, SHA256SUMS).938f22851e68918799da2bee49c88db2e2c103ee, pyg-benchfae1e975.Tests: lane pins via an array-namespace spy (shift-elision lane counts raw arrays with
hi+1tables and never allocates; distant-interval lane routes both columns through one scratch buffer; mixed-dtype lane keeps the plain shift), both sides of the table-budget boundary, non-integer edge-column declines on both engines, the helper's standalone empty-frame guard, and a CPU emulation of the cudf null-mask series shape. Existing round-1 T6 value-parity, offset/negative-interval, and decline pins all pass unchanged.Do not merge without owner review.
🤖 Generated with Claude Code
https://claude.ai/code/session_011AB4RZpph3uSFUpzKnZJcr