Repository navigation
Thread execution contexts through Patches - #10592
robert3005 wants to merge 4 commits into
Conversation
`MaskedArray::try_new` checked that the child has no nulls using the hidden global `legacy_session()`. Take an `ExecutionCtx` instead, and validate during deserialization with a context from the session. Add `MaskedArray::new_unchecked` and use it in the Masked filter, slice and take rules and in Constant take, where the invariants hold by construction. Constant take no longer executes the indices' validity. Signed-off-by: Claude <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y1zHpE1K1Y9HvyU4kt24Tq
Signed-off-by: Claude <noreply@anthropic.com> # Conflicts: # lang/cpp/tests/scan.cpp # vortex-array/src/arrays/masked/tests.rs # vortex-array/src/arrays/masked/vtable/canonical.rs
The masked commit also carried the Display, VarBinView, Null take, ZstdBuffers, RunEnd, FSST and FFI changes, which have their own PRs. Restore those files to develop so this PR only changes MaskedArray and its callers. Signed-off-by: Claude <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y1zHpE1K1Y9HvyU4kt24Tq
Merging this PR will degrade performance by 6.83%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | WallTime | mul_u64_nonnull_neon |
20.8 µs | 39 µs | -46.66% |
| ❌ | WallTime | filter_neon[8] |
959 ns | 1,199 ns | -20.02% |
| ❌ |
Simulation | random_i128[0.01] |
4.8 µs | 5.7 µs | -15.88% |
| ❌ | WallTime | filter_neon[64] |
1.8 µs | 2.1 µs | -15.66% |
| ❌ | WallTime | threshold_avx2[80] |
2.7 µs | 3 µs | -10.22% |
| ❌ | WallTime | threshold_neon[80] |
3.5 µs | 3.9 µs | -10.17% |
| ⚡ | Simulation | chunked_opt_bool_canonical_into[(1000, 10)] |
65.7 µs | 54.9 µs | +19.72% |
| ⚡ | WallTime | threshold_avx512[80] |
2.9 µs | 2.5 µs | +16.47% |
| ⚡ | WallTime | words_gather_dispatch_neon[65536] |
2 µs | 1.8 µs | +10.83% |
| ⚡ | Simulation | take_map[(0.005, 0.05)] |
26.8 µs | 24.3 µs | +10.61% |
| ⚡ | Simulation | take_map[(0.01, 0.05)] |
29 µs | 26.3 µs | +10.08% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing rk/legacy-session-patches (b473671) with develop (8c2534e)
Footnotes
-
12 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. ↩
-
43 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. ↩
e1b5a06 to
0af3b3d
Compare
`Patches::new` validated, and `slice`, `chunk_offset_at`, `min_index` and `max_index` read, the patch indices with the hidden global `legacy_session()`. Take an `ExecutionCtx` instead, as do Patched `lane_range` and `Sparse::try_new`. Sparse, ALP, ALP-RD and BitPacked deserialization validate their patches with a context from the session. The BitPacked and ALP-RD encoders and the Sparse patch reshaping build valid patches by construction and use `Patches::new_unchecked`. Signed-off-by: Claude <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y1zHpE1K1Y9HvyU4kt24Tq
471c088 to
b473671
Compare
These all executed with
legacy_session()and now take anExecutionCtx:Patches::new, which validates that the patch indices are in bounds and, in debug builds, sortedPatches::slice,chunk_offset_at,min_indexandmax_index, which read the indices or chunk offsetslane_rangeSparse::try_newPart of #9446