Merge #231

231: Avoid unnecessary synchronization in `{force,deref}_mut` r=matklad a=danielhenrymantilla

  - `DerefMut` was not delegating to `force_mut()` (contary to the by-ref APIs);
  - `Lazy::force_mut` was not taking advantage of exclusive-mutability access, and instead paying shared-mutability access. Mainly, in the `sync` case, it involved atomic operations which are now skipped.

Fixes #226

Co-authored-by: Daniel Henry-Mantilla <[email protected]>

CHANGELOG.md

@@ -4,6 +4,10 @@
 
 -
 
+## 1.17.2
+
+- Avoid unnecessary synchronization in `Lazy::{force,deref}_mut()`, [#231](https://github.com/matklad/once_cell/pull/231).
+
 ## 1.17.1
 
 - Make `OnceRef` implementation compliant with [strict provenance](https://github.com/rust-lang/rust/issues/95228).

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "once_cell"
-version = "1.17.1"
+version = "1.17.2"
 authors = ["Aleksey Kladov <[email protected]>"]
 license = "MIT OR Apache-2.0"
 edition = "2021"

src/lib.rs

@@ -796,8 +796,14 @@ pub mod unsync {
         /// assert_eq!(*lazy, 92);
         /// ```
         pub fn force_mut(this: &mut Lazy<T, F>) -> &mut T {
-            Self::force(this);
-            Self::get_mut(this).unwrap_or_else(|| unreachable!())
+            if this.cell.get_mut().is_none() {
+                let value = match this.init.get_mut().take() {
+                    Some(f) => f(),
+                    None => panic!("Lazy instance has previously been poisoned"),
+                };
+                this.cell = OnceCell::with_value(value);
+            }
+            this.cell.get_mut().unwrap_or_else(|| unreachable!())
         }
 
         /// Gets the reference to the result of this lazy value if
@@ -844,8 +850,7 @@ pub mod unsync {
 
     impl<T, F: FnOnce() -> T> DerefMut for Lazy<T, F> {
         fn deref_mut(&mut self) -> &mut T {
-            Lazy::force(self);
-            self.cell.get_mut().unwrap_or_else(|| unreachable!())
+            Lazy::force_mut(self)
         }
     }
 
@@ -1324,8 +1329,14 @@ pub mod sync {
         /// assert_eq!(Lazy::force_mut(&mut lazy), &mut 92);
         /// ```
         pub fn force_mut(this: &mut Lazy<T, F>) -> &mut T {
-            Self::force(this);
-            Self::get_mut(this).unwrap_or_else(|| unreachable!())
+            if this.cell.get_mut().is_none() {
+                let value = match this.init.get_mut().take() {
+                    Some(f) => f(),
+                    None => panic!("Lazy instance has previously been poisoned"),
+                };
+                this.cell = OnceCell::with_value(value);
+            }
+            this.cell.get_mut().unwrap_or_else(|| unreachable!())
         }
 
         /// Gets the reference to the result of this lazy value if
@@ -1372,8 +1383,7 @@ pub mod sync {
 
     impl<T, F: FnOnce() -> T> DerefMut for Lazy<T, F> {
         fn deref_mut(&mut self) -> &mut T {
-            Lazy::force(self);
-            self.cell.get_mut().unwrap_or_else(|| unreachable!())
+            Lazy::force_mut(self)
         }
     }
 

xtask/src/main.rs

@@ -48,7 +48,21 @@ fn main() -> xshell::Result<()> {
         let _s = section("TEST_MSRV");
         let _e = push_toolchain(&sh, MSRV)?;
         sh.copy_file("Cargo.lock.msrv", "Cargo.lock")?;
-        cmd!(sh, "cargo build").run()?;
+        if let err @ Err(_) = cmd!(sh, "cargo update -w -v --locked").run() {
+            // `Cargo.lock.msrv` is out of date! Probably from having bumped our own version number.
+            println! {"\
+                Error: `Cargo.lock.msrv` is out of date. \
+                Please run:\n    \
+                (cp Cargo.lock{{.msrv,}} && cargo +{MSRV} update -w -v && cp Cargo.lock{{.msrv,}})\n\
+                \n\
+                Alternatively, `git apply` the `.patch` below:\
+            "}
+            cmd!(sh, "cargo update -q -w").quiet().run()?;
+            sh.copy_file("Cargo.lock", "Cargo.lock.msrv")?;
+            cmd!(sh, "git --no-pager diff --color=always -- Cargo.lock.msrv").quiet().run()?;
+            return err;
+        }
+        cmd!(sh, "cargo build --locked").run()?;
     }
 
     {