Skip to content

Commit

Permalink
Rewrite LazyTypeObjects without once_cell
Browse files Browse the repository at this point in the history
  • Loading branch information
kngwyu committed Feb 6, 2020
1 parent f8de335 commit 65adef8
Show file tree
Hide file tree
Showing 6 changed files with 91 additions and 101 deletions.
1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ parking_lot = { version = "0.10", features = ["nightly"] }
paste = "0.1.6"
pyo3cls = { path = "pyo3cls", version = "=0.9.0-alpha.1" }
unindent = "0.1.4"
once_cell = "1.3.1"

[dev-dependencies]
assert_approx_eq = "1.1.0"
Expand Down
4 changes: 2 additions & 2 deletions guide/src/class.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@ unsafe impl pyo3::PyTypeInfo for MyClass {
const FLAGS: usize = 0;

#[inline]
fn type_object() -> std::ptr::NonNull<pyo3::ffi::PyTypeObject> {
fn type_object() -> &'static pyo3::ffi::PyTypeObject {
use pyo3::type_object::LazyStaticType;
static TYPE_OBJECT: LazyStaticType = LazyStaticType::new();
TYPE_OBJECT.get_pyclass_type::<Self>()
TYPE_OBJECT.get::<Self>()
}
}

Expand Down
4 changes: 2 additions & 2 deletions pyo3-derive-backend/src/pyclass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -384,8 +384,8 @@ fn impl_class(
#[inline]
fn type_object() -> &'static pyo3::ffi::PyTypeObject {
use pyo3::type_object::LazyStaticType;
static TYPE_OBJECT: LazyStaticType = LazyStaticType::new_static();
TYPE_OBJECT.get_pyclass_type::<Self>()
static TYPE_OBJECT: LazyStaticType = LazyStaticType::new();
TYPE_OBJECT.get::<Self>()
}
}

