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

Not shrink Vec on coverting to Bytes (BytesMut::freeze improve) #418

Closed
wants to merge 1 commit into from

Conversation

fanatid
Copy link
Contributor

@fanatid fanatid commented Jul 19, 2020

Closes #401

cargo-asm before
$ cargo asm --rust "<bytes::bytes::Bytes as core::convert::From<alloc::vec::Vec<u8>>>::from"
 fn from(vec: Vec<u8>) -> Bytes {
 push    r15
 push    r14
 push    rbx
 sub     rsp, 32
 mov     rbx, rdi
     self.len() == 0 (liballoc/vec.rs:1409)
     cmp     qword, ptr, [rsi, +, 16], 0
 if vec.is_empty() {
 je      .LBB48_15
 let slice = vec.into_boxed_slice();
 movups  xmm0, xmmword, ptr, [rsi]
 movaps  xmmword, ptr, [rsp], xmm0
 mov     r14, qword, ptr, [rsi, +, 16]
 mov     qword, ptr, [rsp, +, 16], r14
     self.buf.capacity() (liballoc/vec.rs:483)
     mov     rsi, qword, ptr, [rsp, +, 8]
     if self.capacity() != self.len { (liballoc/vec.rs:626)
     cmp     rsi, r14
     if self.capacity() != self.len { (liballoc/vec.rs:626)
     je      .LBB48_11
     assert!(amount <= self.capacity(), "Tried to shrink to a larger capacity"); (liballoc/raw_vec.rs:458)
     jb      .LBB48_7
     if mem::size_of::<T>() == 0 || self.cap == 0 { (liballoc/raw_vec.rs:235)
     test    rsi, rsi
     if mem::size_of::<T>() == 0 || self.cap == 0 { (liballoc/raw_vec.rs:235)
     je      .LBB48_11
     let (ptr, layout) = if let Some(mem) = self.current_memory() { mem } else { return Ok(()) }; (liballoc/raw_vec.rs:460)
     mov     rdi, qword, ptr, [rsp]
     let (ptr, layout) = if let Some(mem) = self.current_memory() { mem } else { return Ok(()) }; (liballoc/raw_vec.rs:460)
     test    rdi, rdi
     je      .LBB48_11
     ReallocPlacement::MayMove if new_size == 0 => { (liballoc/alloc.rs:252)
     test    r14, r14
     je      .LBB48_6
     unsafe { __rust_realloc(ptr, layout.size(), layout.align(), new_size) } (liballoc/alloc.rs:124)
     mov     edx, 1
     mov     rcx, r14
     call    qword, ptr, [rip, +, __rust_realloc@GOTPCREL]
     mov     r15, rax
     Ok(t) => Ok(t), (libcore/result.rs:611)
     test    rax, rax
     jne     .LBB48_10
     Err(AllocError { layout, .. }) => handle_alloc_error(layout), (liballoc/raw_vec.rs:363)
     mov     esi, 1
     mov     rdi, r14
     call    qword, ptr, [rip, +, _ZN5alloc5alloc18handle_alloc_error17h7f6a7a00afe6ad92E@GOTPCREL]
     ud2
.LBB48_15:
 Bytes {
 lea     rax, [rip, +, .L__unnamed_3]
 mov     qword, ptr, [rbx], rax
 xorps   xmm0, xmm0
 movups  xmmword, ptr, [rbx, +, 8], xmm0
 lea     rax, [rip, +, .L__unnamed_15]
 mov     qword, ptr, [rbx, +, 24], rax
     pub unsafe fn drop_in_place<T: ?Sized>(to_drop: *mut T) { (libcore/ptr/mod.rs:184)
     mov     rdi, qword, ptr, [rsi]
     if let Some((ptr, layout)) = self.current_memory() { (liballoc/raw_vec.rs:507)
     test    rdi, rdi
     if mem::size_of::<T>() == 0 || self.cap == 0 { (liballoc/raw_vec.rs:235)
     je      .LBB48_20
     mov     rsi, qword, ptr, [rsi, +, 8]
     test    rsi, rsi
     je      .LBB48_20
     unsafe { __rust_dealloc(ptr, layout.size(), layout.align()) } (liballoc/alloc.rs:102)
     mov     edx, 1
     call    qword, ptr, [rip, +, __rust_dealloc@GOTPCREL]
     jmp     .LBB48_20
.LBB48_6:
     mov     r15d, 1
     mov     edx, 1
     call    qword, ptr, [rip, +, __rust_dealloc@GOTPCREL]
.LBB48_10:
     self.ptr = unsafe { Unique::new_unchecked(memory.ptr.cast().as_ptr()) }; (liballoc/raw_vec.rs:382)
     mov     qword, ptr, [rsp], r15
     self.cap = Self::capacity_from_bytes(memory.size); (liballoc/raw_vec.rs:383)
     mov     qword, ptr, [rsp, +, 8], r14
.LBB48_11:
     let me = ManuallyDrop::new(self); (liballoc/vec.rs:686)
     mov     rax, qword, ptr, [rsp]
 if ptr as usize & 0x1 == 0 {
 test    al, 1
 jne     .LBB48_18
 Bytes {
 mov     qword, ptr, [rbx], rax
 let data = ptr as usize | KIND_VEC;
 or      rax, 1
 Bytes {
 mov     qword, ptr, [rbx, +, 8], r14
 mov     qword, ptr, [rbx, +, 16], rax
 mov     rax, qword, ptr, [rip, +, _ZN5bytes5bytes22PROMOTABLE_EVEN_VTABLE17h45f693bc6b48efcbE@GOTPCREL]
 jmp     .LBB48_19
.LBB48_18:
 Bytes {
 mov     qword, ptr, [rbx], rax
 mov     qword, ptr, [rbx, +, 8], r14
 mov     qword, ptr, [rbx, +, 16], rax
 mov     rax, qword, ptr, [rip, +, _ZN5bytes5bytes21PROMOTABLE_ODD_VTABLE17h7ab92e8c61ff435dE@GOTPCREL]
.LBB48_19:
 mov     qword, ptr, [rbx, +, 24], rax
.LBB48_20:
 }
 mov     rax, rbx
 add     rsp, 32
 pop     rbx
 pop     r14
 pop     r15
 ret
.LBB48_7:
     $crate::panicking::panic($msg) (libcore/macros/mod.rs:10)
     lea     rdi, [rip, +, .L__unnamed_28]
     lea     rdx, [rip, +, .L__unnamed_14]
     mov     esi, 36
     call    qword, ptr, [rip, +, _ZN4core9panicking5panic17h68e56c2eeba99c8cE@GOTPCREL]
     ud2
.LBB48_14:
     mov     rbx, rax
     mov     rdi, rsp
     } (liballoc/vec.rs:691)
     call    core::ptr::drop_in_place
     mov     rdi, rbx
     call    _Unwind_Resume
     ud2
cargo-asm after
$ cargo asm --rust "<bytes::bytes::Bytes as core::convert::From<alloc::vec::Vec<u8>>>::from"
 fn from(vec: Vec<u8>) -> Bytes {
 push    rbx
 mov     rbx, rdi
 if vec.is_empty() {
 mov     rax, qword, ptr, [rsi, +, 16]
     self.len() == 0 (liballoc/vec.rs:1409)
     test    rax, rax
 if vec.is_empty() {
 je      .LBB47_1
 let mut v = mem::ManuallyDrop::new(vec);
 mov     rcx, qword, ptr, [rsi]
 if ptr as usize & 0x1 == 0 {
 test    cl, 1
 jne     .LBB47_6
 Bytes {
 mov     qword, ptr, [rbx], rcx
 let data = ptr as usize | KIND_VEC;
 or      rcx, 1
 Bytes {
 mov     qword, ptr, [rbx, +, 8], rax
 mov     qword, ptr, [rbx, +, 16], rcx
 mov     rax, qword, ptr, [rip, +, _ZN5bytes5bytes22PROMOTABLE_EVEN_VTABLE17h45f693bc6b48efcbE@GOTPCREL]
 jmp     .LBB47_7
.LBB47_1:
 Bytes {
 lea     rax, [rip, +, .L__unnamed_3]
 mov     qword, ptr, [rbx], rax
 xorps   xmm0, xmm0
 movups  xmmword, ptr, [rbx, +, 8], xmm0
 lea     rax, [rip, +, .L__unnamed_15]
 mov     qword, ptr, [rbx, +, 24], rax
     pub unsafe fn drop_in_place<T: ?Sized>(to_drop: *mut T) { (libcore/ptr/mod.rs:184)
     mov     rdi, qword, ptr, [rsi]
     if let Some((ptr, layout)) = self.current_memory() { (liballoc/raw_vec.rs:507)
     test    rdi, rdi
     if mem::size_of::<T>() == 0 || self.cap == 0 { (liballoc/raw_vec.rs:235)
     je      .LBB47_8
     mov     rsi, qword, ptr, [rsi, +, 8]
     test    rsi, rsi
     je      .LBB47_8
     unsafe { __rust_dealloc(ptr, layout.size(), layout.align()) } (liballoc/alloc.rs:102)
     mov     edx, 1
     call    qword, ptr, [rip, +, __rust_dealloc@GOTPCREL]
.LBB47_8:
 }
 mov     rax, rbx
 pop     rbx
 ret
.LBB47_6:
 Bytes {
 mov     qword, ptr, [rbx], rcx
 mov     qword, ptr, [rbx, +, 8], rax
 mov     qword, ptr, [rbx, +, 16], rcx
 mov     rax, qword, ptr, [rip, +, _ZN5bytes5bytes21PROMOTABLE_ODD_VTABLE17h7ab92e8c61ff435dE@GOTPCREL]
.LBB47_7:
 mov     qword, ptr, [rbx, +, 24], rax
 }
 mov     rax, rbx
 pop     rbx
 ret

PR use Vec::as_mut_ptr, but this method available only from 1.37.0. Is this a problem?

@Darksonn
Copy link
Contributor

You need to know the capacity when deallocating it if you don't shrink it.

@fanatid
Copy link
Contributor Author

fanatid commented Jul 20, 2020

Thank you. Allocating/deallocating is a dark area for me in Rust 😞 I see now that in drop function rebuild_boxed_slice called for creating slice for a drop.

Can you explain:

clippy output about drop:

error: calls to `std::mem::drop` with a value that implements `Copy`. Dropping a copy leaves the original intact.
   --> src/bytes.rs:781:9
    |
781 |         drop(Box::into_raw(slice));
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `#[deny(clippy::drop_copy)]` on by default
note: argument has type *mut [u8]
   --> src/bytes.rs:781:14
    |
781 |         drop(Box::into_raw(slice));
    |              ^^^^^^^^^^^^^^^^^^^^
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#drop_copy

@MOZGIII
Copy link

MOZGIII commented Jul 21, 2020

Side note: it would be great if there were tests to assert that no copying occurs, so that it’d be easy to catch the regressions here and it other places.

@fanatid fanatid closed this Jul 23, 2020
@fanatid fanatid deleted the bytesmut-freeze-improve branch July 23, 2020 20:45
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 this pull request may close these issues.

BytesMut::freeze is not zero-cost
3 participants