Repository navigation
fix: alignment and padding of sliced CUDA Arrow bitmaps - #9926
Conversation
Merging this PR will regress 1 benchmark
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | take_fsl_u32_random[256, 10] |
124.4 µs | 159.6 µs | -22.05% |
| ⚡ | Simulation | take_fsl_u32_random[16, 100] |
165.7 µs | 126 µs | +31.53% |
| ⚡ | Simulation | take_fsl_nullable_random[16, 100] |
190.8 µs | 165.4 µs | +15.38% |
| ⚡ | Simulation | take_fsl_f16_random[256, 100] |
231.7 µs | 204.8 µs | +13.15% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing ad/cuda-arrow-bitmap-correctness (50ec1e4) with develop (2c8f160)
Footnotes
-
251 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. ↩
-
1 benchmark was run, but is now archived. If it was deleted in another branch, consider rebasing to remove it from the report. Instead if it was added back, click here to restore it. ↩
|
@claude review |
|
Claude finished @robert3005's task in 3m 43s —— View job Claude Code Review
Reviewed all three files end-to-end (kernel, export path, and stream padding helpers). This is a careful, well-tested fix. My overall assessment: the logic is correct; I have only minor/defensive suggestions and a note on the CodSpeed report. Correctness — CUDA kernel (
|
Signed-off-by: Alexander Droste <alexander.droste@protonmail.com>
…ation Signed-off-by: Alexander Droste <alexander.droste@protonmail.com>
Signed-off-by: Alexander Droste <alexander.droste@protonmail.com>
Signed-off-by: Alexander Droste <alexander.droste@protonmail.com>
Signed-off-by: Alexander Droste <alexander.droste@protonmail.com>
Signed-off-by: Alexander Droste <alexander.droste@protonmail.com>
2b76063 to
ef0c517
Compare
Signed-off-by: Alexander Droste <alexander.droste@protonmail.com>
Signed-off-by: Alexander Droste <alexander.droste@protonmail.com>
Signed-off-by: Alexander Droste <alexander.droste@protonmail.com>
Signed-off-by: Alexander Droste <alexander.droste@protonmail.com>
Signed-off-by: Alexander Droste <alexander.droste@protonmail.com>
Align repack inputs within their backing allocations so full words use aligned loads. Share capped launch constants and exercise grid-stride wrapping with a smaller test cap. Use bitmap-neutral helpers, extract bool export, and compare device pointers for reuse. Cover both bool buffer tails and nonzero-offset reuse while reducing redundant repack cases. Signed-off-by: Alexander Droste <alexander.droste@protonmail.com>
Allocate cuDF-padded bitmap storage and zero only its tail before dictionary bool gathers and run-end validity expansion. Track that tail while preserving logical buffer lengths so Arrow export avoids a copy. Extend existing producer tests to check zero padding and exported device pointer reuse. Signed-off-by: Alexander Droste <alexander.droste@protonmail.com>
Exercise multi-block reduction and grid-stride iteration with a nonzero offset and dirty prefix and tail bits. Assert that the fixture exceeds the test grid cap and matches the exact CPU-derived null count. Signed-off-by: Alexander Droste <alexander.droste@protonmail.com>
Signed-off-by: Alexander Droste <alexander.droste@protonmail.com>
Signed-off-by: Alexander Droste <alexander.droste@protonmail.com>
Summary
Fix CUDA Arrow export of sliced BOOL values and validity bitmaps so their storage is safe for cuDF’s word-based reads.
Correct logical values and offsets are not sufficient:
cuDF’s padded mask extent.
Export now reuses suitably aligned, zero-padded storage or copies/repacks it when necessary. Repacking aligns the input view within its backing allocation and uses bounded byte loads only for the partial tail. Kernel-produced bitmaps are padded up front to avoid export copies.
Focused regression tests cover sliced values and validity, zeroed tails, nonzero-offset reuse, and capped-grid repacking and null counting.