diff --git a/.cspell.dict/cpython.txt b/.cspell.dict/cpython.txt index 41746996ceb..a35c7c8834e 100644 --- a/.cspell.dict/cpython.txt +++ b/.cspell.dict/cpython.txt @@ -70,6 +70,7 @@ finalizers firsttraceable flowgraph formatfloat +freelist freevar freevars fromlist diff --git a/crates/vm/src/builtins/dict.rs b/crates/vm/src/builtins/dict.rs index 43891d3b7f7..c630fc25dff 100644 --- a/crates/vm/src/builtins/dict.rs +++ b/crates/vm/src/builtins/dict.rs @@ -26,6 +26,8 @@ use crate::{ vm::VirtualMachine, }; use alloc::fmt; +use core::cell::Cell; +use core::ptr::NonNull; use rustpython_common::lock::PyMutex; use rustpython_common::wtf8::Wtf8Buf; @@ -60,11 +62,43 @@ impl fmt::Debug for PyDict { } } +thread_local! { + static DICT_FREELIST: Cell> = const { Cell::new(crate::object::FreeList::new()) }; +} + impl PyPayload for PyDict { + const MAX_FREELIST: usize = 80; + const HAS_FREELIST: bool = true; + #[inline] fn class(ctx: &Context) -> &'static Py { ctx.types.dict_type } + + #[inline] + unsafe fn freelist_push(obj: *mut PyObject) -> bool { + DICT_FREELIST.with(|fl| { + let mut list = fl.take(); + let stored = if list.len() < Self::MAX_FREELIST { + list.push(obj); + true + } else { + false + }; + fl.set(list); + stored + }) + } + + #[inline] + unsafe fn freelist_pop() -> Option> { + DICT_FREELIST.with(|fl| { + let mut list = fl.take(); + let result = list.pop().map(|p| unsafe { NonNull::new_unchecked(p) }); + fl.set(list); + result + }) + } } impl PyDict { diff --git a/crates/vm/src/builtins/float.rs b/crates/vm/src/builtins/float.rs index 7ff47328d34..f78b227fc38 100644 --- a/crates/vm/src/builtins/float.rs +++ b/crates/vm/src/builtins/float.rs @@ -15,6 +15,8 @@ use crate::{ protocol::PyNumberMethods, types::{AsNumber, Callable, Comparable, Constructor, Hashable, PyComparisonOp, Representable}, }; +use core::cell::Cell; +use core::ptr::NonNull; use malachite_bigint::{BigInt, ToBigInt}; use num_complex::Complex64; use num_traits::{Signed, ToPrimitive, Zero}; @@ -32,11 +34,43 @@ impl PyFloat { } } +thread_local! { + static FLOAT_FREELIST: Cell> = const { Cell::new(crate::object::FreeList::new()) }; +} + impl PyPayload for PyFloat { + const MAX_FREELIST: usize = 100; + const HAS_FREELIST: bool = true; + #[inline] fn class(ctx: &Context) -> &'static Py { ctx.types.float_type } + + #[inline] + unsafe fn freelist_push(obj: *mut PyObject) -> bool { + FLOAT_FREELIST.with(|fl| { + let mut list = fl.take(); + let stored = if list.len() < Self::MAX_FREELIST { + list.push(obj); + true + } else { + false + }; + fl.set(list); + stored + }) + } + + #[inline] + unsafe fn freelist_pop() -> Option> { + FLOAT_FREELIST.with(|fl| { + let mut list = fl.take(); + let result = list.pop().map(|p| unsafe { NonNull::new_unchecked(p) }); + fl.set(list); + result + }) + } } impl ToPyObject for f64 { diff --git a/crates/vm/src/builtins/list.rs b/crates/vm/src/builtins/list.rs index 84d7a4e309c..ff3ed64f263 100644 --- a/crates/vm/src/builtins/list.rs +++ b/crates/vm/src/builtins/list.rs @@ -27,7 +27,9 @@ use crate::{ use rustpython_common::wtf8::Wtf8Buf; use alloc::fmt; +use core::cell::Cell; use core::ops::DerefMut; +use core::ptr::NonNull; #[pyclass(module = false, name = "list", unhashable = true, traverse = "manual")] #[derive(Default)] @@ -72,11 +74,43 @@ unsafe impl Traverse for PyList { } } +thread_local! { + static LIST_FREELIST: Cell> = const { Cell::new(crate::object::FreeList::new()) }; +} + impl PyPayload for PyList { + const MAX_FREELIST: usize = 80; + const HAS_FREELIST: bool = true; + #[inline] fn class(ctx: &Context) -> &'static Py { ctx.types.list_type } + + #[inline] + unsafe fn freelist_push(obj: *mut PyObject) -> bool { + LIST_FREELIST.with(|fl| { + let mut list = fl.take(); + let stored = if list.len() < Self::MAX_FREELIST { + list.push(obj); + true + } else { + false + }; + fl.set(list); + stored + }) + } + + #[inline] + unsafe fn freelist_pop() -> Option> { + LIST_FREELIST.with(|fl| { + let mut list = fl.take(); + let result = list.pop().map(|p| unsafe { NonNull::new_unchecked(p) }); + fl.set(list); + result + }) + } } impl ToPyObject for Vec { diff --git a/crates/vm/src/builtins/slice.rs b/crates/vm/src/builtins/slice.rs index 4b3bec0530d..c0b68ed9e8a 100644 --- a/crates/vm/src/builtins/slice.rs +++ b/crates/vm/src/builtins/slice.rs @@ -1,5 +1,7 @@ // sliceobject.{h,c} in CPython // spell-checker:ignore sliceobject +use core::cell::Cell; +use core::ptr::NonNull; use rustpython_common::wtf8::{Wtf8Buf, wtf8_concat}; use super::{PyGenericAlias, PyStrRef, PyTupleRef, PyType, PyTypeRef}; @@ -15,7 +17,7 @@ use crate::{ use malachite_bigint::{BigInt, ToBigInt}; use num_traits::{One, Signed, Zero}; -#[pyclass(module = false, name = "slice", unhashable = true, traverse)] +#[pyclass(module = false, name = "slice", unhashable = true, traverse = "manual")] #[derive(Debug)] pub struct PySlice { pub start: Option, @@ -23,11 +25,63 @@ pub struct PySlice { pub step: Option, } +// SAFETY: Traverse properly visits all owned PyObjectRefs +unsafe impl crate::object::Traverse for PySlice { + fn traverse(&self, traverse_fn: &mut crate::object::TraverseFn<'_>) { + self.start.traverse(traverse_fn); + self.stop.traverse(traverse_fn); + self.step.traverse(traverse_fn); + } + + fn clear(&mut self, out: &mut Vec) { + if let Some(start) = self.start.take() { + out.push(start); + } + // stop is not Option, so it will be freed when payload is dropped + // (via drop_in_place on freelist pop, or Box::from_raw on dealloc) + if let Some(step) = self.step.take() { + out.push(step); + } + } +} + +thread_local! { + static SLICE_FREELIST: Cell> = const { Cell::new(crate::object::FreeList::new()) }; +} + impl PyPayload for PySlice { + const MAX_FREELIST: usize = 1; + const HAS_FREELIST: bool = true; + #[inline] fn class(ctx: &Context) -> &'static Py { ctx.types.slice_type } + + #[inline] + unsafe fn freelist_push(obj: *mut PyObject) -> bool { + SLICE_FREELIST.with(|fl| { + let mut list = fl.take(); + let stored = if list.len() < Self::MAX_FREELIST { + list.push(obj); + true + } else { + false + }; + fl.set(list); + stored + }) + } + + #[inline] + unsafe fn freelist_pop() -> Option> { + SLICE_FREELIST.with(|fl| { + let mut list = fl.take(); + let result = list.pop().map(|p| unsafe { NonNull::new_unchecked(p) }); + fl.set(list); + result + }) + } } #[pyclass(with(Comparable, Representable, Hashable))] diff --git a/crates/vm/src/builtins/tuple.rs b/crates/vm/src/builtins/tuple.rs index 8ca2f74a3bf..b7ed066f1d1 100644 --- a/crates/vm/src/builtins/tuple.rs +++ b/crates/vm/src/builtins/tuple.rs @@ -53,6 +53,9 @@ unsafe impl Traverse for PyTuple { } } +// No freelist for PyTuple: structseq types (stat_result, struct_time, etc.) +// are static subtypes sharing the same Rust payload, making type-safe reuse +// impractical without a type-pointer comparison at push time. impl PyPayload for PyTuple { #[inline] fn class(ctx: &Context) -> &'static Py { diff --git a/crates/vm/src/object/core.rs b/crates/vm/src/object/core.rs index 41ddfa26b2e..f4e458e1484 100644 --- a/crates/vm/src/object/core.rs +++ b/crates/vm/src/object/core.rs @@ -171,13 +171,19 @@ pub(super) unsafe fn default_dealloc(obj: *mut PyObject) { // Untrack from GC BEFORE deallocation. if obj_ref.is_gc_tracked() { let ptr = unsafe { NonNull::new_unchecked(obj) }; - rustpython_common::refcount::try_defer_drop(move || { - // untrack_object only removes the pointer address from a HashSet. - // It does NOT dereference the pointer, so it's safe even after deallocation. - unsafe { - crate::gc_state::gc_state().untrack_object(ptr); - } - }); + if T::HAS_FREELIST { + // Freelist types must untrack immediately to avoid race conditions: + // a deferred untrack could remove a re-tracked entry after reuse. + unsafe { crate::gc_state::gc_state().untrack_object(ptr) }; + } else { + rustpython_common::refcount::try_defer_drop(move || { + // untrack_object only removes the pointer address from a HashSet. + // It does NOT dereference the pointer, so it's safe even after deallocation. + unsafe { + crate::gc_state::gc_state().untrack_object(ptr); + } + }); + } } // Extract child references before deallocation to break circular refs (tp_clear) @@ -186,8 +192,17 @@ pub(super) unsafe fn default_dealloc(obj: *mut PyObject) { unsafe { clear_fn(obj, &mut edges) }; } - // Deallocate the object memory - drop(unsafe { Box::from_raw(obj as *mut PyInner) }); + // Try to store in freelist for reuse; otherwise deallocate. + // Only exact types (not heaptype subclasses) go into the freelist, + // because the pop site assumes the cached typ matches the base type. + let pushed = if T::HAS_FREELIST && obj_ref.class().heaptype_ext.is_none() { + unsafe { T::freelist_push(obj) } + } else { + false + }; + if !pushed { + drop(unsafe { Box::from_raw(obj as *mut PyInner) }); + } // Drop child references - may trigger recursive destruction. // The object is already deallocated, so circular refs are broken. @@ -807,6 +822,52 @@ impl PyInner { } } +/// Thread-local freelist storage that properly deallocates cached objects +/// on thread teardown. +/// +/// Wraps a `Vec<*mut PyObject>` and implements `Drop` to convert each +/// raw pointer back into `Box>` for proper deallocation. +pub(crate) struct FreeList { + items: Vec<*mut PyObject>, + _marker: core::marker::PhantomData, +} + +impl FreeList { + pub(crate) const fn new() -> Self { + Self { + items: Vec::new(), + _marker: core::marker::PhantomData, + } + } +} + +impl Default for FreeList { + fn default() -> Self { + Self::new() + } +} + +impl Drop for FreeList { + fn drop(&mut self) { + for ptr in self.items.drain(..) { + drop(unsafe { Box::from_raw(ptr as *mut PyInner) }); + } + } +} + +impl core::ops::Deref for FreeList { + type Target = Vec<*mut PyObject>; + fn deref(&self) -> &Self::Target { + &self.items + } +} + +impl core::ops::DerefMut for FreeList { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.items + } +} + /// The `PyObjectRef` is one of the most used types. It is a reference to a /// python object. A single python object can have multiple references, and /// this reference counting is accounted for by this type. Use the `.clone()` @@ -1720,8 +1781,31 @@ impl PyRef { pub fn new_ref(payload: T, typ: crate::builtins::PyTypeRef, dict: Option) -> Self { let has_dict = dict.is_some(); let is_heaptype = typ.heaptype_ext.is_some(); - let inner = Box::into_raw(PyInner::new(payload, typ, dict)); - let ptr = unsafe { NonNull::new_unchecked(inner.cast::>()) }; + + // Try to reuse from freelist (exact type only, no dict, no heaptype) + let cached = if !has_dict && !is_heaptype { + unsafe { T::freelist_pop() } + } else { + None + }; + + let ptr = if let Some(cached) = cached { + let inner = cached.as_ptr() as *mut PyInner; + unsafe { + core::ptr::write(&mut (*inner).ref_count, RefCount::new()); + (*inner).gc_bits.store(0, Ordering::Relaxed); + core::ptr::drop_in_place(&mut (*inner).payload); + core::ptr::write(&mut (*inner).payload, payload); + // typ, vtable, slots are preserved; dict is None, weak_list was + // cleared by drop_slow_inner before freelist push + } + // Drop the caller's typ since the cached object already holds one + drop(typ); + unsafe { NonNull::new_unchecked(inner.cast::>()) } + } else { + let inner = Box::into_raw(PyInner::new(payload, typ, dict)); + unsafe { NonNull::new_unchecked(inner.cast::>()) } + }; // Track object if: // - HAS_TRAVERSE is true (Rust payload implements Traverse), OR diff --git a/crates/vm/src/object/payload.rs b/crates/vm/src/object/payload.rs index 98c61817568..1af954505f7 100644 --- a/crates/vm/src/object/payload.rs +++ b/crates/vm/src/object/payload.rs @@ -5,6 +5,7 @@ use crate::{ types::PyTypeFlags, vm::{Context, VirtualMachine}, }; +use core::ptr::NonNull; cfg_if::cfg_if! { if #[cfg(feature = "threading")] { @@ -46,6 +47,38 @@ pub trait PyPayload: MaybeTraverse + PyThreadingConstraint + Sized + 'static { fn class(ctx: &Context) -> &'static Py; + /// Whether this type has a freelist. Types with freelists require + /// immediate (non-deferred) GC untracking during dealloc to prevent + /// race conditions when the object is reused. + const HAS_FREELIST: bool = false; + + /// Maximum number of objects to keep in the freelist. + const MAX_FREELIST: usize = 0; + + /// Try to push a dead object onto this type's freelist for reuse. + /// Returns true if the object was stored (caller must NOT free the memory). + /// + /// # Safety + /// `obj` must be a valid pointer to a `PyInner` with refcount 0, + /// after `drop_slow_inner` and `tp_clear` have already run. + #[inline] + unsafe fn freelist_push(_obj: *mut PyObject) -> bool { + false + } + + /// Try to pop a pre-allocated object from this type's freelist. + /// The returned pointer still has the old payload; the caller must + /// reinitialize `ref_count`, `gc_bits`, and `payload`. + /// + /// # Safety + /// The returned pointer (if Some) must point to a valid `PyInner` + /// whose payload is still initialized from a previous allocation. The caller + /// will drop and overwrite `payload` before reuse. + #[inline] + unsafe fn freelist_pop() -> Option> { + None + } + #[inline] fn into_pyobject(self, vm: &VirtualMachine) -> PyObjectRef where