D11396: rust: Make OwningDirstateMap generic and move it into hg-core

SimonSapin phabricator at mercurial-scm.org
Fri Sep 10 15:12:42 UTC 2021


SimonSapin created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  This will enable using it in rhg too.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  rust/Cargo.lock
  rust/hg-core/Cargo.toml
  rust/hg-core/src/dirstate_tree.rs
  rust/hg-core/src/dirstate_tree/owning.rs
  rust/hg-core/src/dirstate_tree/owning_dispatch.rs
  rust/hg-cpython/Cargo.toml
  rust/hg-cpython/src/dirstate.rs
  rust/hg-cpython/src/dirstate/dirstate_map.rs
  rust/hg-cpython/src/dirstate/dispatch.rs
  rust/hg-cpython/src/dirstate/owning.rs
  rust/hg-cpython/src/pybytes_deref.rs

CHANGE DETAILS

diff --git a/rust/hg-cpython/src/pybytes_deref.rs b/rust/hg-cpython/src/pybytes_deref.rs
--- a/rust/hg-cpython/src/pybytes_deref.rs
+++ b/rust/hg-cpython/src/pybytes_deref.rs
@@ -1,4 +1,5 @@
 use cpython::{PyBytes, Python};
+use stable_deref_trait::StableDeref;
 
 /// Safe abstraction over a `PyBytes` together with the `&[u8]` slice
 /// that borrows it. Implements `Deref<Target = [u8]>`.
@@ -40,6 +41,8 @@
     }
 }
 
+unsafe impl StableDeref for PyBytesDeref {}
+
 fn require_send<T: Send>() {}
 
 #[allow(unused)]
diff --git a/rust/hg-cpython/src/dirstate/dirstate_map.rs b/rust/hg-cpython/src/dirstate/dirstate_map.rs
--- a/rust/hg-cpython/src/dirstate/dirstate_map.rs
+++ b/rust/hg-cpython/src/dirstate/dirstate_map.rs
@@ -24,15 +24,17 @@
     dirstate::non_normal_entries::{
         NonNormalEntries, NonNormalEntriesIterator,
     },
-    dirstate::owning::OwningDirstateMap,
     parsers::dirstate_parents_to_pytuple,
