Skip to content

Separate RowFn batch inspection from execution planning - #10899

Merged
connortsui20 merged 3 commits into
developfrom
ct/row-fn-cleanup/batch-inspection
Oct 29, 2026
Merged

connortsui20 merged 3 commits into
developfrom
ct/row-fn-cleanup/batch-inspection

Conversation

@connortsui20

@connortsui20 connortsui20 commented Oct 27, 2026 •

Copy link
Copy Markdown
Member

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.

@codspeed

codspeed Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Merging this PR will degrade performance by 9.47%

3 warnings

⚠️ Walltime results may be unreliable

Some benchmarks used the Walltime instrument outside CodSpeed Macro Runners, which can make the results inconsistent.

Learn more about Macro Runners

See affected benchmarks

⚠️ 3 benchmarks are dominated by system calls

System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.

Learn more about system calls

See affected benchmarks

⚠️ Base and head ran on different runner types

Some benchmarks with significant performance changes were compared across different runner types, so the changes might not come from the code. Run base and head on the same runner type, or use CodSpeed Macro Runners for more stable results.

Learn more about Macro Runners

Open the report in CodSpeed to investigate

⚡ 2 improved benchmarks
❌ 4 regressed benchmarks
✅ 2298 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
❌ Walltime threshold_neon[64] 2.6 µs 4.5 µs -42.32%
❌ CPU Simulation filter_powerlaw_by_random[10000] 813.5 ns 1,221 ns -33.38%
❌ CPU Simulation missing_stats[16] 67.9 µs 85.4 µs -20.44%
❌ Walltime threshold_neon[80] 3.5 µs 4.1 µs -14.56%
⚡ Walltime mul_u64_nonnull_neon 39.2 µs 20.6 µs +89.76%
⚡ Walltime starts_with_neon[8, VarBin] 45.5 µs 41 µs +11.05%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing ct/row-fn-cleanup/batch-inspection (0ddfefa) with develop (f04a28c)

Open in CodSpeed

@connortsui20

Copy link
Copy Markdown
Member Author

im pretty sure the mul_u64_nonnull_neon is a flake, it always changes

@connortsui20
connortsui20 marked this pull request as ready for review October 27, 2026 20:52
@connortsui20
connortsui20 removed the request for review from joseph-isaacs October 27, 2026 21:14
@connortsui20
connortsui20 force-pushed the ct/row-fn-cleanup/batch-inspection branch from aa0745f to 17164f9 Compare October 27, 2026 21:19
@connortsui20
connortsui20 enabled stacked auto-merge (squash) October 29, 2026 14:30
Keep the owned decoder as the required extension point and default borrowed decoding to it. Native primitive inputs share their canonical buffer without executing the array.

Signed-off-by: Connor Tsui <[email protected]>
Inspect the borrowed inputs once for row counts, batch-constant classification, null literals, and strict validity. Batch execution reads that probe instead of scanning the inputs again, and borrows the columns and dtypes instead of collecting owned copies. Execution still plans with one dispatch and executes with a second one.

Signed-off-by: Connor Tsui <[email protected]>
Pin the behavior that a later single-dispatch path must keep: valid-only decoding skips invalid payloads, a null literal skips earlier decoding, lazy division, and device transfers, and validity conjunction stays lazy for device-backed validity.

Signed-off-by: Connor Tsui <[email protected]>
@connortsui20
connortsui20 force-pushed the ct/row-fn-cleanup/batch-inspection branch from 17164f9 to 0ddfefa Compare October 29, 2026 14:30
@connortsui20
connortsui20 merged commit c338bca into develop Oct 29, 2026
82 of 84 checks passed
@connortsui20
connortsui20 deleted the ct/row-fn-cleanup/batch-inspection branch October 29, 2026 15:19
@connortsui20 connortsui20 mentioned this pull request Oct 30, 2026
1 of 2 tasks
connortsui20 added a commit that referenced this pull request Nov 5, 2026
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

- Decodes a direct Boolean batch straight from the borrowed inputs,
after `BatchProbe` has classified all of them, so a later null literal
still skips an earlier fallible expression or device transfer. Fallible
decoders keep valid-row execution.
- Finishes all-null, all-constant, null-free, and dense batches in the
planning visit. Every other batch comes back as a `PartiallyValidBatch`,
the only input that dense retry and valid-row execution accept, so every
caller runs the fast paths first.
- Constructs Boolean output with its validity in the shared executor.
Dense execution masks any output that does not carry that validity, and
a validity of the wrong length returns an error. Primitive-specific CPU
and lane-width packing policies stay out of this layer.
- In builds that enable AVX-512F and AVX-512BW at compile time,
`collect_bool_words_multiversioned` uses the inline packing loop instead
of the `#[target_feature]` copy, so its inlining no longer depends on
codegen-unit placement.

## Performance

