From 8d0911786582704bcb6afd50a0dfb02bc9494fa8 Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Mon, 12 Feb 2024 23:44:28 -0500 Subject: [PATCH 1/4] 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 4a691babaa426131debd10d58751061248c241f7 Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Wed, 14 Feb 2024 18:30:24 -0500 Subject: [PATCH 2/4] 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 a07c05d573d58d272182ec2b98e6d67945527665 Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Wed, 14 Feb 2024 18:48:02 -0500 Subject: [PATCH 3/4] 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 e65e940f1e1cc2284f91c27d66f5cdad8460c5fd Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Wed, 14 Feb 2024 18:56:21 -0500 Subject: [PATCH 4/4] 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 } } }