Repository navigation
Plan and execute common RowFn batches in one dispatch - #10675
Draft
connortsui20 wants to merge 2 commits into
Draft
connortsui20 wants to merge 2 commits into
connortsui20 wants to merge 2 commits into
Conversation
Merging this PR will degrade performance by 10.65%3 warnings
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing 
|
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
3 times, most recently
from
October 27, 2026 19:40
322b0ae to
d624249
Compare
connortsui20
changed the base branch from
develop
to
ct/row-fn-cleanup/batch-inspection
October 27, 2026 19:41
connortsui20
force-pushed
the
ct/row-fn-cleanup/fused-execution
branch
from
October 27, 2026 21:19
d624249 to
712ed09
Compare
connortsui20
force-pushed
the
ct/row-fn-cleanup/fused-execution
branch
from
October 29, 2026 14:30
712ed09 to
d3e04bb
Compare
Base automatically changed from
ct/row-fn-cleanup/batch-inspection
to
develop
October 29, 2026 15:19
Finish all-null, all-constant, null-free, and dense batches in the planning visit, and hand every other batch back as a PartiallyValidBatch for dense retry or valid-row execution. Decode a direct Boolean batch straight from the borrowed inputs, construct Boolean output with the batch validity attached, and mask dense output that does not carry it. Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
A static AVX-512 build inlines collect_bool_words_avx512 only when rustc emits that copy in the caller's codegen unit. This branch moved it out of the RowFn deferred Boolean kernel's unit, adding 15% more instructions per row. Such a build has nothing wider to detect, so use collect_bool_words_inline, which already packs with AVX-512. Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
connortsui20
added a commit
that referenced
this pull request
Oct 29, 2026
Tracking Issue: #9128 ## Summary Prepares the RowFn batch types for #10675 without changing how a batch executes. Execution still plans with one dispatch and executes with a second one, as on `develop`. This is a purely mechanical change that should help with function call and access overhead. ## Changes - Adds `InputElement::decode_ref`, which decodes a borrowed input. The default clones the array for `decode`. Native primitives share their canonical buffer without executing the array. - Inspects the inputs once in `BatchProbe`: row counts, batch-constant classification, null literals, and the conjoined validity. Execution reads the probe instead of scanning the inputs again. A null literal skips the validity conjunction, because every row is null either way. - Keeps the columns in `BatchInputs`, which borrows them when the caller holds them as one slice. `RowFnExecutionArgs` borrows the batch and the dtypes instead of owning copies. `execute_row_inputs` runs over any `RowInputs`, and `execute_rows` adapts a general `ExecutionArgs` to it. - Skips the all-valid check in `finalize_kernel_output` when the output is not nullable, and marks the `CachedId` read `#[inline]`. Adds five regression cases covering invalid payloads, null short circuits, lazy division, device-backed validity, and device-backed primitive input. They pass on the existing execution flow, so #10675 shows that its single-dispatch path keeps that behavior. --------- Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
connortsui20
force-pushed
the
ct/row-fn-cleanup/fused-execution
branch
from
October 29, 2026 15:19
d3e04bb to
37c7a3e
Compare
1 of 2 tasks
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. It builds on #10899, which separates batch inspection from planning and adds the regression cases this PR must keep passing.
Changes
BatchProbehas classified all of them, so a later null literal still skips an earlier fallible expression or device transfer. Fallible decoders keep valid-row execution.PartiallyValidBatch, the only input that dense retry and valid-row execution accept, so every caller runs the fast paths first.collect_bool_words_multiversioneduses the inline packing loop instead of the#[target_feature]copy, so its inlining no longer depends on codegen-unit placement.Performance
Still draft. In the AVX-512 build, the RowFn changes moved the deferred Boolean kernel's AVX-512 packing loop out of its caller's codegen unit, so LLVM could no longer inline it. CodSpeed reported the two partially valid
deferred_bool_avx512cases as 42% and 16% slower. With the last commit, CodSpeed no longer flags either case. Builds that detect AVX-512 at runtime compile the same code as before.CodSpeed on the current head flags
encode_primitives[u8, (2000, 4)]in CPU simulation as 16.6% slower. Simulation uses the AVX2 build, where the last commit changes no code, and the previous head did not show this flag against an olderdevelop. Open question: does the flag come from the newerdevelopbase?AVX-512 investigation on EC2
C7i metal (Xeon Platinum 8488C) with the CodSpeed AVX-512 build flags, the bench profile, one isolated CPU, and 21 paired blocks of 1,000 samples. Ratios are the median paired ratio to
develop(8c87c4c457). Below 1 is faster. An A/A comparison of two identical binaries flags none of the 100 cases.deferred_bool_avx512casedevelopConstantLhs, PartialAcceptedColumns, PartialAcceptedBefore the fix, the kernel executed 15% more instructions per row. LLVM inline remarks show
collect_bool_words_avx512inlined ondevelopand "definition is unavailable" on this branch.Across all 100
#[cpu_features]cases, the fix also returnscompare_string_ltto thedevelopspeed. It makes twointersect_by_rankcases and onestarts_withcase 3.5% to 5.6% slower, but those execute the same instructions as before, so the difference is code placement. The AVX2 build is unchanged.Historical EC2 framework measurements
These numbers belong to an older framework replay and do not establish the performance of this revision. 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.
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.
The separately confirmed NEON comparison gains were published in #10843, which has merged. They do not measure this framework.
CI passed on
322b0ae2eb, which has the same tree as the current head, including all-target, all-feature Clippy with warnings denied and the Linux, Windows, sanitizer, and Miri test jobs.