tokio-rs / bytes

Utilities for working with bytes
MIT License
1.91k stars 288 forks source link

Bytes::make_mut #611

Closed mattfbacon closed 6 months ago

mattfbacon commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

It probably has to work by cases on specific vtables.

rklaehn commented 7 months ago

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 commented 7 months ago

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

rklaehn commented 7 months ago

Sure: https://github.com/tokio-rs/bytes/pull/687

The alternative would be to make BytesMut::from_vec pub https://github.com/tokio-rs/bytes/issues/615 .

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

Sytten commented 6 months ago

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 commented 6 months ago

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