Skip to content

Compare primitive values with RowFn - #9979

Draft
connortsui20 wants to merge 1 commit into
ct/row-fn-output-allocatorfrom
ct/row-fn-compare-primitive
Draft

connortsui20 wants to merge 1 commit into
ct/row-fn-output-allocatorfrom
ct/row-fn-compare-primitive

Conversation

@connortsui20

@connortsui20 connortsui20 commented Sep 22, 2026

Copy link
Copy Markdown
Member

Depends on #10014.

Summary

Moves primitive comparisons onto RowFn so decoding, constants, validity, and packed Boolean output use the shared row executor.

Changes

PrimitiveCompare dispatches primitive types and comparison operators through multiversioned RowVisitor::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 with cargo clippy -p vortex-array --all-targets --all-features -- -D warnings. Tests, formatting, and benchmarks were not rerun locally after the rebase.

@connortsui20 connortsui20 added the changelog/performance A performance improvement label Sep 22, 2026
@codspeed

codspeed Bot commented Sep 22, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 7.34%

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

⚠️ 3 benchmarks measured no execution time

Nothing ran under measurement, usually because the compiler removed the code under test. These results are not comparable, so they count as unchanged.

Preventing compiler optimizations

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 10 improved benchmarks
❌ 36 regressed benchmarks
✅ 2130 untouched benchmarks
⏩ 385 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation take[duplicates/repeated/primitive/nonnull/chunks=16/indices=1000] 28.7 µs 112.4 µs -74.5%
WallTime lt_i64_nullable_avx512 2.8 µs 3.9 µs -27.84%
WallTime lt_i64_nullable_avx2 2.9 µs 3.9 µs -26.14%
WallTime compare_u8_avx512 1.7 µs 2.2 µs -22.68%
WallTime lt_i64_nullable_neon 3.6 µs 4.7 µs -21.87%
WallTime compare_int_nullable_avx512 3.7 µs 4.6 µs -20.44%
WallTime compare_int_nullable_neon 5.2 µs 6.5 µs -20.34%
WallTime compare_u8_avx2 1.7 µs 2.2 µs -18.92%
WallTime compare_int_nullable_avx2 3.9 µs 4.8 µs -18.3%
Simulation int_gt[16] 102.4 µs 123.3 µs -16.98%
Simulation or_chain[16] 344.5 µs 414.2 µs -16.82%
Simulation baseline_lt[4, 1024] 75.4 µs 90.2 µs -16.49%
Simulation or_chain[1024] 360.6 µs 427 µs -15.54%
Simulation float_gt[1024] 258.4 µs 305.8 µs -15.51%
Simulation float_gt[16] 235.6 µs 277.3 µs -15.04%
WallTime compare_u8_neon 2.2 µs 2.5 µs -15.01%
Simulation int_gt[1024] 108.3 µs 127.2 µs -14.82%
Simulation baseline_eq[4, 1024] 75.1 µs 88 µs -14.72%
Simulation baseline_lt[16, 1024] 77 µs 90 µs -14.52%
Simulation baseline_eq[16, 1024] 76.3 µs 89.3 µs -14.51%
... ... ... ... ... ...

ℹ️ 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 (8eb8c0c) with develop (2ab66d7)2

Open in CodSpeed

Footnotes

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

  2. No successful run was found on ct/row-fn-output-allocator (e1ca00c) during the generation of this report, so develop (2ab66d7) was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@connortsui20
connortsui20 marked this pull request as draft September 22, 2026 19:43
@connortsui20
connortsui20 force-pushed the ct/row-fn-compare-primitive branch from 28d9dd8 to cd4b46c Compare September 23, 2026 20:46
@connortsui20
connortsui20 changed the base branch from develop to ct/all-valid-mask-reduction September 23, 2026 20:46
@connortsui20
connortsui20 added this pull request to stack #10017 September 23, 2026 20:46
@connortsui20
connortsui20 force-pushed the ct/row-fn-compare-primitive branch from cd4b46c to 6830c56 Compare September 23, 2026 21:01
@connortsui20
connortsui20 removed this pull request from stack #10017 September 23, 2026 21:02
@connortsui20
connortsui20 added this pull request to stack #10019 September 23, 2026 21:02
@connortsui20
connortsui20 force-pushed the ct/row-fn-compare-primitive branch from 6830c56 to 8594ed8 Compare September 23, 2026 21:06
@connortsui20
connortsui20 removed this pull request from stack #10019 September 23, 2026 21:19
@connortsui20
connortsui20 force-pushed the ct/row-fn-compare-primitive branch from 8594ed8 to cce1488 Compare September 23, 2026 21:19
@connortsui20
connortsui20 added this pull request to stack #10021 September 23, 2026 21:19
@connortsui20
connortsui20 removed this pull request from stack #10021 September 23, 2026 21:23
@connortsui20
connortsui20 changed the base branch from ct/all-valid-mask-reduction to develop September 23, 2026 21:23
@connortsui20
connortsui20 force-pushed the ct/row-fn-compare-primitive branch from cce1488 to 0823a7d Compare September 23, 2026 21:23
@connortsui20
connortsui20 changed the base branch from develop to ct/row-fn-output-allocator September 23, 2026 21:23
@connortsui20
connortsui20 added this pull request to stack #10022 September 23, 2026 21:23
@connortsui20
connortsui20 force-pushed the ct/row-fn-compare-primitive branch from 0823a7d to f2db74e Compare September 23, 2026 21:26
@connortsui20
connortsui20 force-pushed the ct/row-fn-compare-primitive branch from f2db74e to a600c93 Compare September 23, 2026 21:53
connortsui20 added a commit that referenced this pull request Sep 24, 2026
## 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>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
@connortsui20
connortsui20 force-pushed the ct/row-fn-compare-primitive branch from a600c93 to 8eb8c0c Compare September 24, 2026 02:37

This branch has not been deployed

No deployments
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.

1 participant