Implement perf optimization using MaybeUninit#230
Implement perf optimization using MaybeUninit#230tcharding wants to merge 1 commit intorust-bitcoin:masterfrom
MaybeUninit#230Conversation
|
I wrote the code with an Opus 4.6 (all the smelly bits, duplicate test function etc was all me). |
Using a zero initialized array on the stack may be slower than using a `MaybeUninit` because of the required initialization to zero just to overwrite the buffer. However using (testing) `MaybeUninit` is a bit of a nuisance so leave the original function `drain_to_slice` in the codebase but cfg it out unless we are testing.
tcharding
force-pushed
the
push-qzvyppnponrl
branch
from
4d8f18f to
1705412
Compare
| // SAFETY: `[MaybeUninit<u8>; N]` has no initialization requirement, | ||
| // so an uninitialized array of them is sound. This is the standard | ||
| // `uninit_array` pattern. | ||
| let mut ret: [MaybeUninit<u8>; N] = unsafe { MaybeUninit::uninit().assume_init() }; |
There was a problem hiding this comment.
In 1705412:
Can you find a citation for this being the "standard uninit_array pattern"? It appears intuitively that MaybeUninit::uninit().assume_init() is always unsound.
AFAICT, by reading for example this rust-lang thread, the correct way to do this on stable Rust is [const { MaybeUninit::uninit() }; N].
| unsafe { | ||
| core::ptr::write(ptr, byte?); | ||
| ptr = ptr.add(1); | ||
| } |
There was a problem hiding this comment.
In 1705412:
Why use ptr::write instead of Maybeuninit::write? That would save you unsafety, casts, and pointer arithmetic.
Thanks for the disclosure. From a new contributor this would be a "close without bothering to comment" PR. |
|
Ouch, I guess I should not to vibe code issues that I don't care about. Sorry for using your time bro. |
|
Drafting because for me personally this is not high priority. |
2 participants
Using a zero initialized array on the stack may be slower than using a
MaybeUninitbecause of the required initialization to zero just to overwrite the buffer.However using (testing)
MaybeUninitis a bit of a nuisance so leave the original functiondrain_to_slicein the codebase but cfg it out unless we are testing.Note to review: This is a bit smelly, feel free to reprimand me.
Close #146