Repository navigation
Plan and execute common RowFn batches in one dispatch - #10675
Draft
connortsui20 wants to merge 11 commits into
Draft
connortsui20 wants to merge 11 commits into
connortsui20 wants to merge 11 commits into
Conversation
Merging this PR will regress 3 benchmarks2 warnings
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing 
Footnotes
|
connortsui20
force-pushed
the
ct/row-fn-cleanup/fused-execution
branch
from
October 22, 2026 14:09
55d9af7 to
178dcd8
Compare
connortsui20
changed the base branch from
ct/row-fn-cleanup/output-validity
to
develop
October 22, 2026 14:10
connortsui20
force-pushed
the
ct/row-fn-cleanup/fused-execution
branch
from
October 22, 2026 14:18
178dcd8 to
5cbf00a
Compare
connortsui20
added
action/bench-sql
github-actions
Bot
removed
the
action/bench-sql
Contributor
Polar Signals Profiling ResultsLatest Run
Previous Runs (7)
Powered by Polar Signals Cloud |
Contributor
Benchmarks: FineWeb NVMe 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.977x ➖, 0↑ 0↓)
datafusion / parquet / ns (1.001x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.022x ➖, 0↑ 0↓)
duckdb / parquet / ns (1.002x ➖, 0↑ 0↓)
No file size changes detected. |
Contributor
Benchmarks: PolarSignals Profiling 📖Commits: PR datafusion / vortex-file-compressed / ns (0.997x ➖, 0↑ 0↓)
No file size changes detected. |
Contributor
Benchmarks: TPC-H SF=1 on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.003x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.999x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.001x ➖, 0↑ 0↓)
duckdb / parquet / ns (1.002x ➖, 0↑ 0↓)
No file size changes detected. |
Contributor
Benchmarks: Clickbench on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.995x ➖, 1↑ 0↓)
datafusion / parquet / ns (0.994x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.000x ➖, 0↑ 0↓)
duckdb / parquet / ns (0.993x ➖, 2↑ 0↓)
No file size changes detected. |
Contributor
Benchmarks: TPC-H SF=10 on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.993x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.999x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.002x ➖, 0↑ 0↓)
duckdb / parquet / ns (0.999x ➖, 0↑ 0↓)
No file size changes detected. |
Contributor
Benchmarks: TPC-DS SF=1 on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.998x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.999x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.008x ➖, 0↑ 0↓)
duckdb / parquet / ns (1.010x ➖, 0↑ 1↓)
No file size changes detected. |
Contributor
Benchmarks: TPC-H SF=1 on S3 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.083x ➖, 1↑ 5↓)
datafusion / parquet / ns (0.991x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.001x ➖, 0↑ 1↓)
duckdb / parquet / ns (1.069x ➖, 0↑ 1↓)
|
connortsui20
force-pushed
the
ct/row-fn-cleanup/fused-execution
branch
from
October 23, 2026 17:53
5cbf00a to
7c47d4b
Compare
robert3005
force-pushed
the
ct/row-fn-cleanup/fused-execution
branch
from
October 26, 2026 00:51
7c47d4b to
69f5fc0
Compare
connortsui20
added a commit
that referenced
this pull request
Oct 26, 2026
## Summary - Tracking Issue: #9128 - Independent validity candidate: #10742 Routes float32 comparisons through the existing word-packing collectors. This recovers the measured float32 gains from the RowFn comparison investigation with two private dispatch changes. It is independent of the comparison migration and fusion work in #10676 and #10675. ## Changes Adds `F32` to the existing collector selections for array pairs and array/constant comparisons. Integer collector policies remain unchanged. The floating-point documentation now accounts for NaN signs and payloads. **Draft pending fresh isolated benchmarks on this head.** Saved measurements show a consistent float32 gain, but also some unrelated ARM regressions. The later combined packing/validity candidate had an AVX2 regression that has not been attributed to either change individually. <details> <summary>Saved benchmark evidence and validation</summary> The isolated packing snapshot `4c98df49e379853f8e990d9764791b5330511771` was compared with frozen `develop` at `a1693389301e446eec41373cb274d98a303d3ca0`. Measurements used Rust 1.99.1 / LLVM 23.1.1, 16 CGUs, no LTO, CPU 2, and 21 balanced paired blocks on Graviton3 C7g and Sapphire Rapids C7i. This was native wall time, not CodSpeed simulation. Across 48 float32 cases at 8,192 and 65,536 rows, covering array pairs, both constant orientations, equality/ordering, and four validity shapes, the median block ratios of equally weighted geometric means were: | Target | Candidate / baseline time | 95% paired bootstrap interval | | --- | ---: | ---: | | NEON | 0.611 | 0.610–0.611 | | AVX2 | 0.632 | 0.630–0.633 | | AVX-512 | 0.679 | 0.678–0.681 | Among all 375 probe cases, the isolated snapshot had no flagged regressions on AVX2 or AVX-512. ARM had four, including roughly 4.7% slower u64 multiply and 4.6% slower large f64 equality. Those paths do not use the changed float32 collector, so their movement is not evidence of extra float32 work, but it still counts against a no-regression claim. A separate combined packing/validity candidate at `8d8ef8151454bbc73e1cd0b236567542a1401cbe`, based on `d834a11faa79370b4d2e0bbcc73195f451a8a212`, retained the float32 gain but regressed large nullable u64 add by about 18% on AVX2. Profiling found a byte-identical retry hot loop with essentially unchanged instructions per operation and more cycles. The precise placement-sensitive mechanism remains unresolved. That combined result does not isolate this PR. The combined candidate passed 225 RowFn tests and 232 binary-function tests on that newer base. Separate native probes checked scalar-reference results and float/encoding/tail edge cases. The changed file passed the pinned formatter. These are saved checks, not fresh validation of this individual PR. This PR is rebased onto `c750fbd80882f78efd6a2703bc0f3e17f1aca9f9`. The patch is unchanged by the rebase. Tests and benchmarks were not rerun after it. </details> Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
connortsui20
force-pushed
the
ct/row-fn-cleanup/fused-execution
branch
2 times, most recently
from
October 26, 2026 20:53
e8675b5 to
5cf73d7
Compare
connortsui20
removed
the
action/bench-spatial
connortsui20
added
the
action/bench-sql
github-actions
Bot
removed
the
action/bench-sql
connortsui20
force-pushed
the
ct/row-fn-cleanup/fused-execution
branch
from
October 27, 2026 14:34
5cf73d7 to
af0b943
Compare
Keep the owned decoder as the required extension point and default borrowed decoding to it. Native primitive inputs share their canonical buffers directly. Retain decoded storage after the input handle is dropped. Extracted from #9979. Signed-off-by: Robert Kruszewski <github@robertk.io> Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Inspect borrowed inputs once for lengths, dtypes, constant classification, and strict validity. Pair collected inputs with a separate execution plan while retaining the existing dispatch and retry paths. Extracted from #9979. Signed-off-by: Robert Kruszewski <github@robertk.io> Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Let the packed Boolean collectors consume the batch validity when constructing their result. Keep custom output validation before masking and attach validity directly to other canonical outputs. Extracted from #9979. Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Finish eligible batches in the planning visit and retain the planned retry and valid-row paths for the remaining partially valid batches. Decode direct Boolean inputs through private borrowed tuple adapters and keep collector selection outside the row loop. Extracted from #9979. Enforce the Boolean validity-length invariant in release builds before unchecked output construction. Signed-off-by: Robert Kruszewski <github@robertk.io> Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Classify all inputs before decoding so a null literal can skip an earlier fallible expression or device transfer. Keep validity conjunction lazy and preserve valid-only decoding. Add regression coverage for invalid payloads, lazy expressions, and device-backed inputs. Borrow contiguous argument dtypes without adding a primitive-specific decoder hook or packing policy to the generic framework. Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
BatchInputs copied four BatchProbe fields and their documentation. Keep the probe itself instead, so a new probe fact needs one field and one doc comment. Correct the input and dtype docs that still described collection as the place where validity and constants are derived. Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Retry execution relied on its caller to run the all-null, null-literal, all-constant, and dense paths first. A crate caller that skipped them could decode a null constant or reach the all-invalid unreachable in dense finalization. try_execute_without_retry now consumes the collected batch and hands back a PartiallyValidBatch, the only value that retry execution accepts. Remove the validity plumbing that only the unreachable dense retry arm used: the ExecuteRows field, the execute_row_kernel parameter, and the execute_owned_bool parameter, which deferred visits always set to NonNullable. Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Dense execution treated a taken validity as proof that the kernel attached it. A collector that took the validity but built a non-nullable output would have returned every null row as valid. Route on the output nullability as well, so such an output is masked like any other. Construct packed Boolean outputs with BoolArray::try_new. A validity of the wrong length now returns an error, as on the deferred path, instead of panicking, and the unsafe block goes away. Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Inspect inputs for a null literal before conjoining their validity. A null literal makes every row invalid, so the probe no longer builds and optimizes an And expression that it then discards. Validate a Boolean visit once and share the storage dtype between the direct and collected paths. Let execute_row_inputs own the arity check. Execute a non-canonical primitive input directly in decode_ref instead of repeating the canonical downcast through decode. Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
STYLE.md asks for test_<function_name>_<scenario>, and all but one of the older tests in this file follow it. Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
execute_rows already collects its inputs into a slice before batch execution, but a visit that could not finish the batch directly cloned every input into a second list. BatchInputs now borrows the slice that RowInputs::as_slice provides, and clones only inputs that the caller does not hold contiguously. Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
connortsui20
force-pushed
the
ct/row-fn-cleanup/fused-execution
branch
from
October 27, 2026 15:41
41263f2 to
7dc8a2f
Compare
This branch has not been deployed
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.
Tracking Issue: #9128
Summary
Combines planning and execution for eligible RowFn batches in one type dispatch. This is the shared framework layer for #10676, which contains the primitive comparison migration.
Changes
Adds five regression cases covering invalid payloads, null short circuits, lazy division, device-backed validity, and device-backed primitive input. Rebased onto
developata5f574e69bbf9010029f0e62ca59d56cd0c4aa6e.Performance
Still draft. The revised framework has no fresh native performance result. Earlier EC2 experiments found compiler and layout sensitivity, including regressions, rather than a consistent framework speedup. The numbers below belong to an older framework replay and do not establish the performance of this revision.
Historical EC2 framework measurements
Ratios are the median of 21 paired candidate/baseline elapsed-time ratios. Below 1 is faster. This compares the framework alone, F
94be28140d5bd7e6b654af5e462253add2e49044, with D0291778db36809f77ce0219bf9850506d3cba3b6.u64multiply, 8,192 rowsi64add, 32,768 rowsThe actual framework pair order was imbalanced: D ran before F in 16 blocks and after it in five. Treat these figures as diagnostic observations, not reliable causal effect sizes. No speedup or regression claim for the current head follows from this table.
EC2 C7g (Neoverse V1), C6a (EPYC 7R13), and C7i (Xeon Platinum 8488C). Rust 1.99.1 / LLVM 23.1.1, bench profile with 16 CGUs and no LTO, MiMalloc, CPU 2 pinned. The same 455-case public-path microbenchmark suite ran serially across hosts, with 21 paired blocks, 10 ms per case, A/A calibration, and a separate control executable. These are microbenchmarks, not query results. C7i can dispatch to AVX-512 at runtime even in its AVX2 build.
Saved evidence is in the local investigation archive
row-fn-evaluation-20261026:manifest.json,*-matrix-*-medians.csv,*-matrix-*-summary.json, andresults/<host>/results/matrix-*.json.The separately confirmed NEON comparison gains were published in #10843, which has merged. They do not measure this framework.
Prior validation at
5cf73d78f3: 241 RowFn tests and all-target, all-feature Clippy with warnings denied passed. This rebase was reviewed from source, includingrange-diff. Tests, builds, and benchmarks have not been rerun on the rebased head.