Skip to content

Commit

Permalink
Hide GILPool behind a feature to test ability to fully avoid it [ci s…
Browse files Browse the repository at this point in the history
…kip]
  • Loading branch information
adamreichold committed Dec 22, 2023
1 parent 7f626b2 commit 7f9b1cb
Show file tree
Hide file tree
Showing 7 changed files with 85 additions and 15 deletions.
3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ experimental-inspect = []
# Enables macros: #[pyclass], #[pymodule], #[pyfunction] etc.
macros = ["pyo3-macros", "indoc", "unindent"]

# Enables GIL-bound references backed by a thread-local pool
pool = []

# Enables multiple #[pymethods] per #[pyclass]
multiple-pymethods = ["inventory", "pyo3-macros/multiple-pymethods"]

Expand Down
8 changes: 4 additions & 4 deletions src/conversion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@ use crate::inspect::types::TypeInfo;
use crate::pyclass::boolean_struct::False;
use crate::type_object::PyTypeInfo;
use crate::types::PyTuple;
use crate::{
ffi, gil, Py, PyAny, PyCell, PyClass, PyNativeType, PyObject, PyRef, PyRefMut, Python,
};
use crate::{ffi, Py, PyAny, PyCell, PyClass, PyNativeType, PyObject, PyRef, PyRefMut, Python};
use std::cell::Cell;
#[cfg(feature = "pool")]
use std::ptr::NonNull;

/// Returns a borrowed pointer to a Python object.
Expand Down Expand Up @@ -543,12 +542,13 @@ pub unsafe trait FromPyPointer<'p>: Sized {
}
}

#[cfg(feature = "pool")]
unsafe impl<'p, T> FromPyPointer<'p> for T
where
T: 'p + crate::PyNativeType,
{
unsafe fn from_owned_ptr_or_opt(py: Python<'p>, ptr: *mut ffi::PyObject) -> Option<&'p Self> {
gil::register_owned(py, NonNull::new(ptr)?);
crate::gil::register_owned(py, NonNull::new(ptr)?);
Some(&*(ptr as *mut Self))
}
unsafe fn from_borrowed_ptr_or_opt(
Expand Down
61 changes: 53 additions & 8 deletions src/gil.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
//! Interaction with Python's global interpreter lock
#[cfg(feature = "pool")]
use crate::impl_::not_send::{NotSend, NOT_SEND};
use crate::{ffi, Python};
use parking_lot::{const_mutex, Mutex, Once};
use std::cell::Cell;
#[cfg(debug_assertions)]
#[cfg(all(feature = "pool", debug_assertions))]
use std::cell::RefCell;
#[cfg(not(debug_assertions))]
#[cfg(all(feature = "pool", not(debug_assertions)))]
use std::cell::UnsafeCell;
use std::{mem, ptr::NonNull};

Expand Down Expand Up @@ -37,9 +38,9 @@ thread_local_const_init! {
static GIL_COUNT: Cell<isize> = const { Cell::new(0) };

/// Temporarily hold objects that will be released when the GILPool drops.
#[cfg(debug_assertions)]
#[cfg(all(feature = "pool", debug_assertions))]
static OWNED_OBJECTS: RefCell<PyObjVec> = const { RefCell::new(Vec::new()) };
#[cfg(not(debug_assertions))]
#[cfg(all(feature = "pool", not(debug_assertions)))]
static OWNED_OBJECTS: UnsafeCell<PyObjVec> = const { UnsafeCell::new(Vec::new()) };
}

Expand Down Expand Up @@ -139,17 +140,27 @@ where
ffi::Py_InitializeEx(0);

// Safety: the GIL is already held because of the Py_IntializeEx call.
#[cfg(feature = "pool")]
let pool = GILPool::new();
#[cfg(feature = "pool")]
let py = pool.python();
#[cfg(not(feature = "pool"))]
let gil = WithGIL::during_call();
#[cfg(not(feature = "pool"))]
let py = gil.python();

// Import the threading module - this ensures that it will associate this thread as the "main"
// thread, which is important to avoid an `AssertionError` at finalization.
pool.python().import("threading").unwrap();
py.import("threading").unwrap();

// Execute the closure.
let result = f(pool.python());
let result = f(py);

// Drop the pool before finalizing.
#[cfg(feature = "pool")]
drop(pool);
#[cfg(not(feature = "pool"))]
drop(gil);

// Finalize the Python interpreter.
ffi::Py_Finalize();
Expand All @@ -160,6 +171,7 @@ where
/// RAII type that represents the Global Interpreter Lock acquisition.
pub(crate) struct GILGuard {
gstate: ffi::PyGILState_STATE,
#[cfg(feature = "pool")]
pool: mem::ManuallyDrop<GILPool>,
}

Expand Down Expand Up @@ -222,9 +234,14 @@ impl GILGuard {
}

let gstate = unsafe { ffi::PyGILState_Ensure() }; // acquire GIL
#[cfg(feature = "pool")]
let pool = unsafe { mem::ManuallyDrop::new(GILPool::new()) };

Some(GILGuard { gstate, pool })
Some(GILGuard {
gstate,
#[cfg(feature = "pool")]
pool,
})
}
}

Expand All @@ -233,6 +250,7 @@ impl Drop for GILGuard {
fn drop(&mut self) {
unsafe {
// Drop the objects in the pool before attempting to release the thread state
#[cfg(feature = "pool")]
mem::ManuallyDrop::drop(&mut self.pool);

ffi::PyGILState_Release(self.gstate);
Expand Down Expand Up @@ -317,6 +335,30 @@ impl Drop for SuspendGIL {
}
}

/// Used to indicate safe access to the GIL
pub(crate) struct WithGIL;

impl WithGIL {
pub unsafe fn during_call() -> Self {
increment_gil_count();

// Update counts of PyObjects / Py that have been cloned or dropped since last acquisition
POOL.update_counts(Python::assume_gil_acquired());

Self
}

pub fn python(&self) -> Python<'_> {
unsafe { Python::assume_gil_acquired() }
}
}

impl Drop for WithGIL {
fn drop(&mut self) {
decrement_gil_count();
}
}

/// Used to lock safe access to the GIL
pub(crate) struct LockGIL {
count: isize,
Expand Down Expand Up @@ -355,16 +397,17 @@ impl Drop for LockGIL {
///
/// See the [Memory Management] chapter of the guide for more information about how PyO3 uses
/// [`GILPool`] to manage memory.
///
/// [Memory Management]: https://pyo3.rs/main/memory.html#gil-bound-memory
#[cfg(feature = "pool")]
pub struct GILPool {
/// Initial length of owned objects and anys.
/// `Option` is used since TSL can be broken when `new` is called from `atexit`.
start: Option<usize>,
_not_send: NotSend,
}

#[cfg(feature = "pool")]
impl GILPool {
/// Creates a new [`GILPool`]. This function should only ever be called with the GIL held.
///
Expand Down Expand Up @@ -401,6 +444,7 @@ impl GILPool {
}
}

#[cfg(feature = "pool")]
impl Drop for GILPool {
fn drop(&mut self) {
if let Some(start) = self.start {
Expand Down Expand Up @@ -463,6 +507,7 @@ pub unsafe fn register_decref(obj: NonNull<ffi::PyObject>) {
///
/// # Safety
/// The object must be an owned Python reference.
#[cfg(feature = "pool")]
pub unsafe fn register_owned(_py: Python<'_>, obj: NonNull<ffi::PyObject>) {
debug_assert!(gil_is_acquired());
// Ignores the error in case this function called from `atexit`.
Expand Down
1 change: 1 addition & 0 deletions src/impl_/not_send.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@ use crate::Python;
/// Workaround for lack of !Send on stable (<https://github.com/rust-lang/rust/issues/68318>).
pub(crate) struct NotSend(PhantomData<*mut Python<'static>>);

#[cfg(feature = "pool")]
pub(crate) const NOT_SEND: NotSend = NotSend(PhantomData);
20 changes: 18 additions & 2 deletions src/impl_/trampoline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,13 @@ use std::{
panic::{self, UnwindSafe},
};

#[cfg(feature = "pool")]
use crate::gil::GILPool;
#[cfg(not(feature = "pool"))]
use crate::gil::WithGIL;
use crate::{
callback::PyCallbackOutput, ffi, impl_::panic::PanicTrap, methods::IPowModulo,
panic::PanicException, types::PyModule, GILPool, Py, PyResult, Python,
panic::PanicException, types::PyModule, Py, PyResult, Python,
};

#[inline]
Expand Down Expand Up @@ -174,8 +178,14 @@ where
R: PyCallbackOutput,
{
let trap = PanicTrap::new("uncaught panic at ffi boundary");
#[cfg(feature = "pool")]
let pool = unsafe { GILPool::new() };
#[cfg(feature = "pool")]
let py = pool.python();
#[cfg(not(feature = "pool"))]
let gil = unsafe { WithGIL::during_call() };
#[cfg(not(feature = "pool"))]
let py = gil.python();
let out = panic_result_into_callback_output(
py,
panic::catch_unwind(move || -> PyResult<_> { body(py) }),
Expand Down Expand Up @@ -219,8 +229,14 @@ where
F: for<'py> FnOnce(Python<'py>) -> PyResult<()> + UnwindSafe,
{
let trap = PanicTrap::new("uncaught panic at ffi boundary");
let pool = GILPool::new();
#[cfg(feature = "pool")]
let pool = unsafe { GILPool::new() };
#[cfg(feature = "pool")]
let py = pool.python();
#[cfg(not(feature = "pool"))]
let gil = unsafe { WithGIL::during_call() };
#[cfg(not(feature = "pool"))]
let py = gil.python();
if let Err(py_err) = panic::catch_unwind(move || body(py))
.unwrap_or_else(|payload| Err(PanicException::from_panic_payload(payload)))
{
Expand Down
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,7 @@ pub use crate::conversion::{AsPyPointer, FromPyObject, FromPyPointer, IntoPy, To
#[allow(deprecated)]
pub use crate::conversion::{PyTryFrom, PyTryInto};
pub use crate::err::{PyDowncastError, PyErr, PyErrArguments, PyResult};
#[cfg(feature = "pool")]
pub use crate::gil::GILPool;
#[cfg(not(PyPy))]
pub use crate::gil::{prepare_freethreaded_python, with_embedded_python_interpreter};
Expand Down
6 changes: 5 additions & 1 deletion src/marker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,9 @@
//! [`Rc`]: std::rc::Rc
//! [`Py`]: crate::Py
use crate::err::{self, PyDowncastError, PyErr, PyResult};
use crate::gil::{GILGuard, GILPool, SuspendGIL};
#[cfg(feature = "pool")]
use crate::gil::GILPool;
use crate::gil::{GILGuard, SuspendGIL};
use crate::impl_::not_send::NotSend;
use crate::type_object::HasPyGilRef;
use crate::types::{
Expand Down Expand Up @@ -967,6 +969,7 @@ impl<'py> Python<'py> {
///
/// [`.python()`]: crate::GILPool::python
#[inline]
#[cfg(feature = "pool")]
pub unsafe fn new_pool(self) -> GILPool {
GILPool::new()
}
Expand Down Expand Up @@ -1025,6 +1028,7 @@ impl Python<'_> {
/// });
/// ```
#[inline]
#[cfg(feature = "pool")]
pub fn with_pool<F, R>(&self, f: F) -> R
where
F: for<'py> FnOnce(Python<'py>) -> R + Ungil,
Expand Down

0 comments on commit 7f9b1cb

Please sign in to comment.