From 1a885da2314fc7a661a246027a44ec3b14be6a1d Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Thu, 6 Feb 2025 10:58:12 -0800 Subject: [PATCH] Make initialization in `OnceNonZeroUsize::get_or_try_init` `#[cold]`. In typical use, the Some branch is going to be taken extremely frequently but the None branch is only going to be taken once (or a very small number of times) at startup. If `get_or_try_init` is in a performance-sensitive segment of code, then it is important that `get_or_try_init` be inlined and that the compiler understands that the Some branch is (much) more likely than the None branch. When this happens, the call site basically becomes a load followed by a conditional jump that is basically never taken; which is ideal. When `get_or_try_init` is used in many places in the user's code, it is important to avoid inlining any of the None branch into the call sites. Unfortunately, the Rust compiler is sometimes not good at recognizing that code that calls a #[cold] function unconditionally must be cold itself. So, sometimes it isn't enough to mark our f as `#[cold] #[inline(never)]`. Move the entire body of the None branch into a function that is marked because some post-inlining optimization passes in the compiler seem to not understand `#[cold]`, and because we don't want any part of that branch to be in the calling code. --- CHANGELOG.md | 2 ++ Cargo.lock.msrv | 2 +- Cargo.toml | 2 +- src/race.rs | 28 +++++++++++++++------------- 4 files changed, 19 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 38c3bfc..cf82a81 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +- Outline initialization in `race`: [#273](https://github.com/matklad/once_cell/pull/273). + ## 1.20.2 - Remove `portable_atomic` from Cargo.lock if it is not, in fact, used: [#267](https://github.com/matklad/once_cell/pull/267) diff --git a/Cargo.lock.msrv b/Cargo.lock.msrv index 940448b..6ba62d1 100644 --- a/Cargo.lock.msrv +++ b/Cargo.lock.msrv @@ -43,7 +43,7 @@ checksum = "78ca9ab1a0babb1e7d5695e3530886289c18cf2f87ec19a575a0abdce112e3a3" [[package]] name = "once_cell" -version = "1.20.2" +version = "1.20.3" dependencies = [ "critical-section", "parking_lot_core", diff --git a/Cargo.toml b/Cargo.toml index 186d071..32a9894 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "once_cell" -version = "1.20.2" +version = "1.20.3" authors = ["Aleksey Kladov "] license = "MIT OR Apache-2.0" edition = "2021" diff --git a/src/race.rs b/src/race.rs index 9c09323..5ac30ec 100644 --- a/src/race.rs +++ b/src/race.rs @@ -93,19 +93,21 @@ impl OnceNonZeroUsize { F: FnOnce() -> Result, { let val = self.inner.load(Ordering::Acquire); - let res = match NonZeroUsize::new(val) { - Some(it) => it, - None => { - let mut val = f()?.get(); - let exchange = - self.inner.compare_exchange(0, val, Ordering::AcqRel, Ordering::Acquire); - if let Err(old) = exchange { - val = old; - } - unsafe { NonZeroUsize::new_unchecked(val) } - } - }; - Ok(res) + match NonZeroUsize::new(val) { + Some(it) => Ok(it), + None => self.init(f), + } + } + + #[cold] + #[inline(never)] + fn init(&self, f: impl FnOnce() -> Result) -> Result { + let mut val = f()?.get(); + let exchange = self.inner.compare_exchange(0, val, Ordering::AcqRel, Ordering::Acquire); + if let Err(old) = exchange { + val = old; + } + Ok(unsafe { NonZeroUsize::new_unchecked(val) }) } }