Leverage fmt::Arguments::as_str in the write! macro
#100700
Conversation
rustbot
added
the
T-libs
|
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
rust-highfive
added
the
S-waiting-on-review
Likely this is due to |
|
@bors try @rust-timer queue |
|
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
rustbot
added
the
S-waiting-on-perf
|
⌛ Trying commit 23904e5 with merge 175bd086b773bcf4505022b8ae1192cb831763de... |
|
☀️ Try build successful - checks-actions |
|
Queued 175bd086b773bcf4505022b8ae1192cb831763de with parent 09ae784, future comparison URL. |
|
Finished benchmarking commit (175bd086b773bcf4505022b8ae1192cb831763de): comparison URL. Overall result: ❌✅ regressions and improvements - ACTION NEEDEDBenchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @bors rollup=never Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)This benchmark run did not return any relevant results for this metric. CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Footnotes |
rustbot
added
perf-regression
m-ou-se
added
S-waiting-on-author
|
Given these slightly-worse-than-expected results, do you still think it's worth merging this? |
|
The |
perf: improve write_fmt to handle simple strings Per `@dtolnay` suggestion in serde-rs/serde#2697 (comment) - attempt to speed up performance in the cases of a simple string format without arguments: ```rust write!(f, "text") -> f.write_str("text") ``` ```diff + #[inline] pub fn write_fmt(&mut self, f: fmt::Arguments) -> fmt::Result { + if let Some(s) = f.as_str() { + self.buf.write_str(s) + } else { write(self.buf, f) + } } ``` * Hopefully it will improve the simple case for the rust-lang#99012 * Another related (original?) issues rust-lang#10761 * Previous similar attempt to fix it by by `@Kobzol` rust-lang#100700 CC: `@m-ou-se` as probably the biggest expert in everything `format!`
perf: improve write_fmt to handle simple strings In case format string has no arguments, simplify its implementation with a direct call to `output.write_str(value)`. This builds on `@dtolnay` original [suggestion](serde-rs/serde#2697 (comment)). This does not change any expectations because the original `fn write()` implementation calls `write_str` for parts of the format string. ```rust write!(f, "text") -> f.write_str("text") ``` ```diff /// [`write!`]: crate::write! +#[inline] #[stable(feature = "rust1", since = "1.0.0")] pub fn write(output: &mut dyn Write, args: Arguments<'_>) -> Result { + if let Some(s) = args.as_str() { output.write_str(s) } else { write_internal(output, args) } +} + +/// Actual implementation of the [`write`], but without the simple string optimization. +fn write_internal(output: &mut dyn Write, args: Arguments<'_>) -> Result { let mut formatter = Formatter::new(output); let mut idx = 0; ``` * Hopefully it will improve the simple case for the rust-lang#99012 * Another related (original?) issues rust-lang#10761 * Previous similar attempt to fix it by by `@Kobzol` rust-lang#100700 CC: `@m-ou-se` as probably the biggest expert in everything `format!`
perf: improve write_fmt to handle simple strings In case format string has no arguments, simplify its implementation with a direct call to `output.write_str(value)`. This builds on `@dtolnay` original [suggestion](serde-rs/serde#2697 (comment)). This does not change any expectations because the original `fn write()` implementation calls `write_str` for parts of the format string. ```rust write!(f, "text") -> f.write_str("text") ``` ```diff /// [`write!`]: crate::write! +#[inline] #[stable(feature = "rust1", since = "1.0.0")] pub fn write(output: &mut dyn Write, args: Arguments<'_>) -> Result { + if let Some(s) = args.as_str() { output.write_str(s) } else { write_internal(output, args) } +} + +/// Actual implementation of the [`write`], but without the simple string optimization. +fn write_internal(output: &mut dyn Write, args: Arguments<'_>) -> Result { let mut formatter = Formatter::new(output); let mut idx = 0; ``` * Hopefully it will improve the simple case for the rust-lang#99012 * Another related (original?) issues rust-lang#10761 * Previous similar attempt to fix it by by `@Kobzol` rust-lang#100700 CC: `@m-ou-se` as probably the biggest expert in everything `format!`
perf: improve write_fmt to handle simple strings In case format string has no arguments, simplify its implementation with a direct call to `output.write_str(value)`. This builds on `@dtolnay` original [suggestion](serde-rs/serde#2697 (comment)). This does not change any expectations because the original `fn write()` implementation calls `write_str` for parts of the format string. ```rust write!(f, "text") -> f.write_str("text") ``` ```diff /// [`write!`]: crate::write! +#[inline] #[stable(feature = "rust1", since = "1.0.0")] pub fn write(output: &mut dyn Write, args: Arguments<'_>) -> Result { + if let Some(s) = args.as_str() { output.write_str(s) } else { write_internal(output, args) } +} + +/// Actual implementation of the [`write`], but without the simple string optimization. +fn write_internal(output: &mut dyn Write, args: Arguments<'_>) -> Result { let mut formatter = Formatter::new(output); let mut idx = 0; ``` * Hopefully it will improve the simple case for the rust-lang#99012 * Another related (original?) issues rust-lang#10761 * Previous similar attempt to fix it by by `@Kobzol` rust-lang#100700 CC: `@m-ou-se` as probably the biggest expert in everything `format!`
As noted by @CAD97 here, we could use
fmt::Argument::as_str()to slightly optimize the performance ofwrite!(wr, "constant")by usingas_str(). It's a small band-aid to the problem of lacklusterwrite!performance, but it could be worth it.I did some experiments with
as_strinioandfmt'swrite_fmtimplementation, and it seems that for this specific use-case it's enough to include it infmt(benchmarks are located inlibrary/core/benches/fmt.rs):Original code
as_str in io
as_str in fmt
as_str in io + fmt
I included more benchmark results here, but the interesting one is
fmt::write_str_macro1, which becomes ~twice as fast with this change. I benchmarked the code with disabled hyper-threading, ASLR and turbo-boost and enabled performance mode, but it's still quite noisy. Results are from (laptop) Zen 3.Related issue: #10761
r? @m-ou-se