serde-rs / serde

Serialization framework for Rust
https://serde.rs/
Apache License 2.0
8.82k stars 748 forks source link

A few minor `write_str` optimizations #2697

Closed nyurik closed 4 months ago

nyurik commented 4 months ago

Apparently write! generates more code than write_str when used with a simple string, so optimizing it.

oli-obk commented 4 months ago

This sounds like something we could fix directly in libstd. Do you know of any attempts of doing that, or why it hasn't been done? We do similar trickery for format! after all

nyurik commented 4 months ago

I tried it with https://rust.godbolt.org/z/Y8djWsq1P

This issue is also tracked in https://github.com/rust-lang/rust/issues/99012 - but it appears to be highly complex and nowhere near being solved in a near future, thus warranting a workaround at least in the popular crates.

dtolnay commented 4 months ago

(I have not read most of https://github.com/rust-lang/rust/issues/99012.)

As I understand it, write!(f, "...") does not optimize in a straightforward way to f.write_str("...") because:

I think there is a libstd fix and a compiler fix that are worth trying:

  1. Standard library: change core::fmt::Formatter::write_fmt (and maybe others) to the following, and see how bad the regression is.

    + #[inline]
      pub fn write_fmt(&mut self, f: fmt::Arguments) -> fmt::Result {
    +     if let ([s], []) = (f.pieces, f.args) {
    +         self.buf.write_str(s)
    +     } else {
              write(self.buf, f)
    +     }
      }
  2. Compiler voodoo. Make write! lower directly to a dedicated AST node, the way format_args! does since https://github.com/rust-lang/rust/pull/106745. If the receiver ends up being core::fmt::Formatter (or one of a small hardcoded set of other types) then it turns into write_str, otherwise it needs to be handled in a way that is indistinguishable from f.write_fmt(...) — no idea how feasible this is.

nyurik commented 4 months ago

@dtolnay thx so much for the in-depth write-up! TIL. I can try the first approach (relatively simple). The second one clearly involves a lot more in-depth compiler knowledge... maybe someday

nyurik commented 4 months ago

P.S. Turns out Arguments::as_str already does all the needed detections!

nyurik commented 4 months ago

I tried the above suggestion, and it seems to already produce a significantly smaller assembly as seen in https://github.com/rust-lang/rust/pull/121001#issuecomment-1940574019

My only concern is that it will impact either the build time or the bin size in some other case... guess will have to wait for the results...