rust-index-cpython: cache the heads' PyList representation
authorRaphaël Gomès <rgomes@octobus.net>
Wed, 29 Nov 2023 23:22:51 -0500
changeset 51261 9088c6d65ef6
parent 51260 c4f1a790bda8
child 51262 f20c4b307a5a
rust-index-cpython: cache the heads' PyList representation This is the same optimization that the C index does, we just have more separation of the Python and native sides.
rust/hg-core/src/revlog/index.rs
rust/hg-cpython/src/revlog.rs
--- a/rust/hg-core/src/revlog/index.rs	Wed Nov 29 15:58:24 2023 -0500
+++ b/rust/hg-core/src/revlog/index.rs	Wed Nov 29 23:22:51 2023 -0500
@@ -544,14 +544,23 @@
 
     /// Return the head revisions of this index
     pub fn head_revs(&self) -> Result<Vec<Revision>, GraphError> {
-        self.head_revs_filtered(&HashSet::new())
+        self.head_revs_filtered(&HashSet::new(), false)
+            .map(|h| h.unwrap())
+    }
+
+    /// Python-specific shortcut to save on PyList creation
+    pub fn head_revs_shortcut(
+        &self,
+    ) -> Result<Option<Vec<Revision>>, GraphError> {
+        self.head_revs_filtered(&HashSet::new(), true)
     }
 
     /// Return the head revisions of this index
     pub fn head_revs_filtered(
         &self,
         filtered_revs: &HashSet<Revision>,
-    ) -> Result<Vec<Revision>, GraphError> {
+        py_shortcut: bool,
+    ) -> Result<Option<Vec<Revision>>, GraphError> {
         {
             let guard = self
                 .head_revs
@@ -562,7 +571,13 @@
             if !self_head_revs.is_empty()
                 && filtered_revs == self_filtered_revs
             {
-                return Ok(self_head_revs.to_owned());
+                if py_shortcut {
+                    // Don't copy the revs since we've already cached them
+                    // on the Python side.
+                    return Ok(None);
+                } else {
+                    return Ok(Some(self_head_revs.to_owned()));
+                }
             }
         }
 
@@ -592,7 +607,7 @@
             .write()
             .expect("RwLock on Index.head_revs should not be poisoned") =
             (as_vec.to_owned(), filtered_revs.to_owned());
-        Ok(as_vec)
+        Ok(Some(as_vec))
     }
 
     /// Obtain the delta chain for a revision.
--- a/rust/hg-cpython/src/revlog.rs	Wed Nov 29 15:58:24 2023 -0500
+++ b/rust/hg-cpython/src/revlog.rs	Wed Nov 29 23:22:51 2023 -0500
@@ -96,6 +96,7 @@
     data nodemap_mmap: RefCell<Option<PyBuffer>>;
     // Holds a reference to the mmap'ed persistent index data
     data index_mmap: RefCell<Option<PyBuffer>>;
+    data head_revs_py_list: RefCell<Option<PyList>>;
 
     def __new__(
         _cls,
@@ -257,6 +258,7 @@
         self.nt(py).borrow_mut().take();
         self.docket(py).borrow_mut().take();
         self.nodemap_mmap(py).borrow_mut().take();
+        self.head_revs_py_list(py).borrow_mut().take();
         self.index(py).borrow().clear_caches();
         Ok(py.None())
     }
@@ -621,6 +623,7 @@
             RefCell::new(None),
             RefCell::new(None),
             RefCell::new(Some(buf)),
+            RefCell::new(None),
         )
     }
 
@@ -773,13 +776,19 @@
 
     fn inner_headrevs(&self, py: Python) -> PyResult<PyObject> {
         let index = &*self.index(py).borrow();
-        let as_vec: Vec<PyObject> = index
-            .head_revs()
-            .map_err(|e| graph_error(py, e))?
-            .iter()
-            .map(|r| PyRevision::from(*r).into_py_object(py).into_object())
-            .collect();
-        Ok(PyList::new(py, &as_vec).into_object())
+        if let Some(new_heads) =
+            index.head_revs_shortcut().map_err(|e| graph_error(py, e))?
+        {
+            self.cache_new_heads_py_list(new_heads, py);
+        }
+
+        Ok(self
+            .head_revs_py_list(py)
+            .borrow()
+            .as_ref()
+            .expect("head revs should be cached")
+            .clone_ref(py)
+            .into_object())
     }
 
     fn inner_headrevsfiltered(
@@ -790,13 +799,35 @@
         let index = &mut *self.index(py).borrow_mut();
         let filtered_revs = rev_pyiter_collect(py, filtered_revs, index)?;
 
-        let as_vec: Vec<PyObject> = index
-            .head_revs_filtered(&filtered_revs)
+        if let Some(new_heads) = index
+            .head_revs_filtered(&filtered_revs, true)
             .map_err(|e| graph_error(py, e))?
+        {
+            self.cache_new_heads_py_list(new_heads, py);
+        }
+
+        Ok(self
+            .head_revs_py_list(py)
+            .borrow()
+            .as_ref()
+            .expect("head revs should be cached")
+            .clone_ref(py)
+            .into_object())
+    }
+
+    fn cache_new_heads_py_list(
+        &self,
+        new_heads: Vec<Revision>,
+        py: Python<'_>,
+    ) -> PyList {
+        let as_vec: Vec<PyObject> = new_heads
             .iter()
             .map(|r| PyRevision::from(*r).into_py_object(py).into_object())
             .collect();
-        Ok(PyList::new(py, &as_vec).into_object())
+        let new_heads_py_list = PyList::new(py, &as_vec);
+        *self.head_revs_py_list(py).borrow_mut() =
+            Some(new_heads_py_list.clone_ref(py));
+        new_heads_py_list
     }
 
     fn inner_ancestors(