Add BytesMut::try_reserve()
#613
Conversation
|
Hello, I also need this. Any way we can get this merged? |
| @@ -838,7 +906,10 @@ impl BytesMut { | |||
| return true; | |||
| } | |||
|
|
|||
| self.reserve_inner(additional, false) | |||
| match self.reserve_inner(additional, false) { | |||
| Err(err) => panic!("fail to reserve: {}", err), | |||
There was a problem hiding this comment.
This should call std::alloc::handle_alloc_error.
There was a problem hiding this comment.
This codepath (try_reclaim error case in reserve_inner) actually can't be reached (reserve_inner returns an error when it fails to allocate, but this codepath calls reserve_inner with allocate set to false). It should probably be changed to unreachable!().
For the other caller (try_reserve error case), it requires creating a Layout to pass to handle_alloc_error. This is not entirely straightforward. Ideally we'd extract the layout from TryReserveErrorKind but that's currently unstable, so we'll have to guess it.
I currently have this:
let mut layout = Layout::new::<u8>();
if let Ok(array_layout) = layout.array(self.len() + additional) {
layout = array_layout;
}
std::alloc::handle_alloc_error(array_layout);Ergo we create a Layout of [u8; len + additional], and if that fails, we fallback to a simple layout of u8. I think that's good enough? In practice, [u8; len + additional] should always work out. But if len + additional overflows isize, it might error out - and I'm not sure what to give to handle_alloc_error in this case. Or maybe we should only call handle_alloc_error when we successfully create a Layout, and unwrap on the layout creation otherwise?
| /// assert_eq!(buf.as_ptr(), ptr); | ||
| /// ``` | ||
| #[inline] | ||
| pub fn try_reserve(&mut self, additional: usize) -> Result<(), TryReserveError> { |
There was a problem hiding this comment.
We should also return our own error type. The one from std is not publicly constructable, which would mean our implementation would always need to call std, we couldn't refactor it.
|
Actually, it seems like |
|
Vec::try_reserve has been in Rust since 1.57, which was released in December 2, 2021 (almost three years ago). Is there any plan on raising the MSRV anytime soon? Otherwise would you accept a PR gating this API behind a feature gate? |
|
Given that even syn is at 1.61, I think a bump to 1.57 is fine. |
1.56 would be enough for miniz_oxide 0.8 and edition 2021, but bytes may bump it to 1.57 soon tokio-rs/bytes#613 (comment)
I rebased #521.
While I do not claim this implements #484, I think the review was too harsh:
BytesMutis the onlyAPI with a
reservemethod, adding other fallible allocations APIs should not be done in this MR.