Skip to content

Preserve Boolean buffer handles during mask reduction - #10018

Merged
connortsui20 merged 1 commit into
developfrom
ct/bool-mask-buffer-handle
Sep 23, 2026
Merged

connortsui20 merged 1 commit into
developfrom
ct/bool-mask-buffer-handle

Conversation

@connortsui20

@connortsui20 connortsui20 commented Sep 23, 2026 •

Copy link
Copy Markdown
Member

Summary

Preserves the buffer handle and bit offset when masking a Boolean array. The previous reducer called to_bit_buffer(), which requires host memory and panics for device-backed values.

Changes

Rebuilds through try_new_from_handle and adds one device-buffer regression that checks buffer identity, bit offset, length, and attached validity.

@connortsui20
connortsui20 added this pull request to stack #10019 September 23, 2026 21:02
@connortsui20 connortsui20 added the changelog/fix A bug fix label Sep 23, 2026
@connortsui20
connortsui20 force-pushed the ct/bool-mask-buffer-handle branch from d0dc4b9 to d3f98b9 Compare September 23, 2026 21:06
@connortsui20
connortsui20 marked this pull request as ready for review September 23, 2026 21:07
@codspeed

codspeed Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Merging this PR will degrade performance by 21.38%

⚠️ 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.

⚠️ 4 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

⚡ 1 improved benchmark
❌ 4 regressed benchmarks
✅ 2227 untouched benchmarks
⏩ 329 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
❌ Simulation take_fsl_random[128, 10] 32.7 µs 59.4 µs -44.98%
❌ Simulation density_sweep_single_slice[0.9] 28.5 µs 45.8 µs -37.74%
❌ WallTime filtered_sink_i64_avx512[OneNullInEight] 22.3 µs 26.4 µs -15.53%
❌ WallTime dict_canonicalize_gt_u8_neon[1000000] 488.5 µs 547.5 µs -10.77%
⚡ WallTime filtered_owned_i64_avx512[OneNullInEight] 26 µs 22.4 µs +16.36%
⚠️ Simulation take_fsl_u32_random[256, 10] < 1 ns < 1 ns N/A
⚠️ 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/bool-mask-buffer-handle (5b30168) with develop (515e61e)

Open in CodSpeed

Footnotes

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

@connortsui20
connortsui20 removed this pull request from stack #10019 September 23, 2026 21:19
@connortsui20
connortsui20 force-pushed the ct/bool-mask-buffer-handle branch from d3f98b9 to 325f6b4 Compare September 23, 2026 21:19
@connortsui20
connortsui20 changed the base branch from develop to ct/row-fn-output-allocator September 23, 2026 21:19
@connortsui20
connortsui20 added this pull request to stack #10021 September 23, 2026 21:19
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
@connortsui20
connortsui20 removed this pull request from stack #10021 September 23, 2026 21:23
@connortsui20
connortsui20 changed the base branch from ct/row-fn-output-allocator to develop September 23, 2026 21:23
@connortsui20
connortsui20 force-pushed the ct/bool-mask-buffer-handle branch from 325f6b4 to 5b30168 Compare September 23, 2026 21:23
@connortsui20
connortsui20 added this pull request to stack #10022 September 23, 2026 21:23
@connortsui20
connortsui20 merged commit 26231bb into develop Sep 23, 2026
89 of 90 checks passed
@connortsui20
connortsui20 deleted the ct/bool-mask-buffer-handle branch September 23, 2026 21:39
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/fix A bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants