unsafe improvements
#6551
unsafe improvements
#6551
Conversation
The MaybeUninit doesn't ever hold any uninitialized bytes, but if it _did_, then this would have been UB! Because the write can write fewer than 8 bytes, it would leave the remaining bytes uninitialized. Since the `MaybeUninit<u64>` is never uninitialized, we can remove the use of `MaybeUninit`, and just use `u64`. Then it becomes clear that the only thing for the unsafe block is the `copy_nonoverlapping`.
|
thanks @ssbr ! |
|
@ssbr can you please check the build results? |
The alternative would be to actually convert the `debug_assert` to an `assert`, or clamp to 8, but both of these could conceivably have a performance impact, so in the interest of being a pure win, with no downsides, that change is left to a future editor.
I struggled to find them! Sorry, not used to github. The only way I could find was to click into checks, click into the commit history, find the one with the x, click that, and then click to open every check until I found the failures. Is there an easier way? I found two:
|
|
https://github.com/apache/arrow-rs/actions/runs/11334312589/job/31523941790?pr=6551 does not appear to be related to this PR (it happens in main as well) |
|
Not sure if the CI failure is real or transitional |
It's also present without the change, so real, but unrelated. |
@itsjunetime fixed it in #6585 |
Which issue does this PR close?
N/A
Rationale for this change
I discovered these while reviewing the unsafe code of arrow-rs -- this mostly just removes some instances of potentially-unsafe code, replacing them with safe equivalents. That makes it easier to review, at (so far as I know) no performance cost.
What changes are included in this PR?
unsafecode in theOffsetBufferBuilder.u64instead of an always-initializedMaybeUninit<u64>.read_bytes_to_u64.Are there any user-facing changes?
No.