From 8c8ad93e60bcf52d740dd929a486258e670be57c Mon Sep 17 00:00:00 2001 From: ijl <ijl@mailbox.org> Date: Sun, 11 Nov 2018 17:19:05 +0000 Subject: [PATCH] PyObjectAlloc no longer uses specialization and freelist is removed The `impl<T> PyObjectAlloc` for PyObjectWithFreeList is an obstacle to removing specialization. It is unused in all projects linked from the README and within pyo3 itself, so let's just remove it. --- CHANGELOG.md | 5 +- pyo3-derive-backend/src/py_class.rs | 30 ------ src/freelist.rs | 139 ---------------------------- src/lib.rs | 1 - src/typeob.rs | 4 +- tests/test_gc.rs | 30 ------ 6 files changed, 6 insertions(+), 203 deletions(-) delete mode 100644 src/freelist.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 9da4facb962..5b5b533ac38 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## [Unreleased] +### Removed + * `pyo3::freelist` removed. + ## [0.5.0] - 2018-11-11 ### Added @@ -26,7 +29,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Changed * Removes the types from the root module and the prelude. They now live in `pyo3::types` instead. - * All exceptions are consturcted with `py_err` instead of `new`, as they return `PyErr` and not `Self`. + * All exceptions are constructed with `py_err` instead of `new`, as they return `PyErr` and not `Self`. * `as_mut` and friends take and `&mut self` instead of `&self` * `ObjectProtocol::call` now takes an `Option<&PyDict>` for the kwargs instead of an `IntoPyDictPointer`. * `IntoPyDictPointer` was replace by `IntoPyDict` which doesn't convert `PyDict` itself anymore and returns a `PyDict` instead of `*mut PyObject`. diff --git a/pyo3-derive-backend/src/py_class.rs b/pyo3-derive-backend/src/py_class.rs index 1f311cd8725..8ba63aa70db 100644 --- a/pyo3-derive-backend/src/py_class.rs +++ b/pyo3-derive-backend/src/py_class.rs @@ -115,32 +115,6 @@ fn impl_class( None }; - let extra = { - if let Some(freelist) = params.get("freelist") { - Some(quote! { - impl ::pyo3::freelist::PyObjectWithFreeList for #cls { - #[inline] - fn get_free_list() -> &'static mut ::pyo3::freelist::FreeList<*mut ::pyo3::ffi::PyObject> { - static mut FREELIST: *mut ::pyo3::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))); - - <#cls as ::pyo3::typeob::PyTypeCreate>::init_type(); - } - &mut *FREELIST - } - } - } - - #extra - }) - } else { - extra - } - }; - let extra = if !descriptors.is_empty() { let ty = syn::parse_str(&cls.to_string()).expect("no name"); let desc_impls = impl_descriptors(&ty, descriptors); @@ -396,10 +370,6 @@ fn parse_attribute( }; match key.as_str() { - "freelist" => { - // TODO: check if int literal - params.insert("freelist", *ass.right.clone()); - } "name" => match *ass.right { syn::Expr::Path(ref exp) if exp.path.segments.len() == 1 => { params.insert("name", exp.clone().into()); diff --git a/src/freelist.rs b/src/freelist.rs deleted file mode 100644 index 4e6d0414ca8..00000000000 --- a/src/freelist.rs +++ /dev/null @@ -1,139 +0,0 @@ -// Copyright (c) 2017-present PyO3 Project and Contributors - -//! Free allocation list - -use crate::err::PyResult; -use crate::ffi; -use crate::python::Python; -use crate::typeob::{pytype_drop, PyObjectAlloc, PyTypeInfo}; -use std::mem; -use std::os::raw::c_void; - -/// Implementing this trait for custom class adds free allocation list to class. -/// The performance improvement applies to types that are often created and deleted in a row, -/// so that they can benefit from a freelist. -pub trait PyObjectWithFreeList: PyTypeInfo { - fn get_free_list() -> &'static mut FreeList<*mut ffi::PyObject>; -} - -pub enum Slot<T> { - Empty, - Filled(T), -} - -pub struct FreeList<T> { - entries: Vec<Slot<T>>, - split: usize, - capacity: usize, -} - -impl<T> FreeList<T> { - /// Create new `FreeList` instance with specified capacity - pub fn with_capacity(capacity: usize) -> FreeList<T> { - let entries = (0..capacity).map(|_| Slot::Empty).collect::<Vec<_>>(); - - FreeList { - entries, - split: 0, - capacity, - } - } - - /// Pop first non empty item - pub fn pop(&mut self) -> Option<T> { - let idx = self.split; - if idx == 0 { - None - } else { - match mem::replace(&mut self.entries[idx - 1], Slot::Empty) { - Slot::Filled(v) => { - self.split = idx - 1; - Some(v) - } - _ => panic!("FreeList is corrupt"), - } - } - } - - /// Insert a value into the list - pub fn insert(&mut self, val: T) -> Option<T> { - let next = self.split + 1; - if next < self.capacity { - self.entries[self.split] = Slot::Filled(val); - self.split = next; - None - } else { - Some(val) - } - } -} - -impl<T> PyObjectAlloc<T> for T -where - T: PyObjectWithFreeList, -{ - unsafe fn alloc(_py: Python) -> PyResult<*mut ffi::PyObject> { - let obj = if let Some(obj) = <T as PyObjectWithFreeList>::get_free_list().pop() { - ffi::PyObject_Init(obj, <T as PyTypeInfo>::type_object()); - obj - } else { - ffi::PyType_GenericAlloc(<T as PyTypeInfo>::type_object(), 0) - }; - - Ok(obj) - } - - #[cfg(Py_3)] - unsafe fn dealloc(py: Python, obj: *mut ffi::PyObject) { - pytype_drop::<T>(py, obj); - - if ffi::PyObject_CallFinalizerFromDealloc(obj) < 0 { - return; - } - - if let Some(obj) = <T as PyObjectWithFreeList>::get_free_list().insert(obj) { - match (*T::type_object()).tp_free { - Some(free) => free(obj as *mut c_void), - None => { - let ty = ffi::Py_TYPE(obj); - if ffi::PyType_IS_GC(ty) != 0 { - ffi::PyObject_GC_Del(obj as *mut c_void); - } else { - ffi::PyObject_Free(obj as *mut c_void); - } - - // For heap types, PyType_GenericAlloc calls INCREF on the type objects, - // so we need to call DECREF here: - if ffi::PyType_HasFeature(ty, ffi::Py_TPFLAGS_HEAPTYPE) != 0 { - ffi::Py_DECREF(ty as *mut ffi::PyObject); - } - } - } - } - } - - #[cfg(not(Py_3))] - unsafe fn dealloc(py: Python, obj: *mut ffi::PyObject) { - pytype_drop::<T>(py, obj); - - if let Some(obj) = <T as PyObjectWithFreeList>::get_free_list().insert(obj) { - match (*T::type_object()).tp_free { - Some(free) => free(obj as *mut c_void), - None => { - let ty = ffi::Py_TYPE(obj); - if ffi::PyType_IS_GC(ty) != 0 { - ffi::PyObject_GC_Del(obj as *mut c_void); - } else { - ffi::PyObject_Free(obj as *mut c_void); - } - - // For heap types, PyType_GenericAlloc calls INCREF on the type objects, - // so we need to call DECREF here: - if ffi::PyType_HasFeature(ty, ffi::Py_TPFLAGS_HEAPTYPE) != 0 { - ffi::Py_DECREF(ty as *mut ffi::PyObject); - } - } - } - } - } -} diff --git a/src/lib.rs b/src/lib.rs index 5841450bd58..f34b84f594b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -178,7 +178,6 @@ mod conversion; #[doc(hidden)] pub mod derive_utils; mod err; -pub mod freelist; mod instance; mod noargs; mod object; diff --git a/src/typeob.rs b/src/typeob.rs index b118c733eef..3af33607664 100644 --- a/src/typeob.rs +++ b/src/typeob.rs @@ -219,7 +219,7 @@ where pytype_drop::<T>(py, obj); } - default unsafe fn alloc(_py: Python) -> PyResult<*mut ffi::PyObject> { + unsafe fn alloc(_py: Python) -> PyResult<*mut ffi::PyObject> { // TODO: remove this <T as PyTypeCreate>::init_type(); @@ -230,7 +230,7 @@ where Ok(obj) } - default unsafe fn dealloc(py: Python, obj: *mut ffi::PyObject) { + unsafe fn dealloc(py: Python, obj: *mut ffi::PyObject) { Self::drop(py, obj); #[cfg(Py_3)] diff --git a/tests/test_gc.rs b/tests/test_gc.rs index fbf4a533ce3..94f833cfd83 100644 --- a/tests/test_gc.rs +++ b/tests/test_gc.rs @@ -19,36 +19,6 @@ use std::sync::Arc; #[macro_use] mod common; -#[pyclass(freelist = 2)] -struct ClassWithFreelist { - token: PyToken, -} - -#[test] -fn class_with_freelist() { - let ptr; - { - let gil = Python::acquire_gil(); - let py = gil.python(); - - let inst = Py::new(py, |t| ClassWithFreelist { token: t }).unwrap(); - let _inst2 = Py::new(py, |t| ClassWithFreelist { token: t }).unwrap(); - ptr = inst.as_ptr(); - drop(inst); - } - - { - let gil = Python::acquire_gil(); - let py = gil.python(); - - let inst3 = Py::new(py, |t| ClassWithFreelist { token: t }).unwrap(); - assert_eq!(ptr, inst3.as_ptr()); - - let inst4 = Py::new(py, |t| ClassWithFreelist { token: t }).unwrap(); - assert_ne!(ptr, inst4.as_ptr()) - } -} - struct TestDropCall { drop_called: Arc<AtomicBool>, }