changeset 52558:4c9e31984b3a

rust-pyo3: exposition of AncestorsIterator Compared to the early experiments, we have one less `RwLock` in the wrapping. Still that is somewhat redundant with `UnsafePyLeaked` being itself some kind of lock. In the original rust-cpython code, we were borrowing the `RefCell` with a method that can panic. Instead we are now converting the `PoisonError` that unlocking a `RwLock` can produce. Since all methods acquiring the `RwLock` are themselves protected by the GIL and do not release it before returning, nor do they leak RwLock guards, there is no risk of contention on these locks themselves.
author Georges Racinet <georges.racinet@cloudcrane.io>
date Sat, 07 Dec 2024 18:38:37 +0100
parents 736551565871
children 3ffcdbf0b432
files rust/hg-pyo3/src/ancestors.rs rust/hg-pyo3/src/convert_cpython.rs rust/hg-pyo3/src/exceptions.rs rust/hg-pyo3/src/lib.rs
diffstat 4 files changed, 94 insertions(+), 2 deletions(-) [+]
line wrap: on
line diff
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/rust/hg-pyo3/src/ancestors.rs	Sat Dec 07 18:38:37 2024 +0100
@@ -0,0 +1,87 @@
+// ancestors.rs
+//
+// Copyright 2024 Georges Racinet <georges.racinet@cloudcrane.io>
+//
+// This software may be used and distributed according to the terms of the
+// GNU General Public License version 2 or any later version.
+
+//! Bindings for the `hg::ancestors` module provided by the
+//! `hg-core` crate. From Python, this will be seen as `pyo3_rustext.ancestor`
+//! and can be used as replacement for the the pure `ancestor` Python module.
+use cpython::UnsafePyLeaked;
+use pyo3::prelude::*;
+
+use std::sync::RwLock;
+
+use vcsgraph::lazy_ancestors::AncestorsIterator as VCGAncestorsIterator;
+
+use crate::convert_cpython::{
+    proxy_index_extract, proxy_index_py_leak, py_leaked_borrow_mut,
+    py_leaked_or_map_err,
+};
+use crate::exceptions::{map_lock_error, GraphError};
+use crate::revision::{rev_pyiter_collect, PyRevision};
+use crate::util::new_submodule;
+use rusthg::revlog::PySharedIndex;
+
+#[pyclass]
+struct AncestorsIterator {
+    inner: RwLock<UnsafePyLeaked<VCGAncestorsIterator<PySharedIndex>>>,
+}
+
+#[pymethods]
+impl AncestorsIterator {
+    #[new]
+    fn new(
+        index_proxy: &Bound<'_, PyAny>,
+        initrevs: &Bound<'_, PyAny>,
+        stoprev: PyRevision,
+        inclusive: bool,
+    ) -> PyResult<Self> {
+        // Safety: we don't leak the "faked" reference out of
+        // `UnsafePyLeaked`
+        let initvec: Vec<_> = {
+            let borrowed_idx = unsafe { proxy_index_extract(index_proxy)? };
+            rev_pyiter_collect(initrevs, borrowed_idx)?
+        };
+        let (py, leaked_idx) = proxy_index_py_leak(index_proxy)?;
+        let res_ait = unsafe {
+            leaked_idx.map(py, |idx| {
+                VCGAncestorsIterator::new(
+                    idx,
+                    initvec.into_iter().map(|r| r.0),
+                    stoprev.0,
+                    inclusive,
+                )
+            })
+        };
+        let ait =
+            py_leaked_or_map_err(py, res_ait, GraphError::from_vcsgraph)?;
+        let inner = ait.into();
+        Ok(Self { inner })
+    }
+
+    fn __iter__(slf: PyRef<'_, Self>) -> PyRef<'_, Self> {
+        slf
+    }
+
+    fn __next__(slf: PyRefMut<'_, Self>) -> PyResult<Option<PyRevision>> {
+        let mut leaked = slf.inner.write().map_err(map_lock_error)?;
+        // Safety: we don't leak the inner 'static ref out of UnsafePyLeaked
+        let mut inner = unsafe { py_leaked_borrow_mut(&slf, &mut leaked)? };
+        match inner.next() {
+            Some(Err(e)) => Err(GraphError::from_vcsgraph(e)),
+            None => Ok(None),
+            Some(Ok(r)) => Ok(Some(PyRevision(r))),
+        }
+    }
+}
+
+pub fn init_module<'py>(
+    py: Python<'py>,
+    package: &str,
+) -> PyResult<Bound<'py, PyModule>> {
+    let m = new_submodule(py, package, "ancestor")?;
+    m.add_class::<AncestorsIterator>()?;
+    Ok(m)
+}
--- a/rust/hg-pyo3/src/convert_cpython.rs	Sat Dec 07 18:24:24 2024 +0100
+++ b/rust/hg-pyo3/src/convert_cpython.rs	Sat Dec 07 18:38:37 2024 +0100
@@ -263,7 +263,6 @@
 /// This would spare users of the `cpython` crate the additional `unsafe` deref
 /// to inspect the error and return it outside `UnsafePyLeaked`, and the
 /// subsequent unwrapping that this function performs.
-#[allow(dead_code)]
 pub(crate) fn py_leaked_or_map_err<T, E: std::fmt::Debug + Copy>(
     py: cpython::Python,
     leaked: cpython::UnsafePyLeaked<Result<T, E>>,
--- a/rust/hg-pyo3/src/exceptions.rs	Sat Dec 07 18:24:24 2024 +0100
+++ b/rust/hg-pyo3/src/exceptions.rs	Sat Dec 07 18:38:37 2024 +0100
@@ -1,4 +1,4 @@
-use pyo3::exceptions::PyValueError;
+use pyo3::exceptions::{PyRuntimeError, PyValueError};
 use pyo3::import_exception;
 use pyo3::{create_exception, PyErr};
 
@@ -32,3 +32,7 @@
         }
     }
 }
+
+pub fn map_lock_error<T>(e: std::sync::PoisonError<T>) -> PyErr {
+    PyRuntimeError::new_err(format!("In Rust PyO3 bindings: {e}"))
+}
--- a/rust/hg-pyo3/src/lib.rs	Sat Dec 07 18:24:24 2024 +0100
+++ b/rust/hg-pyo3/src/lib.rs	Sat Dec 07 18:38:37 2024 +0100
@@ -1,5 +1,6 @@
 use pyo3::prelude::*;
 
+mod ancestors;
 mod convert_cpython;
 mod dagops;
 mod exceptions;
@@ -17,6 +18,7 @@
     let name: String = m.getattr("__name__")?.extract()?;
     let dotted_name = format!("mercurial.{}", name);
 
+    m.add_submodule(&ancestors::init_module(py, &dotted_name)?)?;
     m.add_submodule(&dagops::init_module(py, &dotted_name)?)?;
     m.add("GraphError", py.get_type::<exceptions::GraphError>())?;
     Ok(())