fix(array): honor the execution allocator for RowFn outputs - #10014
connortsui20 wants to merge 5 commits into
Conversation
Merging this PR will degrade performance by 11.73%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | take[duplicates/repeated/primitive/nonnull/chunks=16/indices=1000] |
28.7 µs | 102.8 µs | -72.13% |
| ❌ | WallTime | filtered_owned_i64_avx512[OneNullInEight] |
22.3 µs | 27.7 µs | -19.29% |
| ❌ | WallTime | filtered_owned_i64_avx2[OneNullInEight] |
22 µs | 25.6 µs | -13.91% |
| ❌ | WallTime | decode_avx2[8192, (External, AllValid)] |
151.5 µs | 173.5 µs | -12.7% |
| ❌ | WallTime | decode_avx2[8192, (External, OneNullInEight)] |
141.5 µs | 160.6 µs | -11.86% |
| ❌ | Simulation | allocate_drop_arrow[65536] |
90.7 µs | 101.9 µs | -11% |
| ❌ | Simulation | non_nullable[256] |
195.5 µs | 218.4 µs | -10.46% |
| ⚡ | WallTime | mul_u64_nonnull_neon |
40 µs | 29.1 µs | +37.75% |
| ⚡ | WallTime | mul_i64_nonnull_neon |
38.5 µs | 32.8 µs | +17.45% |
| ⚡ | WallTime | multiply_shapes_neon[(32768, PerRowPerRow)] |
38.6 µs | 32.9 µs | +17.32% |
| ⚡ | WallTime | filtered_sink_i64_avx512[OneNullInEight] |
26.4 µs | 23.5 µs | +12.44% |
| Simulation | fixed_16_advancing_ptr_safe[100] |
< 1 ns | < 1 ns | N/A | |
| Simulation | preverify_advancing_ptr_unchecked[1000] |
< 1 ns | < 1 ns | N/A | |
| Simulation | preverify_advancing_ptr_unchecked[10000] |
< 1 ns | < 1 ns | N/A |
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-output-allocator (e1ca00c) with develop (2ab66d7)2
Footnotes
-
385 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. ↩
-
No successful run was found on
develop(ad6ee9d) during the generation of this report, so 2ab66d7 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩
7042d42 to
573b85f
Compare
573b85f to
4aef610
Compare
6bf049e to
a200209
Compare
a200209 to
7da447a
Compare
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
7da447a to
e1ca00c
Compare
Depends on #10016.
RowFn output payloads bypass the execution allocator. This change allocates them directly through
ctx.allocator()across owned, deferred-retry, selected, filtered, constant, and sink execution, while preserving zero-copy primitive publication and empty-output paths.OutputElementchooses its collection storage through an associated buffer type and an allocation hook. The executor writes throughOutputBufferslots, and the buffer implementation constructs the array. Vortex primitive and Boolean implementations useBufferMut, while scalar and fixed-size-list sinks use the same storage contract. UTF-8 descriptors, external bytes, and polygon payloads also use the execution allocator. Physical sink parameters remain separate from allocation resources.Regressions check ownership of returned payloads using canonical inputs prepared before allocation tracking, including a context override, constant UTF-8 output, retry execution, and zero-copy reuse. A zero-sized output with
Vecstorage exercises the owned execution paths without requiringBufferMut. Boolean collector selection is preserved.The Boolean dense-retry path from merged #9986 also uses the execution allocator, with coverage in the existing packed-output allocator test.
Validation before the rebase: 168 focused comparison, mask, and RowFn tests passed on the combined stack through #9979, along with
cargo clippy -p vortex-array --all-targets --all-features -- -D warnings. Tests, formatting, and benchmarks were not rerun locally after the rebase.