Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Py::get for GIL-independent access to frozen classes. #3158

Merged
merged 1 commit into from
May 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion guide/pyclass_parameters.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
| `dict` | Gives instances of this class an empty `__dict__` to store custom attributes. |
| <span style="white-space: pre">`extends = BaseType`</span> | Use a custom baseclass. Defaults to [`PyAny`][params-1] |
| <span style="white-space: pre">`freelist = N`</span> | Implements a [free list][params-2] of size N. This can improve performance for types that are often created and deleted in quick succession. Profile your code to see whether `freelist` is right for you. |
| <span style="white-space: pre">`frozen`</span> | Declares that your pyclass is immutable. It removes the borrowchecker overhead when retrieving a shared reference to the Rust struct, but disables the ability to get a mutable reference. |
| <span style="white-space: pre">`frozen`</span> | Declares that your pyclass is immutable. It removes the borrow checker overhead when retrieving a shared reference to the Rust struct, but disables the ability to get a mutable reference. |
| `get_all` | Generates getters for all fields of the pyclass. |
| `mapping` | Inform PyO3 that this class is a [`Mapping`][params-mapping], and so leave its implementation of sequence C-API slots empty. |
| <span style="white-space: pre">`module = "module_name"`</span> | Python code will see the class as being defined in this module. Defaults to `builtins`. |
Expand Down
26 changes: 26 additions & 0 deletions guide/src/class.md
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,32 @@ Python::with_gil(|py| {
});
```

### frozen classes: Opting out of interior mutability

As detailed above, runtime borrow checking is currently enabled by default. But a class can opt of out it by declaring itself `frozen`. It can still use interior mutability via standard Rust types like `RefCell` or `Mutex`, but it is not bound to the implementation provided by PyO3 and can choose the most appropriate strategy on field-by-field basis.

Classes which are `frozen` and also `Sync`, e.g. they do use `Mutex` but not `RefCell`, can be accessed without needing the Python GIL via the `PyCell::get` and `Py::get` methods:

```rust
use std::sync::atomic::{AtomicUsize, Ordering};
# use pyo3::prelude::*;

#[pyclass(frozen)]
struct FrozenCounter {
value: AtomicUsize,
}

let py_counter: Py<FrozenCounter> = Python::with_gil(|py| {
let counter = FrozenCounter { value: AtomicUsize::new(0) };

Py::new(py, counter).unwrap()
});

py_counter.get().value.fetch_add(1, Ordering::Relaxed);
```

Frozen classes are likely to become the default thereby guiding the PyO3 ecosystem towards a more deliberate application of interior mutability. Eventually, this should enable further optimizations of PyO3's internals and avoid downstream code paying the cost of interior mutability when it is not actually required.

## Customizing the class

{{#include ../pyclass_parameters.md}}
Expand Down
1 change: 1 addition & 0 deletions newsfragments/3158.added.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add `PyClass::get` and `Py::get` for GIL-indepedent access to internally synchronized frozen classes.
41 changes: 40 additions & 1 deletion src/instance.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
use crate::pyclass::boolean_struct::False;
// Copyright (c) 2017-present PyO3 Project and Contributors
use crate::conversion::PyTryFrom;
use crate::err::{self, PyDowncastError, PyErr, PyResult};
use crate::gil;
use crate::pycell::{PyBorrowError, PyBorrowMutError, PyCell};
use crate::pyclass::boolean_struct::{False, True};
use crate::types::{PyDict, PyString, PyTuple};
use crate::{
ffi, AsPyPointer, FromPyObject, IntoPy, IntoPyPointer, PyAny, PyClass, PyClassInitializer,
Expand Down Expand Up @@ -366,6 +366,8 @@ where
/// This borrow lasts while the returned [`PyRef`] exists.
/// Multiple immutable borrows can be taken out at the same time.
///
/// For frozen classes, the simpler [`get`][Self::get] is available.
///
/// Equivalent to `self.as_ref(py).borrow()` -
/// see [`PyCell::borrow`](crate::pycell::PyCell::borrow).
///
Expand Down Expand Up @@ -444,6 +446,8 @@ where
///
/// This is the non-panicking variant of [`borrow`](#method.borrow).
///
/// For frozen classes, the simpler [`get`][Self::get] is available.
///
/// Equivalent to `self.as_ref(py).borrow_mut()` -
/// see [`PyCell::try_borrow`](crate::pycell::PyCell::try_borrow).
pub fn try_borrow<'py>(&'py self, py: Python<'py>) -> Result<PyRef<'py, T>, PyBorrowError> {
Expand All @@ -467,6 +471,41 @@ where
{
self.as_ref(py).try_borrow_mut()
}

/// Provide an immutable borrow of the value `T` without acquiring the GIL.
///
/// This is available if the class is [`frozen`][macro@crate::pyclass] and [`Sync`].
///
/// # Examples
///
/// ```
/// use std::sync::atomic::{AtomicUsize, Ordering};
/// # use pyo3::prelude::*;
///
/// #[pyclass(frozen)]
/// struct FrozenCounter {
/// value: AtomicUsize,
/// }
///
/// let cell = Python::with_gil(|py| {
/// let counter = FrozenCounter { value: AtomicUsize::new(0) };
///
/// Py::new(py, counter).unwrap()
/// });
///
/// cell.get().value.fetch_add(1, Ordering::Relaxed);
/// ```
pub fn get(&self) -> &T
where
T: PyClass<Frozen = True> + Sync,
{
let any = self.as_ptr() as *const PyAny;
// SAFETY: The class itself is frozen and `Sync` and we do not access anything but `cell.contents.value`.
unsafe {
let cell: &PyCell<T> = PyNativeType::unchecked_downcast(&*any);
&*cell.get_ptr()
}
}
}

