Avoid execution contexts for simple masked validity - #9713
Conversation
Signed-off-by: Baris Palaska <barispalaska@gmail.com>
Merging this PR will regress 2 benchmarks
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | WallTime | arrow_checked_add_u32_neon[16384] |
12.4 µs | 20.4 µs | -39.26% |
| ❌ | WallTime | deferred_i64_avx2[PerRowPerRow] |
9.9 µs | 11.6 µs | -14.24% |
| ⚡ | WallTime | add_shapes_neon[(128, PerRowPerRow)] |
3.7 µs | 1.9 µs | +98.01% |
| ⚡ | WallTime | subtract_shapes_neon[(128, PerRowPerRow)] |
2.9 µs | 1.8 µs | +56.03% |
| ⚡ | WallTime | multiply_shapes_neon[(128, PerRowPerRow)] |
2.9 µs | 1.9 µs | +51.21% |
| ⚡ | WallTime | arrow_checked_add_u32_avx2[16384] |
21.3 µs | 17.7 µs | +20.36% |
| ⚡ | WallTime | arrow_checked_add_u32_avx512[16384] |
21.3 µs | 17.7 µs | +20.28% |
| ⚡ | WallTime | add_u32_nonnull_neon |
7.9 µs | 6.7 µs | +18.53% |
| ⚡ | WallTime | add_shapes_neon[(16384, PerRowPerRow)] |
11.3 µs | 9.8 µs | +16.06% |
| ⚡ | WallTime | compare_int_nullable_neon |
5.6 µs | 4.9 µs | +15.91% |
| ⚡ | WallTime | add_i32_nonnull_neon |
9 µs | 7.8 µs | +14.87% |
| ⚡ | WallTime | add_i64_nonnull_neon |
11.3 µs | 9.8 µs | +14.74% |
| ⚡ | WallTime | add_i64_nullable_neon |
12.8 µs | 11.3 µs | +13.18% |
| ⚡ | WallTime | compare_int_constant_neon |
4.4 µs | 4 µs | +10.59% |
| ⚡ | WallTime | add_constant_shapes_neon[(16384, PerRowNullableConstant)] |
10.8 µs | 9.8 µs | +10.21% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing bp/masked-validity-fast-path (e4a29cf) with develop (a67cd1b)
Footnotes
-
206 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. ↩
-
4 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. ↩
| #[rstest] | ||
| #[case(Validity::NonNullable, true)] | ||
| #[case(Validity::AllValid, true)] | ||
| #[case(Validity::AllInvalid, false)] | ||
| #[case(Validity::from_iter([true, true, true]), true)] | ||
| #[case(Validity::from_iter([true, false, true]), false)] | ||
| fn test_child_all_valid(#[case] validity: Validity, #[case] expected: bool) -> VortexResult<()> { | ||
| let child = PrimitiveArray::new(vortex_buffer::buffer![1i32, 2, 3], validity).into_array(); | ||
| assert_eq!(child_all_valid(&child)?, expected); | ||
| Ok(()) | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_empty_child_all_valid() -> VortexResult<()> { | ||
| let child = PrimitiveArray::new(Buffer::<i32>::empty(), Validity::AllInvalid).into_array(); | ||
| assert!(child_all_valid(&child)?); | ||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
was this not tested before?
Summary
ExecutionCtxcreation for non-array masked child validity.Tests
cargo nextest run -p vortex-arraycargo clippy -p vortex-array --all-targets --all-featurescargo +nightly fmt --all