rust-index: stop instantiating a C Index
authorGeorges Racinet <georges.racinet@octobus.net>
Sun, 29 Oct 2023 23:54:05 +0100
changeset 51253 96e05f1a99bd
parent 51252 fd1aa5e18f75
child 51254 f94c10334bcb
rust-index: stop instantiating a C Index The only missing piece was the `cache` to be returned from `revlog.parse_index_v1_mixed`, and it really seems that it is essentially repetition of the input, if `inline` is `True`. Not worth a Rust implementation (C implementation is probably there for historical reasons).
mercurial/revlog.py
mercurial/testing/revlog.py
rust/hg-cpython/src/cindex.rs
rust/hg-cpython/src/revlog.rs
tests/test-rust-revlog.py
tests/test-verify.t
--- a/mercurial/revlog.py	Mon Oct 30 21:28:30 2023 +0100
+++ b/mercurial/revlog.py	Sun Oct 29 23:54:05 2023 +0100
@@ -226,8 +226,8 @@
 
 
 def parse_index_v1_mixed(data, inline, default_header):
-    index, cache = parse_index_v1(data, inline)
-    return rustrevlog.MixedIndex(index, data, default_header), cache
+    cache = (0, data) if inline else None
+    return rustrevlog.MixedIndex(data, default_header), cache
 
 
 # corresponds to uncompressed length of indexformatng (2 gigs, 4-byte
--- a/mercurial/testing/revlog.py	Mon Oct 30 21:28:30 2023 +0100
+++ b/mercurial/testing/revlog.py	Sun Oct 29 23:54:05 2023 +0100
@@ -57,5 +57,4 @@
         # not inheriting RevlogBasedTestCase to avoid having a
         # `parseindex` method that would be shadowed by future subclasses
         # this duplication will soon be removed
-        cindex = cparsers.parse_index2(data, False)[0]
-        return MixedIndex(cindex, data, REVLOGV1)
+        return MixedIndex(data, REVLOGV1)
--- a/rust/hg-cpython/src/cindex.rs	Mon Oct 30 21:28:30 2023 +0100
+++ b/rust/hg-cpython/src/cindex.rs	Sun Oct 29 23:54:05 2023 +0100
@@ -9,7 +9,7 @@
 //!
 //! Ideally, we should use an Index entirely implemented in Rust,
 //! but this will take some time to get there.
