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

fmt: Skip calling write_str for empty strings #31966

Closed
wants to merge 1 commit into from

Conversation

bluss
Copy link
Member

@bluss bluss commented Feb 29, 2016

fmt: Skip calling write_str for empty strings

format_args! uses one string literal per used {}, in many cases, the
separating string pieces are empty.

For example, write!(w, "{}", x); will attempt to write one empty
string before the {} format, and write!(w, "{}{}{}", x, y, z);" will
write one empty string piece between each format.

Simply skip writing if the string is empty. It is a cheap branch
compared to the virtual Write::write_str call that it makes possible to
skip.

It does not solve issue #10761 in any way, yet that's where I noticed
this. The testcase in the issue shows a performance difference between
write!(w, "abc") and write!(w, "{}", "abc"), and this change halves
the size of the difference.

`format_args!` uses one string literal per used `{}`, in many cases, the
separating string pieces are empty.

For example, `write!(w, "{}", x);` will attempt to write one empty
string before the `{}` format, and `write!(w, "{}{}{}", x, y, z);" will
write three empty string pieces between formats.

Simply skip writing if the string is empty. It is a cheap branch
compared to the virtual Write::write_str call that it makes possible to
skip.

It does not solve issue rust-lang#10761 in any way, yet that's where I noticed
this. The testcase in the issue shows a performance difference between
`write!(w, "abc")` and `write!(w, "{}", "abc")`, and this change halves
the size of the difference.
@rust-highfive
Copy link
Collaborator

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton
Copy link
Member

I wonder if it may make sense to optimize away these at compile time instead of runtime? They're taking up space in rodata and it may make sense to get rid of them? That may be a bit difficult, however, as we'd have to describe what piece is used when

@bluss
Copy link
Member Author

bluss commented Feb 29, 2016

Yes, that's even better. I don't know how all the pieces work, but maybe there's a way to do it.

@bors
Copy link
Contributor

bors commented Mar 23, 2016

☔ The latest upstream changes (presumably #32390) made this pull request unmergeable. Please resolve the merge conflicts.

@bluss
Copy link
Member Author

bluss commented Mar 23, 2016

Closing for now. Benchmarks on this weren't that radical anyway (less than 10% savings or so).

@bluss bluss closed this Mar 23, 2016
tbu- added a commit to tbu-/rust that referenced this pull request Mar 21, 2017
This means that `writeln!(something, "{}{}", s1, s2)` no longer calls
`write_str` two times with empty strings (before each substitution).

See rust-lang#31966 for an earlier attempt.
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.

4 participants