From d712e3f4f4b7b002f984336031a13c864bea10b8 Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Mon, 12 Feb 2024 23:44:28 -0500 Subject: [PATCH 1/6] perf: improve write_fmt to handle simple strings Per @dtolnay suggestion in https://github.com/serde-rs/serde/pull/2697#issuecomment-1940376414 - attempt to speed up performance in the cases of a simple string format without arguments: ```rust write!(f, "text") -> f.write_str("text") ``` --- library/core/src/fmt/mod.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/library/core/src/fmt/mod.rs b/library/core/src/fmt/mod.rs index e1b7b46a1ed2f..f95ab8dd5f393 100644 --- a/library/core/src/fmt/mod.rs +++ b/library/core/src/fmt/mod.rs @@ -201,14 +201,14 @@ pub trait Write { impl SpecWriteFmt for &mut W { #[inline] default fn spec_write_fmt(mut self, args: Arguments<'_>) -> Result { - write(&mut self, args) + if let Some(s) = args.as_str() { self.write_str(s) } else { write(&mut self, args) } } } impl SpecWriteFmt for &mut W { #[inline] fn spec_write_fmt(self, args: Arguments<'_>) -> Result { - write(self, args) + if let Some(s) = args.as_str() { self.write_str(s) } else { write(self, args) } } } @@ -1582,8 +1582,9 @@ impl<'a> Formatter<'a> { /// assert_eq!(format!("{:0>8}", Foo(2)), "Foo 2"); /// ``` #[stable(feature = "rust1", since = "1.0.0")] + #[inline] pub fn write_fmt(&mut self, fmt: Arguments<'_>) -> Result { - write(self.buf, fmt) + if let Some(s) = fmt.as_str() { self.buf.write_str(s) } else { write(self.buf, fmt) } } /// Flags for formatting @@ -2272,8 +2273,9 @@ impl Write for Formatter<'_> { self.buf.write_char(c) } + #[inline] fn write_fmt(&mut self, args: Arguments<'_>) -> Result { - write(self.buf, args) + if let Some(s) = args.as_str() { self.buf.write_str(s) } else { write(self.buf, args) } } } From 6fa7d6ca16f5f3704bfd48fd3721e8ea329e971b Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Wed, 14 Feb 2024 18:30:24 -0500 Subject: [PATCH 2/6] Use intrinsic --- library/core/src/fmt/mod.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/library/core/src/fmt/mod.rs b/library/core/src/fmt/mod.rs index f95ab8dd5f393..dde8c0c5e2b64 100644 --- a/library/core/src/fmt/mod.rs +++ b/library/core/src/fmt/mod.rs @@ -430,6 +430,19 @@ impl<'a> Arguments<'a> { _ => None, } } + + /// Same as `as_str`, but will only return a `Some` value if it can be determined at compile time. + #[inline] + const fn as_const_str(&self) -> Option<&'static str> { + let s = self.as_str(); + // if unsafe { core::intrinsics::is_val_statically_known(matches!((self.pieces, self.args), ([], []) | ([_], []))) } { + if unsafe { core::intrinsics::is_val_statically_known(s) } { + s + } else { + None + } + + } } #[stable(feature = "rust1", since = "1.0.0")] From c50779fc78752c109d41108da0ef2b284d5a9462 Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Wed, 14 Feb 2024 18:48:02 -0500 Subject: [PATCH 3/6] Fix inlining issue for non-const case --- library/core/src/fmt/mod.rs | 31 +++++++++++++++++++------------ library/core/src/lib.rs | 1 + 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/library/core/src/fmt/mod.rs b/library/core/src/fmt/mod.rs index dde8c0c5e2b64..3c9cd093ad86e 100644 --- a/library/core/src/fmt/mod.rs +++ b/library/core/src/fmt/mod.rs @@ -201,14 +201,22 @@ pub trait Write { impl SpecWriteFmt for &mut W { #[inline] default fn spec_write_fmt(mut self, args: Arguments<'_>) -> Result { - if let Some(s) = args.as_str() { self.write_str(s) } else { write(&mut self, args) } + if let Some(s) = args.as_const_str() { + self.write_str(s) + } else { + write(&mut self, args) + } } } impl SpecWriteFmt for &mut W { #[inline] fn spec_write_fmt(self, args: Arguments<'_>) -> Result { - if let Some(s) = args.as_str() { self.write_str(s) } else { write(self, args) } + if let Some(s) = args.as_const_str() { + self.write_str(s) + } else { + write(self, args) + } } } @@ -431,17 +439,12 @@ impl<'a> Arguments<'a> { } } - /// Same as `as_str`, but will only return a `Some` value if it can be determined at compile time. + /// Same as [`Arguments::as_str`], but will only return `Some(s)` if it can be determined at compile time. + #[must_use] #[inline] const fn as_const_str(&self) -> Option<&'static str> { let s = self.as_str(); - // if unsafe { core::intrinsics::is_val_statically_known(matches!((self.pieces, self.args), ([], []) | ([_], []))) } { - if unsafe { core::intrinsics::is_val_statically_known(s) } { - s - } else { - None - } - + if unsafe { core::intrinsics::is_val_statically_known(s.is_some()) } { s } else { None } } } @@ -1597,7 +1600,7 @@ impl<'a> Formatter<'a> { #[stable(feature = "rust1", since = "1.0.0")] #[inline] pub fn write_fmt(&mut self, fmt: Arguments<'_>) -> Result { - if let Some(s) = fmt.as_str() { self.buf.write_str(s) } else { write(self.buf, fmt) } + if let Some(s) = fmt.as_const_str() { self.buf.write_str(s) } else { write(self.buf, fmt) } } /// Flags for formatting @@ -2288,7 +2291,11 @@ impl Write for Formatter<'_> { #[inline] fn write_fmt(&mut self, args: Arguments<'_>) -> Result { - if let Some(s) = args.as_str() { self.buf.write_str(s) } else { write(self.buf, args) } + if let Some(s) = args.as_const_str() { + self.buf.write_str(s) + } else { + write(self.buf, args) + } } } diff --git a/library/core/src/lib.rs b/library/core/src/lib.rs index b54680a61b4d8..ffaf8af0b11d2 100644 --- a/library/core/src/lib.rs +++ b/library/core/src/lib.rs @@ -176,6 +176,7 @@ #![feature(ip)] #![feature(ip_bits)] #![feature(is_ascii_octdigit)] +#![feature(is_val_statically_known)] #![feature(isqrt)] #![feature(maybe_uninit_uninit_array)] #![feature(non_null_convenience)] From 377594dcedf812dab65adfe2f892a5438b780731 Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Wed, 14 Feb 2024 18:56:21 -0500 Subject: [PATCH 4/6] add safety text --- library/core/src/fmt/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/library/core/src/fmt/mod.rs b/library/core/src/fmt/mod.rs index 3c9cd093ad86e..a1513515c09bf 100644 --- a/library/core/src/fmt/mod.rs +++ b/library/core/src/fmt/mod.rs @@ -444,6 +444,7 @@ impl<'a> Arguments<'a> { #[inline] const fn as_const_str(&self) -> Option<&'static str> { let s = self.as_str(); + // SAFETY: both cases are valid as the result if unsafe { core::intrinsics::is_val_statically_known(s.is_some()) } { s } else { None } } } From 8362b30bba773d6bbc51c428f43f732f7665c4bd Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Wed, 14 Feb 2024 20:44:46 -0500 Subject: [PATCH 5/6] remove const --- library/core/src/fmt/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/fmt/mod.rs b/library/core/src/fmt/mod.rs index a1513515c09bf..47ebf83505337 100644 --- a/library/core/src/fmt/mod.rs +++ b/library/core/src/fmt/mod.rs @@ -442,7 +442,7 @@ impl<'a> Arguments<'a> { /// Same as [`Arguments::as_str`], but will only return `Some(s)` if it can be determined at compile time. #[must_use] #[inline] - const fn as_const_str(&self) -> Option<&'static str> { + fn as_const_str(&self) -> Option<&'static str> { let s = self.as_str(); // SAFETY: both cases are valid as the result if unsafe { core::intrinsics::is_val_statically_known(s.is_some()) } { s } else { None } From 1eee9f580724fe73ac96248b2d9f10e5a9911744 Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Thu, 15 Feb 2024 17:43:48 -0500 Subject: [PATCH 6/6] A much simpler version of write --- library/core/src/fmt/mod.rs | 37 ++++++++++--------------------------- library/core/src/lib.rs | 1 - 2 files changed, 10 insertions(+), 28 deletions(-) diff --git a/library/core/src/fmt/mod.rs b/library/core/src/fmt/mod.rs index 47ebf83505337..17e58eaac8846 100644 --- a/library/core/src/fmt/mod.rs +++ b/library/core/src/fmt/mod.rs @@ -201,22 +201,14 @@ pub trait Write { impl SpecWriteFmt for &mut W { #[inline] default fn spec_write_fmt(mut self, args: Arguments<'_>) -> Result { - if let Some(s) = args.as_const_str() { - self.write_str(s) - } else { - write(&mut self, args) - } + write(&mut self, args) } } impl SpecWriteFmt for &mut W { #[inline] fn spec_write_fmt(self, args: Arguments<'_>) -> Result { - if let Some(s) = args.as_const_str() { - self.write_str(s) - } else { - write(self, args) - } + write(self, args) } } @@ -438,15 +430,6 @@ impl<'a> Arguments<'a> { _ => None, } } - - /// Same as [`Arguments::as_str`], but will only return `Some(s)` if it can be determined at compile time. - #[must_use] - #[inline] - fn as_const_str(&self) -> Option<&'static str> { - let s = self.as_str(); - // SAFETY: both cases are valid as the result - if unsafe { core::intrinsics::is_val_statically_known(s.is_some()) } { s } else { None } - } } #[stable(feature = "rust1", since = "1.0.0")] @@ -1119,8 +1102,14 @@ pub trait UpperExp { /// ``` /// /// [`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; @@ -1599,9 +1588,8 @@ impl<'a> Formatter<'a> { /// assert_eq!(format!("{:0>8}", Foo(2)), "Foo 2"); /// ``` #[stable(feature = "rust1", since = "1.0.0")] - #[inline] pub fn write_fmt(&mut self, fmt: Arguments<'_>) -> Result { - if let Some(s) = fmt.as_const_str() { self.buf.write_str(s) } else { write(self.buf, fmt) } + write(self.buf, fmt) } /// Flags for formatting @@ -2290,13 +2278,8 @@ impl Write for Formatter<'_> { self.buf.write_char(c) } - #[inline] fn write_fmt(&mut self, args: Arguments<'_>) -> Result { - if let Some(s) = args.as_const_str() { - self.buf.write_str(s) - } else { - write(self.buf, args) - } + write(self.buf, args) } } diff --git a/library/core/src/lib.rs b/library/core/src/lib.rs index ffaf8af0b11d2..b54680a61b4d8 100644 --- a/library/core/src/lib.rs +++ b/library/core/src/lib.rs @@ -176,7 +176,6 @@ #![feature(ip)] #![feature(ip_bits)] #![feature(is_ascii_octdigit)] -#![feature(is_val_statically_known)] #![feature(isqrt)] #![feature(maybe_uninit_uninit_array)] #![feature(non_null_convenience)]