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

pyclass: refactor tp_new / tp_dealloc / remove PyCellInner #1657

Merged
merged 2 commits into from
Jun 23, 2021
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
160 changes: 150 additions & 10 deletions CHANGELOG.md

Large diffs are not rendered by default.

12 changes: 12 additions & 0 deletions examples/pyo3-benchmarks/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,17 @@ fn args_kwargs<'a>(
(args, kwargs)
}

#[pyclass]
struct EmptyClass {}

#[pymethods]
impl EmptyClass {
#[new]
fn new() -> Self {
EmptyClass {}
}
}

#[pymodule]
fn _pyo3_benchmarks(_py: Python<'_>, m: &PyModule) -> PyResult<()> {
m.add_function(wrap_pyfunction!(none, m)?)?;
Expand All @@ -63,5 +74,6 @@ fn _pyo3_benchmarks(_py: Python<'_>, m: &PyModule) -> PyResult<()> {
m.add_function(wrap_pyfunction!(simple_kwargs, m)?)?;
m.add_function(wrap_pyfunction!(simple_args_kwargs, m)?)?;
m.add_function(wrap_pyfunction!(args_kwargs, m)?)?;
m.add_class::<EmptyClass>()?;
Ok(())
}
12 changes: 12 additions & 0 deletions examples/pyo3-benchmarks/tests/test_benchmarks.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,3 +89,15 @@ def test_args_kwargs_rs(benchmark):
py = args_kwargs_py(1, "foo", {1: 2}, bar=4, foo=10)
assert rust == py
benchmark(pyo3_benchmarks.args_kwargs, 1, "foo", {1: 2}, a=4, foo=10)


def test_empty_class_init(benchmark):
benchmark(pyo3_benchmarks.EmptyClass)


class EmptyClassPy:
pass


def test_empty_class_init_py(benchmark):
benchmark(EmptyClassPy)
12 changes: 10 additions & 2 deletions guide/src/class.md
Original file line number Diff line number Diff line change
Expand Up @@ -720,8 +720,6 @@ struct MyClass {
debug: bool,
}

impl pyo3::pyclass::PyClassAlloc for MyClass {}

unsafe impl pyo3::PyTypeInfo for MyClass {
type AsRefTarget = PyCell<Self>;

Expand Down Expand Up @@ -774,6 +772,16 @@ impl pyo3::class::impl_::PyClassImpl for MyClass {
let collector = PyClassImplCollector::<Self>::new();
collector.new_impl()
}
fn get_alloc() -> Option<pyo3::ffi::allocfunc> {
use pyo3::class::impl_::*;
let collector = PyClassImplCollector::<Self>::new();
collector.alloc_impl()
}
fn get_free() -> Option<pyo3::ffi::freefunc> {
use pyo3::class::impl_::*;
let collector = PyClassImplCollector::<Self>::new();
collector.free_impl()
}
fn get_call() -> Option<pyo3::ffi::PyCFunctionWithKeywords> {
use pyo3::class::impl_::*;
let collector = PyClassImplCollector::<Self>::new();
Expand Down
42 changes: 30 additions & 12 deletions pyo3-macros-backend/src/pyclass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -378,29 +378,37 @@ fn impl_class(
) -> syn::Result<TokenStream> {
let cls_name = get_class_python_name(cls, attr).to_string();

let alloc = {
if let Some(freelist) = &attr.freelist {
let alloc = attr.freelist.as_ref().map(|freelist| {
quote! {
impl pyo3::freelist::PyClassWithFreeList for #cls {
impl pyo3::class::impl_::PyClassWithFreeList for #cls {
#[inline]
fn get_free_list(_py: pyo3::Python) -> &mut pyo3::freelist::FreeList<*mut pyo3::ffi::PyObject> {
static mut FREELIST: *mut pyo3::freelist::FreeList<*mut pyo3::ffi::PyObject> = 0 as *mut _;
fn get_free_list(_py: pyo3::Python) -> &mut pyo3::impl_::freelist::FreeList<*mut pyo3::ffi::PyObject> {
static mut FREELIST: *mut pyo3::impl_::freelist::FreeList<*mut pyo3::ffi::PyObject> = 0 as *mut _;
unsafe {
if FREELIST.is_null() {
FREELIST = Box::into_raw(Box::new(
pyo3::freelist::FreeList::with_capacity(#freelist)));
pyo3::impl_::freelist::FreeList::with_capacity(#freelist)));
}
&mut *FREELIST
}
}
}

impl pyo3::class::impl_::PyClassAllocImpl<#cls> for pyo3::class::impl_::PyClassImplCollector<#cls> {
#[inline]
fn alloc_impl(self) -> Option<pyo3::ffi::allocfunc> {
Some(pyo3::class::impl_::alloc_with_freelist::<#cls>)
}
}

impl pyo3::class::impl_::PyClassFreeImpl<#cls> for pyo3::class::impl_::PyClassImplCollector<#cls> {
#[inline]
fn free_impl(self) -> Option<pyo3::ffi::freefunc> {
Some(pyo3::class::impl_::free_with_freelist::<#cls>)
}
}
}
} else {
quote! {
impl pyo3::pyclass::PyClassAlloc for #cls {}
}
}
};
});

let descriptors = impl_descriptors(cls, field_options)?;

Expand Down Expand Up @@ -551,6 +559,16 @@ fn impl_class(
let collector = PyClassImplCollector::<Self>::new();
collector.new_impl()
}
fn get_alloc() -> Option<pyo3::ffi::allocfunc> {
use pyo3::class::impl_::*;
let collector = PyClassImplCollector::<Self>::new();
collector.alloc_impl()
}
fn get_free() -> Option<pyo3::ffi::freefunc> {
use pyo3::class::impl_::*;
let collector = PyClassImplCollector::<Self>::new();
collector.free_impl()
}
fn get_call() -> Option<pyo3::ffi::PyCFunctionWithKeywords> {
use pyo3::class::impl_::*;
let collector = PyClassImplCollector::<Self>::new();
Expand Down
135 changes: 132 additions & 3 deletions src/class/impl_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@

use crate::{
ffi,
impl_::freelist::FreeList,
pycell::PyCellLayout,
pyclass_init::PyObjectInit,
type_object::{PyLayout, PyTypeObject},
PyClass, PyMethodDefType, PyNativeType, PyTypeInfo,
PyCell, PyClass, PyMethodDefType, PyNativeType, PyTypeInfo, Python,
};
use std::{marker::PhantomData, thread};
use std::{marker::PhantomData, os::raw::c_void, thread};

/// This type is used as a "dummy" type on which dtolnay specializations are
/// applied to apply implementations from `#[pymethods]` & `#[pyproto]`
Expand Down Expand Up @@ -72,6 +73,12 @@ pub trait PyClassImpl: Sized {
fn get_call() -> Option<ffi::PyCFunctionWithKeywords> {
None
}
fn get_alloc() -> Option<ffi::allocfunc> {
None
}
fn get_free() -> Option<ffi::freefunc> {
None
}
fn for_each_proto_slot(_visitor: &mut dyn FnMut(&[ffi::PyType_Slot])) {}
fn get_buffer() -> Option<&'static PyBufferProcs> {
None
Expand Down Expand Up @@ -100,6 +107,104 @@ impl<T> PyClassCallImpl<T> for &'_ PyClassImplCollector<T> {
}
}

pub trait PyClassAllocImpl<T> {
fn alloc_impl(self) -> Option<ffi::allocfunc>;
}

impl<T> PyClassAllocImpl<T> for &'_ PyClassImplCollector<T> {
fn alloc_impl(self) -> Option<ffi::allocfunc> {
None
}
}

pub trait PyClassFreeImpl<T> {
fn free_impl(self) -> Option<ffi::freefunc>;
}

impl<T> PyClassFreeImpl<T> for &'_ PyClassImplCollector<T> {
fn free_impl(self) -> Option<ffi::freefunc> {
None
}
}

/// Implements a freelist.
///
/// Do not implement this trait manually. Instead, use `#[pyclass(freelist = N)]`
/// on a Rust struct to implement it.
pub trait PyClassWithFreeList: PyClass {
fn get_free_list(py: Python) -> &mut FreeList<*mut ffi::PyObject>;
}

/// Implementation of tp_alloc for `freelist` classes.
pub unsafe extern "C" fn alloc_with_freelist<T: PyClassWithFreeList>(
subtype: *mut ffi::PyTypeObject,
nitems: ffi::Py_ssize_t,
) -> *mut ffi::PyObject {
let py = Python::assume_gil_acquired();

#[cfg(not(Py_3_8))]
bpo_35810_workaround(py, subtype);

let self_type = T::type_object_raw(py);
// If this type is a variable type or the subtype is not equal to this type, we cannot use the
// freelist
if nitems == 0 && subtype == self_type {
if let Some(obj) = T::get_free_list(py).pop() {
ffi::PyObject_Init(obj, subtype);
return obj as _;
}
}

ffi::PyType_GenericAlloc(subtype, nitems)
}

/// Implementation of tp_free for `freelist` classes.
#[allow(clippy::collapsible_if)] // for if cfg!
pub unsafe extern "C" fn free_with_freelist<T: PyClassWithFreeList>(obj: *mut c_void) {
let obj = obj as *mut ffi::PyObject;
debug_assert_eq!(
T::type_object_raw(Python::assume_gil_acquired()),
ffi::Py_TYPE(obj)
);
if let Some(obj) = T::get_free_list(Python::assume_gil_acquired()).insert(obj) {
let ty = ffi::Py_TYPE(obj);

// Deduce appropriate inverse of PyType_GenericAlloc
let free = if ffi::PyType_IS_GC(ty) != 0 {
ffi::PyObject_GC_Del
} else {
ffi::PyObject_Free
};
free(obj as *mut c_void);

if cfg!(Py_3_8) {
if ffi::PyType_HasFeature(ty, ffi::Py_TPFLAGS_HEAPTYPE) != 0 {
ffi::Py_DECREF(ty as *mut ffi::PyObject);
}
}
}
}

/// Workaround for Python issue 35810; no longer necessary in Python 3.8
#[inline]
#[cfg(not(Py_3_8))]
unsafe fn bpo_35810_workaround(_py: Python, ty: *mut ffi::PyTypeObject) {
#[cfg(Py_LIMITED_API)]
{
// Must check version at runtime for abi3 wheels - they could run against a higher version
// than the build config suggests.
use crate::once_cell::GILOnceCell;
static IS_PYTHON_3_8: GILOnceCell<bool> = GILOnceCell::new();

if *IS_PYTHON_3_8.get_or_init(_py, || _py.version_info() >= (3, 8)) {
// No fix needed - the wheel is running on a sufficiently new interpreter.
return;
}
}

ffi::Py_INCREF(ty as *mut ffi::PyObject);
}

// General methods implementation: either dtolnay specialization trait or inventory if
// multiple-pymethods feature is enabled.

Expand Down Expand Up @@ -216,6 +321,7 @@ pub struct ThreadCheckerStub<T: Send>(PhantomData<T>);

impl<T: Send> PyClassThreadChecker<T> for ThreadCheckerStub<T> {
fn ensure(&self) {}
#[inline]
fn new() -> Self {
ThreadCheckerStub(PhantomData)
}
Expand All @@ -224,6 +330,7 @@ impl<T: Send> PyClassThreadChecker<T> for ThreadCheckerStub<T> {

impl<T: PyNativeType> PyClassThreadChecker<T> for ThreadCheckerStub<crate::PyObject> {
fn ensure(&self) {}
#[inline]
fn new() -> Self {
ThreadCheckerStub(PhantomData)
}
Expand Down Expand Up @@ -279,8 +386,30 @@ pub trait PyClassBaseType: Sized {
impl<T: PyClass> PyClassBaseType for T {
type Dict = T::Dict;
type WeakRef = T::WeakRef;
type LayoutAsBase = crate::pycell::PyCellInner<T>;
type LayoutAsBase = crate::pycell::PyCell<T>;
type BaseNativeType = T::BaseNativeType;
type ThreadChecker = T::ThreadChecker;
type Initializer = crate::pyclass_init::PyClassInitializer<Self>;
}

// Default new implementation

pub(crate) unsafe extern "C" fn fallback_new(
_subtype: *mut ffi::PyTypeObject,
_args: *mut ffi::PyObject,
_kwds: *mut ffi::PyObject,
) -> *mut ffi::PyObject {
crate::callback_body!(py, {
Err::<(), _>(crate::exceptions::PyTypeError::new_err(
"No constructor defined",
))
})
}

pub(crate) unsafe extern "C" fn tp_dealloc<T: PyClass>(obj: *mut ffi::PyObject) {
crate::callback_body!(py, {
// Safety: Python will only call tp_dealloc when no references to the object remain.
let cell: &mut PyCell<T> = &mut *(obj as *mut _);
cell.tp_dealloc(py);
})
}
1 change: 0 additions & 1 deletion src/ffi/datetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -650,7 +650,6 @@ mod tests {
fn test_date_fromtimestamp() {
Python::with_gil(|py| {
let args: Py<PyAny> = (100,).into_py(py);
dbg!(args.as_ref(py));
unsafe { PyDateTime_IMPORT() };
let dt: &PyAny = unsafe { py.from_owned_ptr(PyDate_FromTimestamp(args.as_ptr())) };
py_run!(
Expand Down
Loading