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

Optimizing runtime performance for the literal string case #39

Closed
nyurik opened this issue Feb 10, 2024 · 4 comments
Closed

Optimizing runtime performance for the literal string case #39

nyurik opened this issue Feb 10, 2024 · 4 comments

Comments

@nyurik
Copy link
Contributor

nyurik commented Feb 10, 2024

Rust compiler at present is unable to generate as efficient code for write!(f, "string") as it can do for f.write_str("string"). I tried to look through the code, but it seems fairly complex in how you generate the output token stream. Would it be possible to handle the case when the string is known ahead of time and does not contain either { or } characters, and use the write_str instead?

@frozenlib
Copy link
Owner

fn format_args(
&self,
context: DisplayContext,
with: &Option<Expr>,
bounds: &mut Bounds,
generics: &GenericParamSet,
) -> Result<TokenStream> {

Currently, TokenStream used as an argument for write_str is being generated by the format_args function above. However, it seems possible to generate code that uses write_str instead of write! by changing the format_args function to generate something like Vec<FormatPart> below instead of TokenStream.

enum FormatPart {
  Str(String),
  Placeholder {
    format_specifier: String,
    expr : Expr,
  }
}

I would like to make this change in the future if the performance impact is significant.

@nyurik
Copy link
Contributor Author

nyurik commented Feb 11, 2024

Thanks! I made a similar suggestion in dtolnay/thiserror#285 and the pending pull request in dtolnay/thiserror#286 . You can see the assembly difference here Profiling would be fairly tricky though, as it would have to have some sort of a re-usable buffer whose allocation would not impact the profiling results.

It does sound the code here is much more involved than in the thiserror. There it first forms a struct with all the relevant info, and then converts that struct into a tokenstream. I will try to decipher it, but might take a bit of time :)

@nyurik
Copy link
Contributor Author

nyurik commented Feb 12, 2024

P.S. Note that there an another optimizations added to thiserror - dtolnay/thiserror@8a5c4d1 - in case when there is an unused comma, e.g. #[display("{}",)]

@frozenlib
Copy link
Owner

Thanks for the info.

[ ] Make sure write!(f, "literal") is just as efficient as f.write_str("literal")
source: rust-lang/rust#99012

Since ::core::write!() will be optimized in the future, I thought it would be better to modify it in a way that would make it easier to revert, and I came up with a simple way to do so and implemented it.

I also did some benchmarking and it seems to make quite a difference.

before:

test no_placeholder_by_hand_write_str ... bench:           7 ns/iter (+/- 0)
test no_placeholder_derive            ... bench:          12 ns/iter (+/- 0)

after:

test no_placeholder_by_hand_write_str ... bench:           7 ns/iter (+/- 0)
test no_placeholder_derive            ... bench:           7 ns/iter (+/- 1)

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

No branches or pull requests

2 participants