Expand Down
68 changes: 29 additions & 39 deletions src/exceptions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,30 +91,25 @@ macro_rules! import_exception_type_object {
unsafe impl $crate::type_object::PyTypeObject for $name {
fn type_object() -> $crate::Py<$crate::types::PyType> {
use $crate::type_object::LazyHeapType;
static TYPE_OBJECT: LazyHeapType = LazyHeapType::new_heap();

let ptr = TYPE_OBJECT
.get_or_init(|| {
let gil = $crate::Python::acquire_gil();
let py = gil.python();

let imp = py
.import(stringify!($module))
.expect(concat!("Can not import module: ", stringify!($module)));
let cls = imp.get(stringify!($name)).expect(concat!(
"Can not load exception class: {}.{}",
stringify!($module),
".",
stringify!($name)
));

unsafe {
Ok(std::ptr::NonNull::new_unchecked(
$crate::IntoPyPointer::into_ptr(cls) as *mut _,
))
}
})
.unwrap();
static TYPE_OBJECT: LazyHeapType = LazyHeapType::new();

let ptr = TYPE_OBJECT.get_or_init(|py| {
let imp = py
.import(stringify!($module))
.expect(concat!("Can not import module: ", stringify!($module)));
let cls = imp.get(stringify!($name)).expect(concat!(
"Can not load exception class: {}.{}",
stringify!($module),
".",
stringify!($name)
));

unsafe {
std::ptr::NonNull::new_unchecked(
$crate::IntoPyPointer::into_ptr(cls) as *mut _
)
}
});

unsafe { $crate::Py::from_borrowed_ptr(ptr.as_ptr() as *mut $crate::ffi::PyObject) }
}
Expand Down Expand Up @@ -179,21 +174,16 @@ macro_rules! create_exception_type_object {
unsafe impl $crate::type_object::PyTypeObject for $name {
fn type_object() -> $crate::Py<$crate::types::PyType> {
use $crate::type_object::LazyHeapType;
static TYPE_OBJECT: LazyHeapType = LazyHeapType::new_heap();

let ptr = TYPE_OBJECT
.get_or_init(|| {
let gil = $crate::Python::acquire_gil();
let py = gil.python();

Ok($crate::PyErr::new_type(
py,
concat!(stringify!($module), ".", stringify!($name)),
Some(py.get_type::<$base>()),
None,
))
})
.unwrap();
static TYPE_OBJECT: LazyHeapType = LazyHeapType::new();

let ptr = TYPE_OBJECT.get_or_init(|py| {
$crate::PyErr::new_type(
py,
concat!(stringify!($module), ".", stringify!($name)),
Some(py.get_type::<$base>()),
None,
)
});

unsafe { $crate::Py::from_borrowed_ptr(ptr.as_ptr() as *mut $crate::ffi::PyObject) }
}
Expand Down
23 changes: 10 additions & 13 deletions src/pyclass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -253,17 +253,14 @@ where
}

#[cfg(not(Py_LIMITED_API))]
pub(crate) fn create_type_object<T>(
pub(crate) fn initialize_type_object<T>(
py: Python,
module_name: Option<&str>,
) -> PyResult<Box<ffi::PyTypeObject>>
type_object: &mut ffi::PyTypeObject,
) -> PyResult<()>
where
T: PyClass,
{
// Box (or some other heap allocation) is needed because PyType_Ready expects the type object
// to have a permanent memory address.
let mut type_object = Box::new(ffi::PyTypeObject_INIT);

// PyPy will segfault if passed only a nul terminator as `tp_doc`.
// ptr::null() is OK though.
if T::DESCRIPTION == "\0" {
Expand Down Expand Up @@ -310,16 +307,16 @@ where
}

// GC support
<T as class::gc::PyGCProtocolImpl>::update_type_object(&mut type_object);
<T as class::gc::PyGCProtocolImpl>::update_type_object(type_object);

// descriptor protocol
<T as class::descr::PyDescrProtocolImpl>::tp_as_descr(&mut type_object);
<T as class::descr::PyDescrProtocolImpl>::tp_as_descr(type_object);

// iterator methods
<T as class::iter::PyIterProtocolImpl>::tp_as_iter(&mut type_object);
<T as class::iter::PyIterProtocolImpl>::tp_as_iter(type_object);

// basic methods
<T as class::basic::PyObjectProtocolImpl>::tp_as_object(&mut type_object);
<T as class::basic::PyObjectProtocolImpl>::tp_as_object(type_object);

fn to_ptr<T>(value: Option<T>) -> *mut T {
value
Expand Down Expand Up @@ -364,12 +361,12 @@ where
}

// set type flags
py_class_flags::<T>(&mut type_object);
py_class_flags::<T>(type_object);

// register type object
unsafe {
if ffi::PyType_Ready(type_object.as_mut()) == 0 {
Ok(type_object)
if ffi::PyType_Ready(type_object) == 0 {
Ok(())
} else {
PyErr::fetch(py).into()
}
Expand Down
92 changes: 48 additions & 44 deletions src/type_object.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
// Copyright (c) 2017-present PyO3 Project and Contributors
//! Python type object information
use crate::err::PyResult;
use crate::instance::Py;
use crate::pyclass::{create_type_object, PyClass};
use crate::pyclass::{initialize_type_object, PyClass};
use crate::pyclass_init::PyObjectInit;
use crate::types::{PyAny, PyType};
use crate::{ffi, AsPyPointer, Python};
use once_cell::sync::OnceCell;
use std::cell::{Cell, UnsafeCell};
use std::ptr::NonNull;

/// `T: PyObjectLayout<U>` represents that `T` is a concrete representaion of `U` in Python heap.
/// E.g., `PyClassShell` is a concrete representaion of all `pyclass`es, and `ffi::PyObject`
Expand Down Expand Up @@ -127,61 +127,65 @@ where
}
}

/// Type used to store static type objects
/// Lazy type object for Exceptions
#[doc(hidden)]
pub struct LazyTypeObject<T: Copy> {
cell: OnceCell<T>,
}

/// For exceptions.
#[doc(hidden)]
pub type LazyHeapType = LazyTypeObject<std::ptr::NonNull<ffi::PyTypeObject>>;
pub struct LazyHeapType(UnsafeCell<Option<NonNull<ffi::PyTypeObject>>>);

/// For pyclass.
#[doc(hidden)]
pub type LazyStaticType = LazyTypeObject<&'static ffi::PyTypeObject>;

impl<T: Copy> LazyTypeObject<T> {
pub fn get_or_init<F>(&self, constructor: F) -> PyResult<T>
impl LazyHeapType {
pub const fn new() -> Self {
Self(UnsafeCell::new(None))
}
pub fn get_or_init<F>(&self, constructor: F) -> NonNull<ffi::PyTypeObject>
where
F: Fn() -> PyResult<T>,
F: Fn(Python) -> NonNull<ffi::PyTypeObject>,
{
Ok(*self.cell.get_or_try_init(constructor)?)
if let Some(value) = unsafe { &*self.0.get() } {
return value.clone();
}
// We have to get the GIL before setting the value to the global!!!
let gil = Python::acquire_gil();
let py = gil.python();
unsafe {
*self.0.get() = Some(constructor(py));
(*self.0.get()).unwrap()
}
}
}

impl LazyHeapType {
pub const fn new_heap() -> Self {
Self {
cell: OnceCell::new(),
}
}
// This is necessary for making static `LazyHeapType`s
//
// Type objects are shared between threads by the Python interpreter anyway, so it is no worse
// to allow sharing on the Rust side too.
unsafe impl Sync for LazyHeapType {}

/// Lazy type object for PyClass
#[doc(hidden)]
pub struct LazyStaticType {
value: UnsafeCell<ffi::PyTypeObject>,
initialized: Cell<bool>,
}

impl LazyStaticType {
pub const fn new_static() -> Self {
Self {
cell: OnceCell::new(),
pub const fn new() -> Self {
LazyStaticType {
value: UnsafeCell::new(ffi::PyTypeObject_INIT),
initialized: Cell::new(false),
}
}
pub fn get_pyclass_type<T: PyClass>(&self) -> &'static ffi::PyTypeObject {
self.get_or_init(|| {
// automatically initialize the class on-demand
pub fn get<T: PyClass>(&self) -> &ffi::PyTypeObject {
if !self.initialized.get() {
let gil = Python::acquire_gil();
let py = gil.python();
let boxed = create_type_object::<T>(py, T::MODULE)?;
Ok(Box::leak(boxed))
})
.unwrap_or_else(|e| {
let gil = Python::acquire_gil();
e.print(gil.python());
panic!("An error occurred while initializing class {}", T::NAME)
})
initialize_type_object::<T>(py, T::MODULE, unsafe { &mut *self.value.get() })
.unwrap_or_else(|e| {
e.print(py);
panic!("An error occurred while initializing class {}", T::NAME)
});
self.initialized.set(true);
}
unsafe { &*self.value.get() }
}
}

// This is necessary for making static `LazyTypeObject`s
//
// Type objects are shared between threads by the Python interpreter anyway, so it is no worse
// to allow sharing on the Rust side too.
unsafe impl<T: Copy> Sync for LazyTypeObject<T> {}
// This is necessary for making static `LazyStaticType`s
unsafe impl Sync for LazyStaticType {}

0 comments on commit 65adef8

Please sign in to comment.