Repository navigation
Execute filter scalar fns' inputs iteratively with execute_step - #11154
Conversation
Binary (comparisons, arithmetic, AND/OR), Like, Between and FillNull now implement execute_step. Instead of executing their inputs themselves, which starts a nested execute_until per input, they ask the executor for each input as columnar (canonical for Between) and are stepped again once it is. An expression tree of these functions therefore runs in one execute_until, without recursion. Behaviour is unchanged: - AND/OR still answer from a deciding constant first, so `false AND x` never executes `x`, and a null constant operand still decides a comparison or arithmetic result without executing the other side. - execute_parent kernels still run before an input is forced, so compressed-domain kernels keep firing. Between's execute-parent adaptors now read only the lower and upper bounds. The executor hands a suspended parent over with the executing input's slot detached, and reading every child panicked on it; vortex-alp's patched_matches_primitive hit this once Between executed its input in a slot. Each migrated function gets a test that executes non-columnar inputs, and `false AND x` is tested with an `x` that fails if executed. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01UeniBEMPLG2FLT3ZFA1auB Signed-off-by: Joe Isaacs <[email protected]>
Polar Signals Profiling ResultsLatest Run
Previous Runs (7)
Powered by Polar Signals Cloud |
Polar Signals Profiling ResultsLatest Run
Powered by Polar Signals Cloud |
Benchmarks: PolarSignals Profiling 📖Commits: PR datafusion / vortex-file-compressed / ns (1.045x ➖, 0↑ 1↓)
|
Benchmarks: FineWeb NVMe 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.012x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.993x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.992x ➖, 1↑ 0↓)
duckdb / parquet / ns (0.988x ➖, 0↑ 0↓)
|
Benchmarks: TPC-H SF=1 on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.997x ➖, 0↑ 0↓)
datafusion / parquet / ns (1.000x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.990x ➖, 0↑ 0↓)
duckdb / parquet / ns (0.996x ➖, 0↑ 0↓)
|
Benchmarks: TPC-H SF=10 on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.997x ➖, 0↑ 0↓)
datafusion / parquet / ns (1.000x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.992x ➖, 0↑ 0↓)
duckdb / parquet / ns (0.997x ➖, 0↑ 0↓)
|
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.998x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.001x ➖, 0↑ 0↓)
duckdb / parquet / ns (1.004x ➖, 0↑ 1↓)
|
Benchmarks: Clickbench on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.993x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.994x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.999x ➖, 0↑ 0↓)
duckdb / parquet / ns (0.996x ➖, 1↑ 0↓)
|
Benchmarks: TPC-H SF=1 on S3 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.948x ➖, 2↑ 0↓)
datafusion / parquet / ns (0.972x ➖, 2↑ 1↓)
duckdb / vortex-file-compressed / ns (1.165x ➖, 1↑ 6↓)
duckdb / parquet / ns (1.038x ➖, 0↑ 3↓)
|
Merging this PR will improve performance by 19.57%2 warnings
Performance Changes
Tip Curious why performance improved? Comment Comparing Footnotes
|
Rework the migration to a minimal diff. Each migrated function's execute is back to develop's version untouched; its new execute_step only asks for the inputs it needs with the new require_input! macro, the scalar function counterpart of require_child!, and then calls that unchanged execute: - Binary requests both operands as columnar, unless a constant operand already decides the result (`false AND x`, `x OR true`, a null comparison operand), so the other operand is still never executed. - Like requests its input and pattern as columnar, unless the pattern is a null constant. - Between requests its input as canonical. - FillNull requests its input as columnar once the fill is constant, so a lazy fill is still materialized by execute before the input's FillNull kernels run. The execute_step doc now allows keeping the logic in execute and calling it from execute_step instead of panicking in execute. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01UeniBEMPLG2FLT3ZFA1auB Signed-off-by: Joe Isaacs <[email protected]>
CodSpeed flagged the scalar_fn probe benchmarks on Binary (bool AND about 14% slower). A probe executes the function over constant single-row inputs, so the per-call cost of execute_step dominates: - Binary's AND/OR check called constant_boolean, which with two constant operands computed the whole result only for execute to compute it again. It now only checks whether an operand is a constant, and skips the check entirely when both operands are already columnar. - Every check cloned its input through ExecutionArgs::get. A new provided method, ExecutionArgs::input_matches, tests an input in place; VecExecutionArgs and the row-function args override it to avoid the clone, and require_input!, Binary, Like and FillNull use it. Locally, interleaved runs of the probe benchmarks against develop go from up to +30% to within +4%, except valid_repeated on bool at about +14% (under 15 ns per call). Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01UeniBEMPLG2FLT3ZFA1auB Signed-off-by: Joe Isaacs <[email protected]>
## Summary Follow-up to #11110. Keep the remaining noisy cases out of automatic CodSpeed runs while preserving them for local benchmarks. The Python scalar exclusions in #11110 never took effect because the runner exports simulation as `instrumentation`, while the fixture checked for `simulation`. Use the plugin's resolved mode so these cases are actually skipped. ## Changes - Allow a trailing comma in `#[divan::bench(...)]` when the CPU-feature macro appends its options. The multiline bitpacked take definition covers this case in CI builds. - Share the resolved simulation check between the scalar and filter fixtures. Also skip the 2 integer one-shot filtered scans that still report system calls under simulation. - Keep AVX2 bitpacked take with 80 selections local after batching still produced a regression alert. Retain the 64-selection case and both AVX-512 cases in CI. - Keep AVX-512 nullable integer comparison local because its minimum changes by more than 10% while its median stays stable. - Keep the small random power-law boolean filter and the large correlated power-law boolean filter local under simulation. The former has repeated unexplained changes, and the latter repeatedly makes system calls that simulation cannot measure. <details> <summary>Evidence from recent PR reports</summary> Reviewed 40 CodSpeed report comments updated between November 2 at 19:28 UTC and November 3 at 19:28 UTC, including their comparison revisions and the affected measurement modes. | Case | Evidence | | --- | --- | | Python integer scalar scans | [#11154](#11154 (comment)) still reports the repeated scan's 10 system calls. [A develop run after #11110](https://github.com/vortex-data/vortex/actions/runs/41677647530/job/126062463160) executes both integer scalar cases instead of skipping them. The [runner exports the legacy `instrumentation` value](https://github.com/CodSpeedHQ/codspeed/blob/main/crates/runner-shared/src/measurement_mode.rs), which pytest-codspeed 5.0.3 normalizes to simulation. | | Python integer filtered scans | [#11130](#11130 (comment)) reports 30 system calls totaling 14.3 ms in `test_scan_filter[int]`. [#11138](#11138 (comment)) reports 30 calls totaling 16.3 ms in `test_scan_filter_to_arrow_array[int]`. | | AVX2 `threshold[80]` | [#11004](#11004 (comment)) compares `f7b5b1d864` with `e59b236d7b`: 2.183 to 2.531 µs, reported as a 13.75% regression. Both use 1000 samples of 16 iterations. The head median is 2.556 µs, so this is not just a single outlying minimum. The PR does not change the fastlanes code or benchmark. | | AVX-512 `lt_i64_nullable` | [#11168](#11168 (comment)) reports 7.342 to 8.217 µs. Several unrelated CUDA PRs report the opposite direction, including [#10872](#10872 (comment)), at 8.208 to 7.348 µs. Their head medians remain around 8.3 to 8.5 µs. | | Boolean `filter_powerlaw_by_random[10000]` | [#11167](#11167 (comment)) reports a 16.2% regression, [#11168](#11168 (comment)) a 23.56% improvement, and [#11169](#11169 (comment)) a 13.06% regression. The cause is unknown. | | Boolean `filter_powerlaw_by_correlated_runs[100000]` | [#10871](#10871 (comment)) and [#11165](#11165 (comment)) each report 2 system calls totaling about 44 µs. | The NEON chunked-take alerts on #11090 are on a PR that changes execution substantially, so those cases remain enabled. Other dictionary simulation alerts need more attribution before excluding them. </details> CI passes formatting, Python lint and script tests, Rust lint, default-feature and all-feature Rust builds, all 3 CPU walltime benchmark jobs, and Python benchmarks (36 passed, 4 intentionally skipped). The previously failed simulation shard also passes. Some remaining CI jobs and CodSpeed performance analysis are still pending. No local checks were run. The pre-commit and pre-push hooks were bypassed as requested. --------- Signed-off-by: Connor Tsui <[email protected]>
## Summary Stacked on #11310. Until that merges, this PR shows only the last commit. `Lower` and `Strpos` decode their inputs inside `execute`, in a nested executor. When the input is a slice or a mask over a run-end or chunked array, the function gets a canonical array and runs once per row, even when the encoding could have pushed it into its values. #11310 lets the executor retry parent-reduce rules once a child is rewritten, but only for functions that hand their inputs back to the executor through `execute_step`. This continues the `execute_step` migration from #11100 and #11154 for `Lower` and `Strpos`. ## Changes - `Lower::execute_step` and `Strpos::execute_step` request their inputs with `require_input!(args, i => AnyColumnar)`, as `Like` and `Between` do, then call `execute`. - Tests: `lower`'s `preserves_runs_and_dictionary` gets a sliced case, and `strpos` gets `preserves_runs_of_a_slice`. Both check that a function over a sliced run-end input stays run-end encoded. Both fail without this change, and both fail without #11310. Checks: - `cargo test -p vortex-array` passes. - `cargo clippy -p vortex-array --all-targets --all-features -- -D warnings` passes. - `cargo +nightly-2026-09-10 fmt -p vortex-array --check` passes. Benchmark numbers will follow. No existing benchmark covers scalar functions over sliced or masked encoded inputs. This change was written with help from an AI coding agent and I haven't reviewed it yet. --------- Signed-off-by: Robert Kruszewski <[email protected]> Signed-off-by: Alfonso Subiotto <[email protected]> Co-authored-by: Robert Kruszewski <[email protected]> Co-authored-by: Claude Opus 5.5 <[email protected]> Co-authored-by: Amp <[email protected]> Co-authored-by: Robert Kruszewski <[email protected]>
Summary
This follows #11100, which added
ScalarFnVTable::execute_stepand migratedNot. It migrates the scalar functions that make up scan filters:Binary(comparisons, arithmetic,AND/OR),Like,BetweenandFillNull.