+    pybytes_deref::PyBytesDeref,
 };
 use hg::{
     dirstate::parsers::Timestamp,
     dirstate::MTIME_UNSET,
     dirstate::SIZE_NON_NORMAL,
+    dirstate_tree::dirstate_map::DirstateMap as TreeDirstateMap,
     dirstate_tree::dispatch::DirstateMapMethods,
     dirstate_tree::on_disk::DirstateV2ParseError,
+    dirstate_tree::owning::OwningDirstateMap,
     revlog::Node,
     utils::files::normalize_case,
     utils::hg_path::{HgPath, HgPathBuf},
@@ -62,8 +64,13 @@
         on_disk: PyBytes,
     ) -> PyResult<PyObject> {
         let (inner, parents) = if use_dirstate_tree {
-            let (map, parents) = OwningDirstateMap::new_v1(py, on_disk)
+            let on_disk = PyBytesDeref::new(py, on_disk);
+            let mut map = OwningDirstateMap::new_empty(on_disk);
+            let (on_disk, map_placeholder) = map.get_mut_pair();
+
+            let (actual_map, parents) = TreeDirstateMap::new_v1(on_disk)
                 .map_err(|e| dirstate_error(py, e))?;
+            *map_placeholder = actual_map;
             (Box::new(map) as _, parents)
         } else {
             let bytes = on_disk.data(py);
@@ -86,10 +93,13 @@
         let dirstate_error = |e: DirstateError| {
             PyErr::new::<exc::OSError, _>(py, format!("Dirstate error: {:?}", e))
         };
-        let inner = OwningDirstateMap::new_v2(
-            py, on_disk, data_size, tree_metadata,
+        let on_disk = PyBytesDeref::new(py, on_disk);
+        let mut map = OwningDirstateMap::new_empty(on_disk);
+        let (on_disk, map_placeholder) = map.get_mut_pair();
+        *map_placeholder = TreeDirstateMap::new_v2(
+            on_disk, data_size, tree_metadata.data(py),
         ).map_err(dirstate_error)?;
-        let map = Self::create_instance(py, Box::new(inner))?;
+        let map = Self::create_instance(py, Box::new(map))?;
         Ok(map.into_object())
     }
 
diff --git a/rust/hg-cpython/src/dirstate.rs b/rust/hg-cpython/src/dirstate.rs
--- a/rust/hg-cpython/src/dirstate.rs
+++ b/rust/hg-cpython/src/dirstate.rs
@@ -12,9 +12,7 @@
 mod copymap;
 mod dirs_multiset;
 mod dirstate_map;
-mod dispatch;
 mod non_normal_entries;
-mod owning;
 mod status;
 use crate::{
     dirstate::{
diff --git a/rust/hg-cpython/Cargo.toml b/rust/hg-cpython/Cargo.toml
--- a/rust/hg-cpython/Cargo.toml
+++ b/rust/hg-cpython/Cargo.toml
@@ -26,6 +26,7 @@
 libc = '*'
 log = "0.4.8"
 env_logger = "0.7.1"
+stable_deref_trait = "1.2.0"
 
 [dependencies.cpython]
 version = "0.6.0"
diff --git a/rust/hg-cpython/src/dirstate/dispatch.rs b/rust/hg-core/src/dirstate_tree/owning_dispatch.rs
rename from rust/hg-cpython/src/dirstate/dispatch.rs
rename to rust/hg-core/src/dirstate_tree/owning_dispatch.rs
--- a/rust/hg-cpython/src/dirstate/dispatch.rs
+++ b/rust/hg-core/src/dirstate_tree/owning_dispatch.rs
@@ -1,18 +1,18 @@
-use crate::dirstate::owning::OwningDirstateMap;
-use hg::dirstate::parsers::Timestamp;
-use hg::dirstate_tree::dispatch::DirstateMapMethods;
-use hg::dirstate_tree::on_disk::DirstateV2ParseError;
-use hg::matchers::Matcher;
-use hg::utils::hg_path::{HgPath, HgPathBuf};
-use hg::CopyMapIter;
-use hg::DirstateEntry;
-use hg::DirstateError;
-use hg::DirstateParents;
-use hg::DirstateStatus;
-use hg::PatternFileWarning;
-use hg::StateMapIter;
-use hg::StatusError;
-use hg::StatusOptions;
+use crate::dirstate::parsers::Timestamp;
+use crate::dirstate_tree::dispatch::DirstateMapMethods;
+use crate::dirstate_tree::on_disk::DirstateV2ParseError;
+use crate::dirstate_tree::owning::OwningDirstateMap;
+use crate::matchers::Matcher;
+use crate::utils::hg_path::{HgPath, HgPathBuf};
+use crate::CopyMapIter;
+use crate::DirstateEntry;
+use crate::DirstateError;
+use crate::DirstateParents;
+use crate::DirstateStatus;
+use crate::PatternFileWarning;
+use crate::StateMapIter;
+use crate::StatusError;
+use crate::StatusOptions;
 use std::path::PathBuf;
 
 impl DirstateMapMethods for OwningDirstateMap {
diff --git a/rust/hg-cpython/src/dirstate/owning.rs b/rust/hg-core/src/dirstate_tree/owning.rs
rename from rust/hg-cpython/src/dirstate/owning.rs
rename to rust/hg-core/src/dirstate_tree/owning.rs
--- a/rust/hg-cpython/src/dirstate/owning.rs
+++ b/rust/hg-core/src/dirstate_tree/owning.rs
@@ -1,11 +1,9 @@
-use cpython::PyBytes;
-use cpython::Python;
-use hg::dirstate_tree::dirstate_map::DirstateMap;
-use hg::DirstateError;
-use hg::DirstateParents;
+use super::dirstate_map::DirstateMap;
+use stable_deref_trait::StableDeref;
+use std::ops::Deref;
 
 /// Keep a `DirstateMap<'on_disk>` next to the `on_disk` buffer that it
-/// borrows. This is similar to the owning-ref crate.
+/// borrows.
 ///
 /// This is similar to [`OwningRef`] which is more limited because it
 /// represents exactly one `&T` reference next to the value it borrows, as
@@ -13,11 +11,11 @@
 /// arbitrarily-nested data structures.
 ///
 /// [`OwningRef`]: https://docs.rs/owning_ref/0.4.1/owning_ref/struct.OwningRef.html
-pub(super) struct OwningDirstateMap {
+pub struct OwningDirstateMap {
     /// Owned handle to a bytes buffer with a stable address.
     ///
     /// See <https://docs.rs/owning_ref/0.4.1/owning_ref/trait.StableAddress.html>.
-    on_disk: PyBytes,
+    on_disk: Box<dyn Deref<Target = [u8]> + Send>,
 
     /// Pointer for `Box<DirstateMap<'on_disk>>`, typed-erased because the
     /// language cannot represent a lifetime referencing a sibling field.
@@ -28,12 +26,13 @@
 }
 
 impl OwningDirstateMap {
-    pub fn new_v1(
-        py: Python,
-        on_disk: PyBytes,
-    ) -> Result<(Self, Option<DirstateParents>), DirstateError> {
-        let bytes: &'_ [u8] = on_disk.data(py);
-        let (map, parents) = DirstateMap::new_v1(bytes)?;
+    pub fn new_empty<OnDisk>(on_disk: OnDisk) -> Self
+    where
+        OnDisk: Deref<Target = [u8]> + StableDeref + Send + 'static,
+    {
+        let on_disk = Box::new(on_disk);
+        let bytes: &'_ [u8] = &on_disk;
+        let map = DirstateMap::empty(bytes);
 
         // Like in `bytes` above, this `'_` lifetime parameter borrows from
         // the bytes buffer owned by `on_disk`.
@@ -42,30 +41,12 @@
         // Erase the pointed type entirely in order to erase the lifetime.
         let ptr: *mut () = ptr.cast();
 
-        Ok((Self { on_disk, ptr }, parents))
+        Self { on_disk, ptr }
     }
 
-    pub fn new_v2(
-        py: Python,
-        on_disk: PyBytes,
-        data_size: usize,
-        tree_metadata: PyBytes,
-    ) -> Result<Self, DirstateError> {
-        let bytes: &'_ [u8] = on_disk.data(py);
-        let map =
-            DirstateMap::new_v2(bytes, data_size, tree_metadata.data(py))?;
-
-        // Like in `bytes` above, this `'_` lifetime parameter borrows from
-        // the bytes buffer owned by `on_disk`.
-        let ptr: *mut DirstateMap<'_> = Box::into_raw(Box::new(map));
-
-        // Erase the pointed type entirely in order to erase the lifetime.
-        let ptr: *mut () = ptr.cast();
-
-        Ok(Self { on_disk, ptr })
-    }
-
-    pub fn get_mut<'a>(&'a mut self) -> &'a mut DirstateMap<'a> {
+    pub fn get_mut_pair<'a>(
+        &'a mut self,
+    ) -> (&'a [u8], &'a mut DirstateMap<'a>) {
         // SAFETY: We cast the type-erased pointer back to the same type it had
         // in `new`, except with a different lifetime parameter. This time we
         // connect the lifetime to that of `self`. This cast is valid because
@@ -76,7 +57,11 @@
         // SAFETY: we dereference that pointer, connecting the lifetime of the
         // new   `&mut` to that of `self`. This is valid because the
         // raw pointer is   to a boxed value, and `self` owns that box.
-        unsafe { &mut *ptr }
+        (&self.on_disk, unsafe { &mut *ptr })
+    }
+
+    pub fn get_mut<'a>(&'a mut self) -> &'a mut DirstateMap<'a> {
+        self.get_mut_pair().1
     }
 
     pub fn get<'a>(&'a self) -> &'a DirstateMap<'a> {
@@ -84,6 +69,10 @@
         let ptr: *mut DirstateMap<'a> = self.ptr.cast();
         unsafe { &*ptr }
     }
+
+    pub fn on_disk<'a>(&'a self) -> &'a [u8] {
+        &self.on_disk
+    }
 }
 
 impl Drop for OwningDirstateMap {
@@ -105,13 +94,12 @@
 fn _static_assert_is_send<T: Send>() {}
 
 fn _static_assert_fields_are_send() {
-    _static_assert_is_send::<PyBytes>();
     _static_assert_is_send::<Box<DirstateMap<'_>>>();
 }
 
 // SAFETY: we don’t get this impl implicitly because `*mut (): !Send` because
 // thread-safety of raw pointers is unknown in the general case. However this
 // particular raw pointer represents a `Box<DirstateMap<'on_disk>>` that we
-// own. Since that `Box` and `PyBytes` are both `Send` as shown in above, it
-// is sound to mark this struct as `Send` too.
+// own. Since that `Box` is `Send` as shown in above, it is sound to mark
+// this struct as `Send` too.
 unsafe impl Send for OwningDirstateMap {}
diff --git a/rust/hg-core/src/dirstate_tree.rs b/rust/hg-core/src/dirstate_tree.rs
--- a/rust/hg-core/src/dirstate_tree.rs
+++ b/rust/hg-core/src/dirstate_tree.rs
@@ -1,5 +1,7 @@
 pub mod dirstate_map;
 pub mod dispatch;
 pub mod on_disk;
+pub mod owning;
+mod owning_dispatch;
 pub mod path_with_basename;
 pub mod status;
diff --git a/rust/hg-core/Cargo.toml b/rust/hg-core/Cargo.toml
--- a/rust/hg-core/Cargo.toml
+++ b/rust/hg-core/Cargo.toml
@@ -24,6 +24,7 @@
 sha-1 = "0.9.6"
 twox-hash = "1.5.0"
 same-file = "1.0.6"
+stable_deref_trait = "1.2.0"
 tempfile = "3.1.0"
 crossbeam-channel = "0.4"
 micro-timer = "0.3.0"
diff --git a/rust/Cargo.lock b/rust/Cargo.lock
--- a/rust/Cargo.lock
+++ b/rust/Cargo.lock
@@ -1,5 +1,7 @@
 # This file is automatically @generated by Cargo.
 # It is not intended for manual editing.
+version = 3
+
 [[package]]
 name = "adler"
 version = "0.2.3"
@@ -396,6 +398,7 @@
  "regex",
  "same-file",
  "sha-1",
+ "stable_deref_trait",
  "tempfile",
  "twox-hash",
  "zstd",
@@ -411,6 +414,7 @@
  "hg-core",
  "libc",
  "log",
+ "stable_deref_trait",
 ]
 
 [[package]]
@@ -865,6 +869,12 @@
 ]
 
 [[package]]
+name = "stable_deref_trait"
+version = "1.2.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a8f112729512f8e442d81f95a8a7ddf2b7c6b8a1a6f509a95864142b30cab2d3"
+
+[[package]]
 name = "static_assertions"
 version = "1.1.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"



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


More information about the Mercurial-devel mailing list