String::new() is slower than "".to_string()

[wooster0]

I tried this code:

pub fn a() -> String {
    String::new()
}

pub fn b() -> String {
    "".to_string()
}

I expected to see this happen: both functions contain identical code.

Instead, this happened: https://rust.godbolt.org/z/P53MMG4W6.

For String::new() it does one more instruction: mov rcx, qword ptr [rip + .L__unnamed_1]. This rcx is then read further below at mov qword ptr [rdi], rcx.
The "".to_string() version however does not do this extra step and immediately reads from a constant: mov qword ptr [rdi], 1.
This means that String::new() does slightly more work than "".to_string() but I expect them to be identical.

Before 1.52.0 the functions generated identical code: https://rust.godbolt.org/z/8647E6YhY.

oli noted the following:

The problem is probably because it copies its value from the constant

rust/library/alloc/src/raw_vec.rs

Line 64 in d620ae1

pub const NEW: Self = Self::new();

instead of immediately creating the appropriate value

Meta

This currently happens on nightly.

[wooster0]

@rustbot label regression-untriaged

[JohnTitor]

Regressed since 1.52.0, between nightly-2021-03-04 and nightly-2021-03-05 so I assume it's caused (revealed?) by #81451.

[cynecx]

Reduced llvm-ir:

target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

%"std::string::String" = type { [0 x i64], %"std::vec::Vec<u8>", [0 x i64] }
%"std::vec::Vec<u8>" = type { [0 x i64], { i8*, i64 }, [0 x i64], i64, [0 x i64] }

@0 = private unnamed_addr constant <{ [16 x i8] }> <{ [16 x i8] c"\01\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00" }>, align 8

define void @abc(%"std::string::String"* %0) unnamed_addr {
start:
  %1 = load i8*, i8** bitcast (<{ [16 x i8] }>* @0 to i8**), align 8
  %ptr = bitcast %"std::string::String"* %0 to i8**
  store i8* %1, i8** %ptr, align 8
  ret void
}

; This works by replacing the pointer type with an integral type

define void @cba(%"std::string::String"* %0) unnamed_addr {
start:
  %1 = load i64, i64* bitcast (<{ [16 x i8] }>* @0 to i64*), align 8
  %ptr = bitcast %"std::string::String"* %0 to i64*
  store i64 %1, i64* %ptr, align 8
  ret void
}

LLVM fails to figure out that the load can be replaced with a constant (Probably because of the load bitcast pattern for pointer types).

[klensy]

It isn't only in String main thing there in const. The same can be seen in vec:

pub fn foo()->Vec<u8>{
    vec![]
}

pub fn foo()->Vec<u8>{
    vec![].clone()
}

example::foo:
        mov     rax, rdi
        mov     rcx, qword ptr [rip + .L__unnamed_1]
        mov     qword ptr [rdi], rcx
        xorps   xmm0, xmm0
        movups  xmmword ptr [rdi + 8], xmm0
        ret

.L__unnamed_1:
        .asciz  "\001\000\000\000\000\000\000\000\000\000\000\000\000\000\000"


example::foo:
        mov     rax, rdi
        mov     qword ptr [rdi], 1
        xorps   xmm0, xmm0
        movups  xmmword ptr [rdi + 8], xmm0
        ret

[apiraino]

Assigning priority as discussed in the Zulip thread of the Prioritization Working Group.

@rustbot label -I-prioritize +P-high

[klensy]

This can be somewhat solved by using RawVec::new() directly, without pub const NEW: Self = Self::new(); hack, i.e. for Vec:

    pub const fn new() -> Self {
        Vec { buf: RawVec::new(), len: 0 }
    }

instead of

    pub const fn new() -> Self {
        Vec { buf: RawVec::NEW, len: 0 }
    }

but i been hit by

error: `RawVec::<T>::new` is not yet stable as a const fn
   --> library\alloc\src\vec\mod.rs:420:20
    |
420 |         Vec { buf: RawVec::new(), len: 0 }
    |                    ^^^^^^^^^^^^^
    |
    = help: Const-stable functions can only call other const-stable functions

error: aborting due to previous error

Tried solving this by placing rustc_allow_const_fn_unstable in some places, but no luck. Any suggestions?

[nbdd0121]

Seems like you hit #75794.

[lqd]

RawVec::new is pub, so maybe not ?

It could be because these stability changes are seen by the bootstrap compiler and not your own version of liballoc.

There are a few examples of attributes needing to be cfg_attred out until a new version of the compiler with your changes is used for bootstrap (and then these attributes are removed when that happens.)

Depending on whether you need to target the bootstrap compiler, or to "temporarily hide" your changes from it, it can look like #![cfg_attr(not(bootstrap), ...)].

[klensy]

@lqd
Tried to change Vec to:

    #[inline]
    #[rustc_const_stable(feature = "const_vec_new", since = "1.39.0")]
    #[stable(feature = "rust1", since = "1.0.0")]
    #[cfg(bootstrap)]
    pub const fn new() -> Self {
        Vec { buf: RawVec::NEW, len: 0 }
    }

    #[inline]
    #[rustc_const_stable(feature = "const_vec_new", since = "1.39.0")]
    #[stable(feature = "rust1", since = "1.0.0")]
    #[cfg(not(bootstrap))]
    pub const fn new() -> Self {
        Vec { buf: RawVec::new(), len: 0 }
    }

and RawVec to

    #[rustc_allow_const_fn_unstable(const_fn)]
    #[cfg_attr(not(bootstrap), rustc_const_unstable(feature="rawvec_const_new_fake_stable", issue="99999"))]
    pub const fn new() -> Self {
        Self::new_in(Global)
    }

with adding #![feature(rawvec_const_new_fake_stable)]

But this clearly wrong (tried few permutations of cfg's), as error the same.

[lqd]

Asking about how to handle this on zulip, e.g. in the t-libs stream, would probably be best then.

[klensy]

Found workaround, but this bloats rcgu size a lot, sadly.

[nikic]

Fixed upstream by llvm/llvm-project@6a19cb8.

[SaadiSave]

Fixed upstream by llvm/llvm-project@6a19cb8.

I assume that this will make its way into Rust with LLVM 14? Or will it become a patch release?

[nikic]

Fixed upstream by llvm/llvm-project@6a19cb8.

I assume that this will make its way into Rust with LLVM 14? Or will it become a patch release?

This will be part of the LLVM 14 upgrade.

[klensy]

Fixed by #93577 https://rust.godbolt.org/z/vMdfPzfT5, but test should be added, to check regressions in future.