-
+#![allow(dead_code)]
 use cpython::{
     exc::ImportError, exc::TypeError, ObjectProtocol, PyClone, PyErr,
     PyObject, PyResult, PyTuple, Python, PythonObject,
--- a/rust/hg-cpython/src/revlog.rs	Mon Oct 30 21:28:30 2023 +0100
+++ b/rust/hg-cpython/src/revlog.rs	Sun Oct 29 23:54:05 2023 +0100
@@ -6,7 +6,6 @@
 // GNU General Public License version 2 or any later version.
 
 use crate::{
-    cindex,
     conversion::{rev_pyiter_collect, rev_pyiter_collect_or_else},
     utils::{node_from_py_bytes, node_from_py_object},
     PyRevision,
@@ -87,7 +86,6 @@
 }
 
 py_class!(pub class MixedIndex |py| {
-    data cindex: RefCell<cindex::Index>;
     @shared data index: hg::index::Index;
     data nt: RefCell<Option<CoreNodeTree>>;
     data docket: RefCell<Option<PyObject>>;
@@ -98,11 +96,10 @@
 
     def __new__(
         _cls,
-        cindex: PyObject,
         data: PyObject,
         default_header: u32,
     ) -> PyResult<MixedIndex> {
-        Self::new(py, cindex, data, default_header)
+        Self::new(py, data, default_header)
     }
 
     /// Compatibility layer used for Python consumers needing access to the C index
@@ -111,11 +108,11 @@
     /// that may need to build a custom `nodetree`, based on a specified revset.
     /// With a Rust implementation of the nodemap, we will be able to get rid of
     /// this, by exposing our own standalone nodemap class,
-    /// ready to accept `MixedIndex`.
-    def get_cindex(&self) -> PyResult<PyObject> {
+    /// ready to accept `Index`.
+/*    def get_cindex(&self) -> PyResult<PyObject> {
         Ok(self.cindex(py).borrow().inner().clone_ref(py))
     }
-
+*/
     // Index API involving nodemap, as defined in mercurial/pure/parsers.py
 
     /// Return Revision if found, raises a bare `error.RevlogError`
@@ -602,18 +599,12 @@
 }
 
 impl MixedIndex {
-    fn new(
-        py: Python,
-        cindex: PyObject,
-        data: PyObject,
-        header: u32,
-    ) -> PyResult<MixedIndex> {
+    fn new(py: Python, data: PyObject, header: u32) -> PyResult<MixedIndex> {
         // Safety: we keep the buffer around inside the class as `index_mmap`
         let (buf, bytes) = unsafe { mmap_keeparound(py, data)? };
 
         Self::create_instance(
             py,
-            RefCell::new(cindex::Index::new(py, cindex)?),
             hg::index::Index::new(
                 bytes,
                 IndexHeader::parse(&header.to_be_bytes())
@@ -666,10 +657,6 @@
         Ok(self.nt(py))
     }
 
-    pub fn clone_cindex(&self, py: Python) -> cindex::Index {
-        self.cindex(py).borrow().clone_ref(py)
-    }
-
     /// Returns the full nodemap bytes to be written as-is to disk
     fn inner_nodemap_data_all(&self, py: Python) -> PyResult<PyBytes> {
         let nodemap = self.get_nodetree(py)?.borrow_mut().take().unwrap();
--- a/tests/test-rust-revlog.py	Mon Oct 30 21:28:30 2023 +0100
+++ b/tests/test-rust-revlog.py	Sun Oct 29 23:54:05 2023 +0100
@@ -27,24 +27,16 @@
 class RustRevlogIndexTest(revlogtesting.RevlogBasedTestBase):
     def test_heads(self):
         idx = self.parseindex()
-        rustidx = revlog.MixedIndex(idx, revlogtesting.data_non_inlined, header)
+        rustidx = revlog.MixedIndex(revlogtesting.data_non_inlined, header)
         self.assertEqual(rustidx.headrevs(), idx.headrevs())
 
-    def test_get_cindex(self):
-        # drop me once we no longer need the method for shortest node
-        idx = self.parseindex()
-        rustidx = revlog.MixedIndex(idx, revlogtesting.data_non_inlined, header)
-        cidx = rustidx.get_cindex()
-        self.assertTrue(idx is cidx)
-
     def test_len(self):
         idx = self.parseindex()
-        rustidx = revlog.MixedIndex(idx, revlogtesting.data_non_inlined, header)
+        rustidx = revlog.MixedIndex(revlogtesting.data_non_inlined, header)
         self.assertEqual(len(rustidx), len(idx))
 
     def test_ancestors(self):
-        idx = self.parseindex()
-        rustidx = revlog.MixedIndex(idx, revlogtesting.data_non_inlined, header)
+        rustidx = revlog.MixedIndex(revlogtesting.data_non_inlined, header)
         lazy = LazyAncestors(rustidx, [3], 0, True)
         # we have two more references to the index:
         # - in its inner iterator for __contains__ and __bool__
--- a/tests/test-verify.t	Mon Oct 30 21:28:30 2023 +0100
+++ b/tests/test-verify.t	Sun Oct 29 23:54:05 2023 +0100
@@ -311,7 +311,8 @@
   $ cat start b > .hg/store/data/a.i
 
   $ hg verify -q
-   a@1: broken revlog! (index a is corrupted)
+   a@1: broken revlog! (index a is corrupted) (no-rust !)
+   a@1: broken revlog! (abort: unexpected inline revlog length) (rust !)
   warning: orphan data file 'data/a.i'
   not checking dirstate because of previous errors
   1 warnings encountered!