Skip to content

Split up binary ScalarFn#6966

Draft
gatesn wants to merge 4 commits intodevelopfrom
ngates/split-binary-fn
Draft

Split up binary ScalarFn#6966
gatesn wants to merge 4 commits intodevelopfrom
ngates/split-binary-fn

Conversation

@gatesn
Copy link
Contributor

@gatesn gatesn commented Mar 14, 2026

Edit: im not convinced of this PR, but the benchmarks have surfaced some interesting results. So digging in


Adds new scalar functions for comparison, arithmetic, and logical operators.

Previously the single binary function typically just switched on the logic for each of these, making it much harder to read and follow.

This also gives us more granular kernel selection, e.g. for arrays that wish to provide a comparison kernel we would previously have to invoke the kernel for any binary function.

gatesn added 4 commits March 13, 2026 21:08
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
/// Adaptor that bridges [`CompareKernel`] implementations to [`ExecuteParentKernel`] for the
/// new [`Comparison`] scalar function.
#[derive(Default, Debug)]
pub struct ComparisonCompareExecuteAdaptor<V>(pub V);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hate this. we should just rewrite into the new kernel traits

@codspeed-hq
Copy link

codspeed-hq bot commented Mar 14, 2026

Merging this PR will degrade performance by 92.63%

⚡ 6 improved benchmarks
❌ 10 regressed benchmarks
✅ 993 untouched benchmarks
⏩ 1515 skipped benchmarks1

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation pushdown_compare[(1000, 64, 4)] 473.7 µs 390.6 µs +21.26%
Simulation pushdown_compare[(1000, 4, 4)] 472.8 µs 217.2 µs ×2.2
Simulation pushdown_compare[(1000, 16, 8)] 495.4 µs 253.4 µs +95.55%
Simulation pushdown_compare[(1000, 16, 4)] 473.3 µs 243.2 µs +94.63%
Simulation pushdown_compare[(10000, 16, 8)] 682.4 µs 1,548.1 µs -55.92%
Simulation pushdown_compare[(1000, 4, 8)] 474.5 µs 220.6 µs ×2.2
Simulation pushdown_compare[(10000, 16, 4)] 634 µs 1,438.4 µs -55.92%
Simulation pushdown_compare[(10000, 4, 4)] 634 µs 1,170.5 µs -45.84%
Simulation pushdown_compare[(1000, 64, 8)] 484.4 µs 434 µs +11.63%
Simulation pushdown_compare[(10000, 64, 8)] 644.5 µs 3,353.9 µs -80.78%
Simulation pushdown_compare[(10000, 4, 8)] 643.2 µs 1,190.8 µs -45.98%
Simulation pushdown_compare[(10000, 64, 4)] 633.6 µs 2,913.4 µs -78.25%
Simulation old_alp_prim_test_between[f64, 16384] 332.9 µs 372.3 µs -10.59%
Simulation old_alp_prim_test_between[f64, 32768] 541.4 µs 609.8 µs -11.2%
Simulation eq_pushdown_low_match 2.3 ms 30.9 ms -92.63%
Simulation eq_pushdown_high_match 2.4 ms 31.3 ms -92.34%

Comparing ngates/split-binary-fn (8d922f4) with develop (fc4d111)

Open in CodSpeed

Footnotes

  1. 1515 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant