Skip to content

Compute Widen aggregates on the values - #10890

Merged
connortsui20 merged 12 commits into
developfrom
ct/narrow-aggregate-kernels
Oct 28, 2026
Merged

connortsui20 merged 12 commits into
developfrom
ct/narrow-aggregate-kernels

Conversation

@connortsui20

@connortsui20 connortsui20 commented Oct 27, 2026 •

Copy link
Copy Markdown
Member

Summary

Every aggregate on a Widen currently widens the values to the logical type and then runs the canonical accumulator. This PR runs MinMax, Min, Max, IsConstant, IsSorted and Sum on the narrow values instead, so they read the values without a widened copy and keep the values' own cached statistics and kernels.

Decoding a Widen value is a strictly increasing, one-to-one map that keeps validity, so these aggregates give the same answer on the values as on the Widen, and every value a partial holds widens exactly. One kernel runs the aggregate on the values and casts its partial to the logical dtype. Because the values run the same accumulator code as a canonical array, edge cases such as an empty batch or a single null match by construction. The kernel serves only integer logical types and only these aggregates, which are registered by their exact ids.

Based on #10888.

Changes

Sum needs two more conditions. The accumulator must sum each batch from zero and add the total to the running sum, which it does only for Sum with the legacy Stat::Sum slot (Sum with skip_nans). SumV2 and Sum without skip_nans add each value to the running sum, and a batch total cannot match that near the 64-bit bounds. The sum must also be unable to overflow: Sum adds every integer type into a 64-bit sum but checks for overflow at different points for different widths, so the kernel serves only batches of at most 2^(64 − b) rows for values of b bits, where no check can fire.

Mask::last no longer panics on an empty all-true mask, which the new tests reach through Last on an empty batch.

The tests accumulate each aggregate over batches encoded as Widen and as canonical arrays, and compare every kernel partial, running partial and final result. A vortex-fastlanes integration test covers a BitPacked child.

@codspeed

codspeed Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Merging this PR will regress 3 benchmarks

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

⚠️ 1 benchmark is 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

⚡ 3 improved benchmarks
❌ 3 regressed benchmarks
✅ 2264 untouched benchmarks
⏩ 370 skipped benchmarks1
🗄️ 30 archived benchmarks run2

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
❌ CPU Simulation filter_powerlaw_by_random[10000] 232.8 ns 808.1 ns -71.19%
❌ CPU Simulation compress_alp[f32, (1000, 0.1, 0.25)] 114.5 µs 129.3 µs -11.43%
❌ CPU Simulation take[(16, 8, 1)] 108 µs 121.6 µs -11.23%
⚡ CPU Simulation sum[widen] 539.8 µs 108.7 µs ×5
⚡ CPU Simulation min_max[widen] 450.4 µs 132.6 µs ×3.4
⚡ Walltime starts_with_avx512[64, VarBin] 60.2 µs 53.5 µs +12.54%

Tip

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


Comparing ct/narrow-aggregate-kernels (7dd6ba4) with develop (2fb259b)

Open in CodSpeed

Footnotes

  1. 370 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. ↩

  2. 30 benchmarks were run, but are now archived. If they were deleted in another branch, consider rebasing to remove them from the report. Instead if they were added back, click here to restore them. ↩

@connortsui20
connortsui20 marked this pull request as ready for review October 27, 2026 18:53
connortsui20 added a commit that referenced this pull request Oct 27, 2026
A Narrow aggregate kernel can now run the aggregate on the child and widen
its partial, for the integer aggregates where decoding gives the same
result. The NR3 brief notes the design that landed.

Signed-off-by: Connor Tsui <[email protected]>
return Ok(None);
};

// NB: Exactness rests on integer decoding. A decimal `Narrow` (CD4 in #10733) needs its own

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.

this is a weird comment to leave

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It's because I need to make this change as soon as this stack merges. I can remove it but Im going to change it very soon

@robert3005 robert3005 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.

Not sure where the ls

@connortsui20
connortsui20 force-pushed the ct/narrow-aggregate-kernels branch from 82532e4 to 0635333 Compare October 28, 2026 15:39
@connortsui20 connortsui20 changed the title Compute Narrow aggregates on the values Compute Widen aggregates on the values Oct 28, 2026
@connortsui20
connortsui20 force-pushed the ct/narrow-aggregate-kernels branch from 0635333 to e978e6f Compare October 28, 2026 15:54
@connortsui20
connortsui20 force-pushed the ct/narrow-aggregate-kernels branch from e978e6f to 15b34ec Compare October 28, 2026 16:17
Base automatically changed from ct/narrow-benchmarks to develop October 28, 2026 16:42
Move the private `sum_state_scalar` of `SumV2` to `SumState::to_scalar`,
so that `Sum`, `SumV2` and encoding kernels build the sum scalar the same
way. Document the contracts of `accumulate_primitive` and
`AggregateFnVTable::accumulate`.

No behavior changes.

Signed-off-by: Connor Tsui <[email protected]>
`Mask::last` computed `len - 1` eagerly inside `then_some`, so an empty all-true mask panicked
with a subtraction overflow in debug builds. The `Last` aggregate reaches it for every empty batch
with all-valid validity.

Signed-off-by: Connor Tsui <[email protected]>
Add aggregate kernels for `Widen` that build the exact partial of the
logical dtype from the values, so that `MinMax`, `Min`, `Max`,
`IsConstant`, `IsSorted` and `Sum` with `skip_nans` no longer widen the
values first. The extrema and verdict kernels ask the values and decode
any value they return. The sum kernel sums the values into the sum state
of the logical dtype. It declines values that have a sum kernel of their
own, exact or fallback (`RunEnd`, `Sparse`, `Chunked`), constant values
and batches of more than 2^32 rows. Every other aggregate keeps the
default path.

The kernels are registered in `AggregateFnSession::default` for their
exact aggregate ids. Scalar access and the kernels share one decode
function.

Signed-off-by: Connor Tsui <[email protected]>
Accumulate each aggregate over batches encoded as `Widen` and as
canonical arrays, and check every kernel partial, running partial and
result. Each consumer builds its own batch, so no accumulator reads a
statistic that encoding or another accumulator cached. The cases cover
the child bounds, nullable and non-nullable logical dtypes, batch
boundaries, empty and all-null batches, a single null under strict
`IsSorted`, a running sum overflow, and the sum declining constant and
chunked values. Add a `vortex-fastlanes` integration test with a
`BitPacked` child.

Signed-off-by: Connor Tsui <[email protected]>
Replace the MinMax, IsConstant, IsSorted and Sum kernels of Widen with
one kernel that runs the same aggregate on the values and casts the
partial to the partial dtype of the logical dtype. Decoding is strictly
increasing, one-to-one and keeps validity, so the partials agree, and the
values keep their own cached statistics and aggregate kernels.

Sum is served only when the aggregate has the Stat::Sum slot, so that
each batch is summed from zero, and when the batch has at most 2^32
rows, so that the per-block and per-value overflow checks agree.

Signed-off-by: Connor Tsui <[email protected]>
The aggregate kernel no longer decodes scalars, so scalar access is the only caller. Move the function beside it in the operations vtable.

Signed-off-by: Connor Tsui <[email protected]>
The Widen aggregate kernel runs Sum through its accumulator and no longer builds a sum state itself. Restore SumV2's private sum_state_scalar, the Sum to_scalar match and the doc lines that only the old Widen sum kernel needed.

Signed-off-by: Connor Tsui <[email protected]>
State the Sum overflow bound for 64-bit logical dtypes only, explain that skip_nans matters for integer sums only because it selects the Stat::Sum slot, and say why the chunked values test compares results.

Signed-off-by: Connor Tsui <[email protected]>
A logical type narrower than 64 bits is summed in the same blocks as its
values, so its overflow checks agree at any length.

Signed-off-by: Connor Tsui <[email protected]>
Sum adds every integer type into a 64-bit sum, so values of b bits cannot
overflow it within 2^(64 - b) rows, whatever the logical width. One rule
replaces the fixed 2^32 bound and the special case for 64-bit logical
types.

Signed-off-by: Connor Tsui <[email protected]>
@connortsui20
connortsui20 force-pushed the ct/narrow-aggregate-kernels branch from 15b34ec to 7dd6ba4 Compare October 28, 2026 16:42
@connortsui20
connortsui20 merged commit 37031b1 into develop Oct 28, 2026
86 of 88 checks passed
@connortsui20
connortsui20 deleted the ct/narrow-aggregate-kernels branch October 28, 2026 17:03
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