feat: Add output: &mut [MaybeUninit<u8>] support - #65
Conversation
This commit adds new methods to support encoding and decoding with a `&mut [MaybeUninit<u8>]` as output. It only supports the raw APIs.
Erouan50
changed the title
output: &mut [MaybeUninit<u8>] support
I agree, no such methods should be unsafe as there's nothing inherently unsafe of it |
NobodyXu
left a comment
NobodyXu
left a comment
There was a problem hiding this comment.
I'd say it looks pretty good, but I'm not a maintainer or member so you'd need to get a member to approve jt
|
Thank you @Erouan50 -- we also would love to use this feature downstream in arrow-rs (see apache/arrow-rs#9583) |
|
Looks like @BurntSushi is the core (only) maintainer of this repo: Let us know if there is anything we can do to help |
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
I also went over this PR carefully (but am not a committer) and it makes sense to me
| self.compress_uninit(input, output) | ||
| } | ||
|
|
||
| /// Compresses all bytes in `input` into a freshly allocated `Vec`. |
There was a problem hiding this comment.
I think you could reduce the diff (and thus make this PR easier to review) if you didn't move the location of this method
There was a problem hiding this comment.
That's fair, I just moved them.
| let mut i = 0; | ||
| while n >= 0b1000_0000 { | ||
| data[i] = (n as u8) | 0b1000_0000; | ||
| data[i].write((n as u8) | 0b1000_0000); |
There was a problem hiding this comment.
looking at this code, I think it could be significantly faster if we (in a follow on PR) omitted the bounds check / verified capacity at a higher level
I think we would need to get profiling to do so
|
@BurntSushi do you want to take a look at this? |
4 participants
Hello,
This pull request adds new methods to support encoding and decoding with a
&mut [MaybeUninit<u8>]as output. It only supports the raw APIs.The implementation was surprising easy, my only concern is I don't know if
raw::Decoder::decompress_uninit()andraw::Encoder::compress_uninit()should be unsafe or not. My opinion is no because calling those methods is safe; the unsafe part is to read the output without truncating it before.Fixes #62, maybe?