Little improves in CString new when creating from slice

[AngelicosPhosphoros]

Old code already contain optimization for cases with &str and &[u8] args. This commit adds a specialization for &mut[u8] too.

Also, I added usage of old slice in search for zero bytes instead of new buffer because it produce better code for constant inputs on Windows LTO builds. For other platforms, this wouldn't cause any difference because it calls libc anyway.

Inlined _new method into spec trait to reduce amount of code generated to CString::new callers.

[rust-highfive]

r? @kennytm

(rust-highfive has picked a reviewer for you, use r? to override)

[AngelicosPhosphoros]

I checked generated asm for CString::new call using such dylib:

use std::ffi::{CString, NulError};

#[no_mangle]
pub extern "Rust" fn make_c_string()->Result<CString, NulError>{
    CString::new("Hello world!")
}

Asm generated using command: rustc +stage1 --crate-type=cdylib cstring_new.rs --emit=asm -Copt-level=3 -Ccodegen-units=1

Old generated code

    .text
    .def     @feat.00;
    .scl    3;
    .type   0;
    .endef
    .globl  @feat.00
.set @feat.00, 0
    .file   "cstring_new.a85eb7bc-cgu.0"
    .def     make_c_string;
    .scl    2;
    .type   32;
    .endef
    .section    .text,"xr",one_only,make_c_string
    .globl  make_c_string
    .p2align    4, 0x90
make_c_string:
.seh_proc make_c_string
    pushq   %rsi
    .seh_pushreg %rsi
    pushq   %rdi
    .seh_pushreg %rdi
    subq    $56, %rsp
    .seh_stackalloc 56
    .seh_endprologue
    movq    %rcx, %rsi
    leaq    __unnamed_1(%rip), %rdx
    leaq    32(%rsp), %rdi
    movl    $12, %r8d
    movq    %rdi, %rcx
    callq   _ZN70_$LT$$RF$str$u20$as$u20$std..ffi..c_str..CString..new..SpecIntoVec$GT$8into_vec17h42e14556a4e783e0E
    movq    %rsi, %rcx
    movq    %rdi, %rdx
    callq   _ZN3std3ffi5c_str7CString4_new17hd06ee8640c50ad96E
    movq    %rsi, %rax
    addq    $56, %rsp
    popq    %rdi
    popq    %rsi
    retq
    .seh_endproc

    .section    .rdata,"dr",one_only,__unnamed_1
__unnamed_1:
    .ascii  "Hello world!"

New generated code

    .text
    .def     @feat.00;
    .scl    3;
    .type   0;
    .endef
    .globl  @feat.00
.set @feat.00, 0
    .file   "cstring_new.558cd7f7-cgu.0"
    .def     make_c_string;
    .scl    2;
    .type   32;
    .endef
    .section    .text,"xr",one_only,make_c_string
    .globl  make_c_string
    .p2align    4, 0x90
make_c_string:
.seh_proc make_c_string
    pushq   %rsi
    .seh_pushreg %rsi
    subq    $32, %rsp
    .seh_stackalloc 32
    .seh_endprologue
    movq    %rcx, %rsi
    leaq    __unnamed_1(%rip), %rdx
    movl    $12, %r8d
    callq   _ZN66_$LT$$RF$str$u20$as$u20$std..ffi..c_str..CString..new..NewImpl$GT$8new_impl17ha5748a2c8ce696e6E
    movq    %rsi, %rax
    addq    $32, %rsp
    popq    %rsi
    retq
    .seh_endproc

    .section    .rdata,"dr",one_only,__unnamed_1
__unnamed_1:
    .ascii  "Hello world!"

It can be seen that new caller code is shorter.

As for lto performance, I compiled same code used rustc +stage1 --crate-type=cdylib cstring_new.rs -Copt-level=3 -Ccodegen-units=1 -Clto=fat then looked it using IdaPRO (on Windows).
When I used memchr on new buffer, there is unrolled loop which checks each byte of new buffer if it is zero and branch on it. When I use self, compiler eliminates check completely.

On Unix, there is probably no difference because Rust uses libc implementation anyway in that case.

[AngelicosPhosphoros]

I have 2 questions:

1. Should I prefer uninit allocation instead of vec![0u8; ...] to avoid zeroing memory? It is easy to add, I just not sure if I should.
2. Maybe it is good idea to make some methods #[inline], specifically CString::from_vec_unchecked and implementation of NewImpl::<&[u8]>::new_impl because currently first one is not inlined into new_impl() and second one not inlined for impls for &mut [u8] and &str despite them being inside std. Should I do that?

Example:

; _$LT$$RF$mut$u20$$u5b$u8$u5d$$u20$as$u20$std..ffi..c_str..CString..new..NewImpl$GT$::new_impl::hcd1310bd9104df0d
_ZN83_$LT$$RF$mut$u20$$u5b$u8$u5d$$u20$as$u20$std__ffi__c_str__CString__new__NewImpl$GT$8new_impl17hcd1310bd9104df0dE proc near
push    rsi
sub     rsp, 20h
mov     rsi, rcx
call    _ZN75_$LT$$RF$$u5b$u8$u5d$$u20$as$u20$std__ffi__c_str__CString__new__NewImpl$GT$8new_impl17h8473efdc7dbe7405E ; _$LT$$RF$$u5b$u8$u5d$$u20$as$u20$std..ffi..c_str..CString..new..NewImpl$GT$::new_impl::h8473efdc7dbe7405
mov     rax, rsi
add     rsp, 20h
pop     rsi
retn
_ZN83_$LT$$RF$mut$u20$$u5b$u8$u5d$$u20$as$u20$std__ffi__c_str__CString__new__NewImpl$GT$8new_impl17hcd1310bd9104df0dE endp

[AngelicosPhosphoros]

Surprisingly, adding #[inline] doesn't change generated code in std.

[AngelicosPhosphoros]

It turns out that with_capacity already returns vector with exact capacity needed.
Would rewrite.

[AngelicosPhosphoros]

Removed all code related to new allocation mechanism.
I think, other changes (especially &mut [u8] specialization) still useful so I would keep PR open.

[AngelicosPhosphoros]

Code with LTO on Windows with current nightly.

Code with LTO on Windows with my fixes.

[AngelicosPhosphoros]

JFYI: I wouldn't be able to participate until 15th January. I would return to this PR after that if needed.

[AngelicosPhosphoros]

@kennytm May you review my PR please? :)

[Mark-Simulacrum]

This seems okay to me, thanks.

@bors r+

[bors]

📌 Commit 4b62a77 has been approved by Mark-Simulacrum