This PR lowers the fixed cost of each RowFn call, so CodSpeed shows no
gain for it alone. The gain shows up in #10676, which moves primitive
comparison onto the RowFn executor. To isolate this PR, #11220 applied
the two #10676 commits directly on `develop`. Without this PR,
comparisons on small arrays are 10% to 16% slower than `develop` in CPU
simulation, including the zone map pruning benchmarks, `i64_lt`, and
`timestamp_gte`. The added time barely changes between 16 and 1,024
zones (18.0 µs and 17.3 µs for `int_gt`), so it is paid per call, not
per row. On top of this PR, #10676 shows none of these regressions, and
its walltime comparison gains are larger: `compare_u64_avx2` improves by
31% instead of 16%, and seven more comparison benchmarks improve by 12%
to 18%.

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_avx512` cases 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. #11220 also shows
`compare_u8_avx512` and `compare_f32_avx512` as 14% and 10% slower
without this PR, which matches the same packing problem, but that
build's assembly wasn't inspected.

CodSpeed on the current head flags three varbin comparison cases and
`random_i8[0.01]` as 12% to 18% slower, and one `take_filter` case as
13% faster. None of them runs a RowFn. #10676 reverses the varbin and
`take_filter` changes without touching their code, so they follow code
placement. `random_i8[0.01]` isn't a stable signal either: its `develop`
baseline moved from 99.6 µs to 119.3 µs between `bc58587` and `c9a3fbe`,
and #11220 moved it 13% the other way.

<details>
<summary>CodSpeed results for #10676 with and without this PR</summary>

The first column is #11220, which compares the #10676 commits on
`develop` with `develop` (`c9a3fbe`). The second column is the #10676
report, which compares the same commits on this PR with this PR
(`b0a64dd`). Against `develop` (`bc58587`), this PR flags none of these
benchmarks. CodSpeed marks the walltime runners as unreliable, and the
two columns come from different runs.

| Benchmark | Mode | Without this PR | With this PR |
| --- | --- | ---: | ---: |
| `int_gt[16]` | CPU simulation | 15.6% slower | not flagged |
| `int_gt[1024]` | CPU simulation | 14.5% slower | not flagged |
| `float_gt[16]` | CPU simulation | 12.2% slower | not flagged |
| `float_gt[1024]` | CPU simulation | 12.1% slower | not flagged |
| `or_chain[16]` | CPU simulation | 12.3% slower | not flagged |
| `or_chain[1024]` | CPU simulation | 11.9% slower | not flagged |
| `is_not_null_pred[16]` | CPU simulation | 11.6% slower | not flagged |
| `is_not_null_pred[1024]` | CPU simulation | 10.5% slower | not flagged
|
| `i64_lt` | CPU simulation | 11.7% slower | not flagged |
| `timestamp_gte` | CPU simulation | 10.3% slower | not flagged |
| `bench_compare_sliced_dict_primitive[(1000, 10000)]` | CPU simulation
| 11.5% slower | not flagged |
| `encode_primitives[f32, (2000, 2)]` | CPU simulation | 12.8% slower |
not flagged |
| `compare_u8_avx512` | Walltime | 14.4% slower | not flagged |
| `compare_u8_neon` | Walltime | 13.3% slower | not flagged |
| `compare_f32_avx512` | Walltime | 10.4% slower | not flagged |
| `compare_float_avx2` | Walltime | 18.4% faster | 28.2% faster |
| `compare_u64_avx2` | Walltime | 15.7% faster | 30.8% faster |
| `lt_i64_nullable_avx2` | Walltime | not flagged | 18.4% faster |
| `compare_int_constant_neon` | Walltime | not flagged | 16.2% faster |
| `compare_int_eq_avx2` | Walltime | not flagged | 15.4% faster |
| `compare_f32_avx2` | Walltime | not flagged | 15.4% faster |
| `compare_int_constant_avx2` | Walltime | not flagged | 15.3% faster |
| `compare_int_avx2` | Walltime | not flagged | 13.5% faster |
| `compare_int_nullable_avx2` | Walltime | not flagged | 12.5% faster |

</details>

<details>
<summary>AVX-512 investigation on EC2</summary>

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_avx512` case | `develop` | Before fix | With fix |
| --- | ---: | ---: | ---: |
| `ConstantLhs, PartialAccepted` | 32.1 µs | 1.354 | 1.025 |
| `Columns, PartialAccepted` | 37.7 µs | 1.159 | 1.018 |

Before the fix, the kernel executed 15% more instructions per row. LLVM
inline remarks show `collect_bool_words_avx512` inlined on `develop` and
"definition is unavailable" on this branch.

Across all 100 `#[cpu_features]` cases, the fix also returns
`compare_string_lt` to the `develop` speed. It makes two
`intersect_by_rank` cases and one `starts_with` case 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.

</details>

<details>
<summary>Historical EC2 framework measurements</summary>

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
D `0291778db36809f77ce0219bf9850506d3cba3b6`.

| Workload | C7g NEON | C6a AVX2 | C7i AVX2 build | C7i AVX-512 build |
| --- | ---: | ---: | ---: | ---: |
| `u64` multiply, 8,192 rows | 1.732 | 0.979 | 0.992 | 0.998 |
| Nullable `i64` add, 32,768 rows | 1.006 | 1.004 | 1.154 | 1.005 |

The 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.

</details>

The separately confirmed NEON comparison gains were published in #10843,
which has merged. They do not measure this framework.

CI passed on `b0a64dd`, including all-target, all-feature Clippy with
warnings denied and the Linux, Windows, sanitizer, and Miri test jobs.
The CodSpeed check fails on the flags described above.

---------

Signed-off-by: Connor Tsui <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/performance A performance improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants