Skip to content

Compare primitive values through the shared RowFn executor - #10676

Draft
connortsui20 wants to merge 1 commit into
ct/row-fn-cleanup/fused-executionfrom
ct/row-fn-cleanup/primitive-compare
Draft

connortsui20 wants to merge 1 commit into
ct/row-fn-cleanup/fused-executionfrom
ct/row-fn-cleanup/primitive-compare

Conversation

@connortsui20

@connortsui20 connortsui20 commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

Summary

Replaces the primitive comparison migration in #9979, using the shared executor from #10675. This PR depends on #10675 and has been replayed onto its rebased head 7c47d4bcfee3370684c1b22de999e61f66628afa.

Reopened as a draft. The controlled measurements showed substantial AVX2 gains as well as ARM and AVX-512 regressions. They justify further work before merging, but do not establish that this migration has no benefit. #10741 isolates some float32 packing gains without this migration, while #10742 explores a smaller validity change. Neither is a complete replacement for every observed benefit here.

Changes

The private row kernel selects the primitive type and comparison operator. The framework owns borrowed inputs, constant handling, strict validity, and packed Boolean output. Removes the primitive operand wrapper and handwritten primitive kernels while preserving the public Binary identity and Vortex's floating-point total ordering.

Native Graviton3 measurements showed regressions when this migration used only #10673. Removing only the fused-execution patch while keeping all three foundations also slowed the tested comparisons. The dependency on #10675 keeps that execution work available when this migration merges.

Existing comparison tests exercise the shared executor. Source range-diff review confirms that the migration patch is unchanged by the rebase. Tests, formatting, and benchmarks were not rerun on the rebased head, so earlier measurements are not validation of this revision.

@connortsui20
connortsui20 added this pull request to stack #10677 October 21, 2026 15:10
@codspeed

codspeed Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Merging this PR will regress 9 benchmarks

4 warnings, 1 note

⚠️ 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

⚠️ Unable to measure the execution of 1 benchmark

This benchmark measured no execution time, so its result is not comparable and counts as unchanged. This usually happens when the compiler removes the code under test.

Prevent compiler optimizations

See affected benchmarks

⚠️ 18 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

ℹ️ 40 benchmarks have no flamegraph

The flamegraph wasn't generated: the symbols of the profiling trace could not be resolved, because no information about the language frames was collected.

See affected benchmarks

⚡ 21 improved benchmarks
❌ 9 regressed benchmarks
✅ 2378 untouched benchmarks
⏩ 12 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
❌ Walltime filter_neon[80] 2.5 µs 3.2 µs -20.79%
❌ CPU Simulation filter_powerlaw_by_mostly_false[250000] 137.4 µs 158.7 µs -13.39%
❌ CPU Simulation density_sweep_dense_runs[0.001] 52.3 µs 59.5 µs -12.16%
❌ Walltime filter_neon[64] 1.8 µs 2 µs -11.52%
❌ CPU Simulation filter_random_by_mostly_false[250000] 162.5 µs 183.1 µs -11.26%
❌ Walltime bitpack_blocked_compress_avx2 6.7 µs 7.6 µs -11.04%
❌ CPU Simulation filter_powerlaw_by_random[250000] 152.5 µs 170.5 µs -10.56%
❌ CPU Simulation filter_random_by_power_law[250000] 165.6 µs 184.7 µs -10.34%
❌ CPU Simulation filter_powerlaw_by_power_law[250000] 165.8 µs 184.8 µs -10.31%
⚡ Walltime compare_f32_avx2 4.5 µs 2.6 µs +74.38%
⚡ Walltime compare_f32_avx512 3.5 µs 2.4 µs +44.87%
⚡ Walltime compare_float_avx2 4.9 µs 3.9 µs +27.91%
⚡ Walltime compare_f32_neon 5.2 µs 4.1 µs +26.4%
⚡ CPU Simulation chunked_dict_primitive_canonical_into[f64, (1000, 1000, 10)] 191.4 µs 151.9 µs +26.07%
⚡ Walltime compare_int_constant_neon 4 µs 3.3 µs +21.29%
⚡ Walltime compare_u64_avx2 3.5 µs 2.9 µs +20.35%
⚡ CPU Simulation missing_stats[16] 85.4 µs 71.7 µs +19%
⚡ Walltime compare_int_neon 4.6 µs 3.9 µs +17.64%
⚡ Walltime compare_int_eq_neon 4.6 µs 4 µs +15.19%
⚡ Walltime compare_u64_neon 4.5 µs 3.9 µs +14.09%
... ... ... ... ... ...

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.

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/primitive-compare (3583fc2) with ct/row-fn-cleanup/fused-execution (7c47d4b)

Open in CodSpeed

Footnotes

  1. 12 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@connortsui20
connortsui20 force-pushed the ct/row-fn-cleanup/primitive-compare branch from dee1a57 to 3a38935 Compare October 22, 2026 14:09
@connortsui20
connortsui20 removed this pull request from stack #10677 October 22, 2026 14:10
@connortsui20
connortsui20 changed the base branch from ct/row-fn-cleanup/fused-execution to ct/row-fn-cleanup/batch-inputs October 22, 2026 14:10
@connortsui20
connortsui20 added this pull request to stack #10695 October 22, 2026 14:11
@connortsui20
connortsui20 force-pushed the ct/row-fn-cleanup/primitive-compare branch from 3a38935 to 33ccf9b Compare October 22, 2026 14:49
@connortsui20 connortsui20 added the changelog/performance A performance improvement label Oct 22, 2026
@connortsui20
connortsui20 removed this pull request from stack #10695 October 22, 2026 15:12
@connortsui20
connortsui20 force-pushed the ct/row-fn-cleanup/primitive-compare branch from 33ccf9b to b837021 Compare October 22, 2026 15:13
@connortsui20
connortsui20 changed the base branch from ct/row-fn-cleanup/batch-inputs to ct/row-fn-cleanup/fused-execution October 22, 2026 15:13
@connortsui20
connortsui20 added this pull request to stack #10697 October 22, 2026 15:13

connortsui20 commented Oct 23, 2026 •

Copy link
Copy Markdown
Member Author

Edit: reopened as a draft. The controlled measurements showed substantial AVX2 gains as well as ARM and AVX-512 regressions. Those results justify holding the PR for further work, but do not establish that the migration has no benefit. The earlier closure treated those two conclusions as equivalent.

#10741 isolates a useful float32 packing policy, and #10742 explores a smaller validity change. Neither replaces every benefit observed in the migration. Both remain independent drafts with their own measurement limits.

#10675 and this PR are being rebased while preserving their dependency. The saved measurements are historical evidence, not validation of the rebased heads.

Keep the primitive kernel focused on type and operator dispatch. Delegate borrowed inputs, constants, strict validity, and packed Boolean output to the lower stack layers while preserving Binary identity and floating-point total ordering.

Replaces the comparison migration in #9979.

Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
@connortsui20
connortsui20 force-pushed the ct/row-fn-cleanup/primitive-compare branch from b837021 to 3583fc2 Compare October 23, 2026 17:53

This branch has not been deployed

No deployments
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.

1 participant