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

bad performance of char literal to_string() vs str literal to_string() #73462

Closed
matthiaskrgr opened this issue Jun 18, 2020 · 6 comments · Fixed by #73465
Closed

bad performance of char literal to_string() vs str literal to_string() #73462

matthiaskrgr opened this issue Jun 18, 2020 · 6 comments · Fixed by #73465
Labels
I-slow Issue: Problems and improvements with respect to performance of generated code. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@matthiaskrgr
Copy link
Member

I was quite surprised when I found out that

pub fn conv() -> String {
    'a'.to_string()
}

generates several times the instruction count of

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

(according to godbold).
Checking the (debug) build size of println!("{}", "a".to_string()); vs println!("{}", 'a'.to_string()); revealed the str variant being ~20kb lighter.

When doing some quick-and-dirty benchmarks, the str variant was more than 2x faster than the char-variant. benchmark code

test tests::bench_char_to_string ... bench:      39,890 ns/iter (+/- 42,078)
test tests::bench_str_to_string  ... bench:      14,422 ns/iter (+/- 2,427)

Is this something that could be optimized?

@alex
Copy link
Member

alex commented Jun 18, 2020

The problem seems to be that char doesn't directly implement to_string, it comes from the blanket impl of Display. Adding an impl of ToString directly on char that looks like:

    let mut s = String::new();
    s.push(self);
    s

is probably the fix

@tesuji
Copy link
Contributor

tesuji commented Jun 18, 2020

godbolt link: https://rust.godbolt.org/z/aYEaZr
I will add a specialization of ToString for char.

@jonas-schievink jonas-schievink added I-slow Issue: Problems and improvements with respect to performance of generated code. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jun 18, 2020
@tesuji
Copy link
Contributor

tesuji commented Jun 18, 2020

Hm. {int}.to_string() is slow like char.to_string()

Manishearth added a commit to Manishearth/rust that referenced this issue Jun 18, 2020
@pickfire
Copy link
Contributor

pickfire commented Jun 20, 2020

Looks like byte and integer runs as slow.

test tests::bench_byte_to_string ... bench:      51,257 ns/iter (+/- 1,623)
test tests::bench_char_to_string ... bench:      43,246 ns/iter (+/- 7,624)
test tests::bench_int_to_string  ... bench:      52,218 ns/iter (+/- 3,077)
test tests::bench_str_to_string  ... bench:      15,701 ns/iter (+/- 380)

Benchmark code https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=a440f60fb09e7e6e4c60eef0a1ebfac1

@bors bors closed this as completed in d2272d4 Jun 20, 2020
@pickfire
Copy link
Contributor

Should we open another issue for byte and int?

@tesuji
Copy link
Contributor

tesuji commented Jun 20, 2020

yeah, do it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-slow Issue: Problems and improvements with respect to performance of generated code. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants