Patchwork D11405: rust: Move lazy initialization of `Repo::dirstate_map` into a generic struct

login
register
mail settings
Submitter phabricator
Date Sept. 13, 2021, 6:13 p.m.
Message ID <differential-rev-PHID-DREV-v7537fmly6ekqheafxdz-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/49733/
State Superseded
Headers show

Comments

phabricator - Sept. 13, 2021, 6:13 p.m.
SimonSapin created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  More components of `Repo` will be added following the same pattern.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D11405

AFFECTED FILES
  rust/hg-core/src/repo.rs

CHANGE DETAILS




To: SimonSapin, #hg-reviewers
Cc: mercurial-patches, mercurial-devel

Patch

diff --git a/rust/hg-core/src/repo.rs b/rust/hg-core/src/repo.rs
--- a/rust/hg-core/src/repo.rs
+++ b/rust/hg-core/src/repo.rs
@@ -23,7 +23,7 @@ 
     config: Config,
     // None means not known/initialized yet
     dirstate_parents: Cell<Option<DirstateParents>>,
-    dirstate_map: RefCell<Option<OwningDirstateMap>>,
+    dirstate_map: LazyCell<OwningDirstateMap, DirstateError>,
 }
 
 #[derive(Debug, derive_more::From)]
@@ -196,7 +196,7 @@ 
             dot_hg,
             config: repo_config,
             dirstate_parents: Cell::new(None),
-            dirstate_map: RefCell::new(None),
+            dirstate_map: LazyCell::new(Self::new_dirstate_map),
         };
 
         requirements::check(&repo)?;
@@ -302,24 +302,52 @@ 
     pub fn dirstate_map(
         &self,
     ) -> Result<Ref<OwningDirstateMap>, DirstateError> {
-        let mut borrowed = self.dirstate_map.borrow();
+        self.dirstate_map.get_or_init(self)
+    }
+
+    pub fn dirstate_map_mut(
+        &self,
+    ) -> Result<RefMut<OwningDirstateMap>, DirstateError> {
+        self.dirstate_map.get_mut_or_init(self)
+    }
+}
+
+/// Lazily-initialized component of `Repo` with interior mutability
+///
+/// This differs from `OnceCell` in that the value can still be "deinitialized"
+/// later by setting its inner `Option` to `None`.
+struct LazyCell<T, E> {
+    value: RefCell<Option<T>>,
+    // `Fn`s that don’t capture environment are zero-size, so this box does
+    // not allocate:
+    init: Box<dyn Fn(&Repo) -> Result<T, E>>,
+}
+
+impl<T, E> LazyCell<T, E> {
+    fn new(init: impl Fn(&Repo) -> Result<T, E> + 'static) -> Self {
+        Self {
+            value: RefCell::new(None),
+            init: Box::new(init),
+        }
+    }
+
+    fn get_or_init(&self, repo: &Repo) -> Result<Ref<T>, E> {
+        let mut borrowed = self.value.borrow();
         if borrowed.is_none() {
             drop(borrowed);
             // Only use `borrow_mut` if it is really needed to avoid panic in
             // case there is another outstanding borrow but mutation is not
             // needed.
-            *self.dirstate_map.borrow_mut() = Some(self.new_dirstate_map()?);
-            borrowed = self.dirstate_map.borrow()
+            *self.value.borrow_mut() = Some((self.init)(repo)?);
+            borrowed = self.value.borrow()
         }
         Ok(Ref::map(borrowed, |option| option.as_ref().unwrap()))
     }
 
-    pub fn dirstate_map_mut(
-        &self,
-    ) -> Result<RefMut<OwningDirstateMap>, DirstateError> {
-        let mut borrowed = self.dirstate_map.borrow_mut();
+    pub fn get_mut_or_init(&self, repo: &Repo) -> Result<RefMut<T>, E> {
+        let mut borrowed = self.value.borrow_mut();
         if borrowed.is_none() {
-            *borrowed = Some(self.new_dirstate_map()?);
+            *borrowed = Some((self.init)(repo)?);
         }
         Ok(RefMut::map(borrowed, |option| option.as_mut().unwrap()))
     }