impl<T> Py<T> {
Expand Down
46 changes: 44 additions & 2 deletions src/pycell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,10 @@ use crate::exceptions::PyRuntimeError;
use crate::impl_::pyclass::{
PyClassBaseType, PyClassDict, PyClassImpl, PyClassThreadChecker, PyClassWeakRef,
};
use crate::pyclass::{boolean_struct::False, PyClass};
use crate::pyclass::{
boolean_struct::{False, True},
PyClass,
};
use crate::pyclass_init::PyClassInitializer;
use crate::type_object::{PyLayout, PySizedLayout};
use crate::types::PyAny;
Expand Down Expand Up @@ -290,6 +293,8 @@ impl<T: PyClass> PyCell<T> {

/// Immutably borrows the value `T`. This borrow lasts as long as the returned `PyRef` exists.
///
/// For frozen classes, the simpler [`get`][Self::get] is available.
///
/// # Panics
///
/// Panics if the value is currently mutably borrowed. For a non-panicking variant, use
Expand All @@ -316,6 +321,8 @@ impl<T: PyClass> PyCell<T> {
///
/// This is the non-panicking variant of [`borrow`](#method.borrow).
///
/// For frozen classes, the simpler [`get`][Self::get] is available.
///
/// # Examples
///
/// ```
Expand Down Expand Up @@ -410,6 +417,41 @@ impl<T: PyClass> PyCell<T> {
.map(|_: ()| &*self.contents.value.get())
}

/// Provide an immutable borrow of the value `T` without acquiring the GIL.
///
/// This is available if the class is [`frozen`][macro@crate::pyclass] and [`Sync`].
///
/// While the GIL is usually required to get access to `&PyCell<T>`,
/// compared to [`borrow`][Self::borrow] or [`try_borrow`][Self::try_borrow]
/// this avoids any thread or borrow checking overhead at runtime.
///
/// # Examples
///
/// ```
/// use std::sync::atomic::{AtomicUsize, Ordering};
/// # use pyo3::prelude::*;
///
/// #[pyclass(frozen)]
/// struct FrozenCounter {
/// value: AtomicUsize,
/// }
///
/// Python::with_gil(|py| {
/// let counter = FrozenCounter { value: AtomicUsize::new(0) };
///
/// let cell = PyCell::new(py, counter).unwrap();
///
/// cell.get().value.fetch_add(1, Ordering::Relaxed);
/// });
/// ```
pub fn get(&self) -> &T
where
T: PyClass<Frozen = True> + Sync,
{
// SAFETY: The class itself is frozen and `Sync` and we do not access anything but `self.contents.value`.
unsafe { &*self.get_ptr() }
}

/// Replaces the wrapped value with a new one, returning the old value.
///
/// # Panics
Expand Down Expand Up @@ -450,7 +492,7 @@ impl<T: PyClass> PyCell<T> {
std::mem::swap(&mut *self.borrow_mut(), &mut *other.borrow_mut())
}

fn get_ptr(&self) -> *mut T {
pub(crate) fn get_ptr(&self) -> *mut T {
self.contents.value.get()
}

Expand Down
24 changes: 24 additions & 0 deletions tests/test_class_basics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -502,3 +502,27 @@ fn inherited_weakref() {
);
});
}

#[test]
fn access_frozen_class_without_gil() {
use std::sync::atomic::{AtomicUsize, Ordering};

#[pyclass(frozen)]
struct FrozenCounter {
value: AtomicUsize,
}

let py_counter: Py<FrozenCounter> = Python::with_gil(|py| {
let counter = FrozenCounter {
value: AtomicUsize::new(0),
};

let cell = PyCell::new(py, counter).unwrap();

cell.get().value.fetch_add(1, Ordering::Relaxed);

cell.into()
});

assert_eq!(py_counter.get().value.load(Ordering::Relaxed), 1);
}
14 changes: 11 additions & 3 deletions tests/ui/invalid_frozen_pyclass_borrow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ pub struct Foo {
field: u32,
}

fn borrow_mut_fails(foo: Py<Foo>, py: Python){
fn borrow_mut_fails(foo: Py<Foo>, py: Python) {
let borrow = foo.as_ref(py).borrow_mut();
}

Expand All @@ -16,8 +16,16 @@ struct MutableBase;
#[pyclass(frozen, extends = MutableBase)]
struct ImmutableChild;

fn borrow_mut_of_child_fails(child: Py<ImmutableChild>, py: Python){
fn borrow_mut_of_child_fails(child: Py<ImmutableChild>, py: Python) {
let borrow = child.as_ref(py).borrow_mut();
}

fn main(){}
fn py_get_of_mutable_class_fails(class: Py<MutableBase>) {
class.get();
}

fn pyclass_get_of_mutable_class_fails(class: &PyCell<MutableBase>) {
class.get();
}

fn main() {}
28 changes: 26 additions & 2 deletions tests/ui/invalid_frozen_pyclass_borrow.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ error[E0271]: type mismatch resolving `<Foo as PyClass>::Frozen == False`
10 | let borrow = foo.as_ref(py).borrow_mut();
| ^^^^^^^^^^ expected `False`, found `True`
|
note: required by a bound in `PyCell::<T>::borrow_mut`
note: required by a bound in `pyo3::PyCell::<T>::borrow_mut`
--> src/pycell.rs
|
| T: PyClass<Frozen = False>,
Expand All @@ -16,8 +16,32 @@ error[E0271]: type mismatch resolving `<ImmutableChild as PyClass>::Frozen == Fa
20 | let borrow = child.as_ref(py).borrow_mut();
| ^^^^^^^^^^ expected `False`, found `True`
|
note: required by a bound in `PyCell::<T>::borrow_mut`
note: required by a bound in `pyo3::PyCell::<T>::borrow_mut`
--> src/pycell.rs
|
| T: PyClass<Frozen = False>,
| ^^^^^^^^^^^^^^ required by this bound in `PyCell::<T>::borrow_mut`

error[E0271]: type mismatch resolving `<MutableBase as PyClass>::Frozen == True`
--> tests/ui/invalid_frozen_pyclass_borrow.rs:24:11
|
24 | class.get();
| ^^^ expected `True`, found `False`
|
note: required by a bound in `pyo3::Py::<T>::get`
--> src/instance.rs
|
| T: PyClass<Frozen = True> + Sync,
| ^^^^^^^^^^^^^ required by this bound in `Py::<T>::get`

error[E0271]: type mismatch resolving `<MutableBase as PyClass>::Frozen == True`
--> tests/ui/invalid_frozen_pyclass_borrow.rs:28:11
|
28 | class.get();
| ^^^ expected `True`, found `False`
|
note: required by a bound in `pyo3::PyCell::<T>::get`
--> src/pycell.rs
|
| T: PyClass<Frozen = True> + Sync,
| ^^^^^^^^^^^^^ required by this bound in `PyCell::<T>::get`