Compare primitive values with RowFn - #9979
connortsui20 wants to merge 1 commit into
Conversation
Merging this PR will degrade performance by 11.94%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | WallTime | mul_u64_nonnull_neon |
28.7 µs | 40.6 µs | -29.4% |
| ❌ | WallTime | lt_i64_nullable_avx512 |
2.8 µs | 3.9 µs | -27.98% |
| ❌ | WallTime | lt_i64_nullable_neon |
3.6 µs | 5 µs | -27.03% |
| ❌ | WallTime | lt_i64_nullable_avx2 |
2.9 µs | 4 µs | -26.95% |
| ❌ | WallTime | compare_u8_avx512 |
1.7 µs | 2.1 µs | -22.15% |
| ❌ | WallTime | filtered_owned_i64_avx2[OneNullInEight] |
21.8 µs | 27.6 µs | -20.93% |
| ❌ | WallTime | compare_int_nullable_avx512 |
3.7 µs | 4.6 µs | -20.02% |
| ❌ | WallTime | compare_int_nullable_neon |
5.1 µs | 6.4 µs | -19.93% |
| ❌ | WallTime | filtered_owned_i64_avx512[OneNullInEight] |
22.3 µs | 27.8 µs | -19.76% |
| ❌ | WallTime | compare_u8_avx2 |
1.8 µs | 2.2 µs | -19.16% |
| ❌ | Simulation | int_gt[16] |
100.9 µs | 123.8 µs | -18.5% |
| ❌ | Simulation | or_chain[16] |
342.9 µs | 414.1 µs | -17.19% |
| ❌ | WallTime | multiply_shapes_neon[(32768, PerRowPerRow)] |
32.5 µs | 39.2 µs | -17.09% |
| ❌ | WallTime | compare_int_nullable_avx2 |
3.9 µs | 4.7 µs | -16.94% |
| ❌ | WallTime | compare_u8_neon |
2.2 µs | 2.6 µs | -16.77% |
| ❌ | WallTime | mul_i64_nonnull_neon |
32.6 µs | 39.2 µs | -16.68% |
| ❌ | Simulation | baseline_lt[4, 1024] |
75.1 µs | 89.8 µs | -16.41% |
| ❌ | Simulation | int_gt[1024] |
106.8 µs | 127.5 µs | -16.29% |
| ❌ | Simulation | bench_compare_sliced_dict_primitive[(3333, 10000)] |
76.9 µs | 91.7 µs | -16.07% |
| ❌ | Simulation | or_chain[1024] |
359.5 µs | 428.2 µs | -16.05% |
| ... | ... | ... | ... | ... | ... |
ℹ️ 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-compare-primitive (c2e7937) with develop (e88216e)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(0ff150e) during the generation of this report, so e88216e was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩
28d9dd8 to
cd4b46c
Compare
cd4b46c to
6830c56
Compare
6830c56 to
8594ed8
Compare
8594ed8 to
cce1488
Compare
cce1488 to
0823a7d
Compare
0823a7d to
f2db74e
Compare
f2db74e to
a600c93
Compare
## Summary
Allows lazy masks to attach directly as validity when an encoding can
inspect validity through metadata and the input is `AllValid` or
`NonNullable`. Array-backed validity and encodings that have not opted
in retain the existing lazy-mask fallback.
The reduction rule, in pseudocode, is:
```text
Mask(Array(values, validity = AllValid), m)
-> Array(values, validity = m)
Mask(Array(values, validity = NonNullable), m)
-> Array(values, validity = m)
```
This applies when the input encoding opts into
`VALIDITY_IS_METADATA_ONLY` and supports mask reduction. `m` is a
non-nullable Boolean array with the same length as the input, and it may
be lazy. The reduction reuses the values and attaches `m` as validity
without executing it. The output dtype is nullable.
The identity is `AllValid AND m = m`, so no intermediate validity array
is needed.
## Changes
Adds `MaskReduce::VALIDITY_IS_METADATA_ONLY`, defaulting to false, and
enables it for Boolean, primitive, decimal, string, list, struct, map,
and ByteBool encodings. String, decimal, and list reducers preserve
existing buffers and children when rebuilding. List-view masking also
preserves its metadata without revalidating the zero-copy flag.
Two focused tests cover direct attachment to an all-valid primitive
array and the fallback for array-backed validity, including an all-true
bitmap. Existing constant-mask tests remain unchanged, and all tests
stay inline. The Boolean device-buffer fix landed in #10018.
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`. The
rebase preserves this PR's patches. Tests, formatting, and benchmarks
were not rerun locally after the rebase.
---------
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
a600c93 to
8eb8c0c
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. `OutputElement` chooses its collection storage through an associated buffer type and an allocation hook. The executor writes through `OutputBuffer` slots, and the buffer implementation constructs the array. Vortex primitive and Boolean implementations use `BufferMut`, 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 `Vec` storage exercises the owned execution paths without requiring `BufferMut`. 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. --------- Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
8eb8c0c to
c2e7937
Compare
Depends on #10014.
Summary
Moves primitive comparisons onto
RowFnso decoding, constants, validity, and packed Boolean output use the shared row executor.Changes
PrimitiveComparedispatches primitive types and comparison operators through multiversionedRowVisitor::visit_bool, replacing the primitive lane kernels and their operand wrapper. Float comparisons retain Vortex's total ordering.Depends on #10014 for execution-allocator ownership of Boolean output. The comparison tests added in #9948 are retained and now exercise the RowFn implementation.
Validation before the rebase: 168 focused comparison, mask, and RowFn tests passed on the combined stack, including
comparison_uses_execution_allocator, along withcargo clippy -p vortex-array --all-targets --all-features -- -D warnings. Tests, formatting, and benchmarks were not rerun locally after the rebase.