String::from and str::to_owned are not inlined

[nagisa]

#![crate_type="rlib"]

pub fn banana() -> String {
    String::from("")
}

pub fn peach() -> String {
    "".to_owned()
}

pub fn expected() -> String {
    unsafe {
    String::from_utf8_unchecked("".as_bytes().to_owned())
    }
}

Playground (Use show assembly or llvm-ir, make sure to turn on optimisations)

All these functions should be equivalent.

[nagisa]

A fix would involve adding an #[inline] attribute in following locations:

rust/src/liballoc/string.rs

Line 2201 in 727eabd

fn from(s: &'a str) -> String {

rust/src/liballoc/str.rs

Line 206 in 727eabd

fn to_owned(&self) -> String {

[nagisa]

It might be prudent to also investigate whether similar one-liner functions are inlined (if they haven’t an #[inline] attribute already).

[matthiaskrgr]

I can have a look tomorrow.
I played around with -C inline-threshold earlier on godbolt but it didn't seem to have an effect at all which I don't quite understand https://rust.godbolt.org/z/b3Se3j

[matthiaskrgr]

Note on the side: for "", expected() has the fewest instructions, but if I pass a none zero &str into the functions expected() has the most instns (might still be worth it, may need benchmarks) https://rust.godbolt.org/z/ep1ivP

[nagisa]

-Cinline-threshold does not work in this scenario because this function already has an instantiation in liballoc (i.e another codegen unit) and therefore is not available for inlining at all. #[inline] would make it available for inlining again, I believe.

if I pass a none zero &str into the functions expected() has the most instns (might still be worth it, may need benchmarks

Right, because it starts allocating stuff. It will result in larger binary (if actually inlined), but that already happens for [T]::to_owned, so it should be fine for str::to_owned as well.

[ollie27]

The #[inline] attribute was actually removed by #32182.

[matthiaskrgr]

Interesting, so if I understand correctly the motivation for the "outlining" was to make the functions generate the same code by llvm.
I guess it's ok if we mark all of them #[inline]?

[ollie27]

Adding #[inline] might bloat binary sizes which I believe is why it was removed. You'll have to test if it's actually worth it.

[matthiaskrgr]

I did a quick and dirty test and rust/build/x86_64-unknown-linux-gnu/stage2/lib size went from 212_929_188 bytes to 213_310_526 bytes, so an increase of 381_338 bytes.

[matthiaskrgr]

Here are the size change numbers of some tools (sizes in bytes):

binary	master	inline	diff	%diff
cargo	12852720	12900008	+47288	+0.36%
clippy-driver	5561512	5579672	+18160	+0.34%
rls	22212128	22347944	+135816	+0.61%
rustfmt	7744464	7768376	+23912	+0.30%

[jens1o]

How much faster is it?