`impl FromIterator for BytesMut` exposes implementation details about bytes

[seanmonstar]

What's the bug? That the compiler is able to make the copying faster? That seems like a thing we want to keep.

It doesn't actually expose in the type system the internals.

[jorgehermo9]

I don't think people expect a from_iter to be free.

They could think it from something like a Vec<u8>, as it happens with Bytes::from(vec)

You see it like its free in the benchmark most likely because the compiler optimizes that a lot, since it is a hardcoded Vec.

Try with something like this (note the std::hint::black_box):

use bytes::BytesMut; // 1.6.1

#[global_allocator]
static ALLOC: dhat::Alloc = dhat::Alloc; // 0.3.3

fn main() {
    let _guard = dhat::Profiler::new_heap();

    let src = vec![1; 1024];
    let mut_from_vec = std::hint::black_box(measure(|| BytesMut::from_iter(src)));
    assert_eq!(mut_from_vec, 0); // doesn't allocate!

    let mut_from_array = std::hint::black_box(measure(|| BytesMut::from_iter([1; 1024])));
    assert_eq!(mut_from_array, 1024);
}

fn measure<T>(f: impl FnOnce() -> T) -> u64 {
    let before = dhat::HeapStats::get();
    f();
    let after = dhat::HeapStats::get();
    after.total_bytes - before.total_bytes
}

[aatifsyed]

The fix would be to turn off specialization like this:

let src = vec![1; 1024];
let from_vec = measure(|| BytesMut::from_iter(NoSpecialize(src)));
assert_eq!(from_vec, 1024); // this used to be 0

struct NoSpecialize<T>(T);

impl Iterator for NoSpecialize<vec::IntoIter<u8>> {
    type Item = u8;
    fn next(&mut self) -> Option<Self::Item> {
        self.0.next()
    }
    fn size_hint(&self) -> (usize, Option<usize>) {
        self.0.size_hint()
    }
}

impl IntoIterator for NoSpecialize<Vec<u8>> {
    type Item = u8;
    type IntoIter = NoSpecialize<vec::IntoIter<u8>>;
    fn into_iter(self) -> Self::IntoIter {
        NoSpecialize(self.0.into_iter())
    }
}

[Darksonn]

I believe the section you're quoting is only talking about BytesMut. We already have impl From<Vec<u8>> for Bytes.

[aatifsyed]

I believe the section you're quoting is only talking about BytesMut. We already have impl From<Vec> for Bytes.

Ah, my mistake! I've updated the issue and comments as appropriate :)

[aatifsyed]

If you agree that this is a bug, I'll happily open a PR

[aatifsyed]

Perhaps "bug" is a bit strong.
BytesMut goes to moderate lengths to not allow cheap conversion from a Vec<u8> so that users cannot rely on it.
But because of the interaction of the above traits, a wily user can get a "free" conversion to BytesMut.

If the allocation strategy changes in future (which the comment in from_vec seems to want to leave open), the "free" conversion becomes not free, which violates the comment.