Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bytes::make_mut #611

Closed
mattfbacon opened this issue Apr 29, 2023 · 8 comments · Fixed by #695
Closed

Bytes::make_mut #611

mattfbacon opened this issue Apr 29, 2023 · 8 comments · Fixed by #695

Comments

@mattfbacon
Copy link

Would it be possible to add a make_mut method on Bytes that converts to BytesMut in place if possible? For example if the Bytes was created from a Vec and has only one reference, then it could turn that Vec into a BytesMut and give that to you in place.

If it can't convert it could either return Option or just make a copy. I see this library has an emphasis on avoiding implicit slow operations (good!) so the Option approach might be better.

@vilgotf
Copy link

vilgotf commented Jul 31, 2023

Returning the original buffer upon failure allows defining a fallback path which is much more flexible. Mirroring https://doc.rust-lang.org/stable/std/sync/struct.Arc.html#method.try_unwrap sounds the best to me as Bytes is essentially an Arc<Vec<u8>>

@Darksonn
Copy link
Contributor

Darksonn commented Aug 1, 2023

It's not super simple to do this, because as far as I recall, Bytes has more representations than BytesMut, so its not necessarily so easy to convert.

@mattfbacon
Copy link
Author

It probably has to work by cases on specific vtables.

@rklaehn
Copy link

rklaehn commented Apr 9, 2024

There is From for Vec, and a private from_vec for BytesMut. So it seems all the parts are in place.

Something like this:

if self.is_unique() {
  Ok(BytesMut::from_vec(Vec::from(self)))
} else {
  Err(self)
}

https://docs.rs/bytes/latest/src/bytes/bytes_mut.rs.html#831

@mattfbacon
Copy link
Author

This looks like a great approach; would you be willing to make it into a PR?

@rklaehn
Copy link

rklaehn commented Apr 9, 2024

Sure: #687

The alternative would be to make BytesMut::from_vec pub #615 .

Should this be a dedicated method, or a TryFrom instance? Let me know what tests you need.

@Sytten
Copy link
Contributor

Sytten commented Apr 13, 2024

Can we also pack into this PR the equivalent of Arc::make_mut @rklaehn ?

Because its nice to be able to try it, but you are kind of stuck if you actually want to have a mutable object and you dont care if it makes a copy.

@Sytten
Copy link
Contributor

Sytten commented Apr 14, 2024

I made a zero-copy (if possible) version to convert directly to mut without going through a Vec first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants