Add From Vec<u8> for BytesMut

[leshow]

This impl exists for Bytes but not BytesMut, you have to use a &[u8] which calls to_vec again when it could just use the original vec.

[taiki-e]

I think it's intentional that From<Vec<u8>> has not been provided.

bytes/src/bytes_mut.rs

Lines 735 to 742 in 7daa7fe

	// For now, use a `Vec` to manage the memory for us, but we may want to
	// change that in the future to some alternate allocator strategy.
	//
	// Thus, we don't expose an easy way to construct from a `Vec` since an
	// internal change could make a simple pattern (`BytesMut::from(vec)`)
	// suddenly a lot more expensive.
	#[inline]
	pub(crate) fn from_vec(mut vec: Vec<u8>) -> BytesMut {

[leshow]

Right okay. There's a From<&[u8]> impl, that already uses that method though, but does an extra allocation, how would a From<Vec<u8>> be more expensive than that? Feel free to close this if it's not wanted.

[Kobzol]

@taiki-e If I understand it correctly, you don't want to enable From<Vec<u8>>, because although it is fast now, it may become silently slow in the future. But by doing this you anyway force people to use the slow variant From<&[u8]>, just sooner than it's required (maybe the internal representation won't even change).

Couldn't this be handled with a deprecation warning instead, if such internal representation change occurs in the future? In that way we could get a fast solution now and a warning if it slows down in the future. I'm asking because we assumed that Vec<u8> -> BytesMut is cheap in our codebase, so it would be nice to have this method as long as it is possible.

[taiki-e]

Sorry for the late response.

how would a From<Vec<u8>> be more expensive than that?

For example, if we add From<Vec<u8>> in the current implementation, the following two are equivalent:

BytesMut::from(slice.to_vec());
BytesMut::from(&slice);

However, if the implementation of BytesMut is changed, From<Vec<u8>> may be equivalent to From<&[u8]>.
As a result, the first line becomes equivalent to BytesMut::from(&slice.to_vec()), which is inefficient (to_vec called twice).

Couldn't this be handled with a deprecation warning instead, if such internal representation change occurs in the future? In that way we could get a fast solution now and a warning if it slows down in the future. I'm asking because we assumed that Vec<u8> -> BytesMut is cheap in our codebase, so it would be nice to have this method as long as it is possible.

Unfortunately, deprecation doesn't work with trait impl and its items: playground

[taiki-e]

We don't want to provide an API that relies on From<Vec<u8>> being cheaper than From<&[u8]>, so I'll close this PR, but thanks for the patch @leshow!

[Kobzol]

There could be a special method from_vec outside any From trait impl, which could be easily deprecated. But I guess that you don't want that either?

[taiki-e]

@Kobzol Even if it's deprecatable, I don't think it's preferable to add a new API that is known likely to be deprecated in the future.
(Unless it makes available functionality that is not currently available. In this case, Vec to MytesMut conversion is already available via From<&[u8]>.)

[Kobzol]

Well it would enable a faster version of this functionality for now, and would also preserve backwards compatibility with users that were already assuming that this fast conversion exists. But I understand if you don't think that's worth it.