Skip to content

GH-50645: [C++][Compute] Optimize min/max_element_wise with word at a time validity handling#50647

Open
Kanishk-Dendukuri wants to merge 1 commit into
apache:mainfrom
Kanishk-Dendukuri:opt-minmax-element-wise
Open

GH-50645: [C++][Compute] Optimize min/max_element_wise with word at a time validity handling#50647
Kanishk-Dendukuri wants to merge 1 commit into
apache:mainfrom
Kanishk-Dendukuri:opt-minmax-element-wise

Conversation

@Kanishk-Dendukuri

@Kanishk-Dendukuri Kanishk-Dendukuri commented Jul 26, 2026

Copy link
Copy Markdown

Rationale for this change

The variadic min_element_wise / max_element_wise kernels (ScalarMinMax in scalar_compare.cc) fold arguments using a multi pass approach: an antiextreme sentinel fill, a separate validity bitmap pass (BitmapOr/BitmapAnd), and a per element value pass with a per-element validity branch.

This can be rewritten as a single word at a time pass (handling validity 64 bits at a time) that fuses validity and value computation and keeps fully-valid words branchless. That removes the redundant passes and the per-element mispredicting branch, giving a measured ~2-7x speedup on numeric types, with no behavioral change.

What changes are included in this PR?

Rewrote the numeric ScalarMinMax fold to a word at a time algorithm (CombineWordwise / CombineArrays / FoldArrayIntoOutput), with an all-valid branchless fast path and an all null skip.

Added a random, sliced/offset test (MinMaxElementWiseRandom) covering the new word loop across null densities and offsets.

Benchmarks

archery benchmark diff (this branch vs main), 5 reps; throughput, change %:

benchmark null baseline contender change %
ArrayArray Int64 50% 285.338M/s 2.429G/s +751.4%
ArrayArray Int64 0.01% 808.895M/s 3.594G/s +344.3%
ArrayArray Int64 1% 645.295M/s 2.823G/s +337.5%
ArrayArray Int64 10% 610.443M/s 2.433G/s +298.5%
ArrayArray Int64 0% 1.422G/s 5.447G/s +283.1%
ArrayArray Int64 100% 5.696G/s 14.671G/s +157.6%
ArrayScalar Int64 50% 1.075G/s 2.748G/s +155.7%
ArrayScalar Int64 1% 1.657G/s 3.387G/s +104.4%
ArrayScalar Int64 10% 1.628G/s 2.758G/s +69.4%
ArrayScalar Int64 0.01% 2.300G/s 3.817G/s +66.0%
ArrayScalar Int64 0% 2.482G/s 3.782G/s +52.4%
ArrayScalar Int64 100% 8.625G/s 10.134G/s +17.5%

Are these changes tested?

Yes. All existing ElementWise tests pass, plus a new MinMaxElementWiseRandom typed test that cross checks the kernel against a reference across lengths (65/128/200), null densities (0–100%), offsets (0/1/3/7), both skip_nulls, and min/max, over all numeric types.

Are there any user-facing changes?

No

AI Usage

Parts of the code, tests, and this description were written with AI assistance.

Closes #50645

@Reranko05 Reranko05 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you run python -m pre_commit run clang-format --files cpp/src/arrow/compute/kernels/scalar_compare.cc cpp/src/arrow/compute/kernels/scalar_compare_test.cc to fix the formatting issues?

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[C++][Compute] Optimize min/max_element_wise with word at a time validity handling

3 participants