From 9c4ab5f1ea637893dbca821a9c76a7854b0f6571 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Fri, 6 Mar 2026 01:11:46 +0900 Subject: [PATCH 1/4] =?UTF-8?q?Add=20per-size=20tuple=20freelist=20(20=20b?= =?UTF-8?q?uckets=20=C3=97=202000=20each)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implement PyTuple freelist matching tuples[PyTuple_MAXSAVESIZE]: - TupleFreeList with 20 per-size buckets (sizes 1..=20, 2000 capacity each) - freelist_push uses pre-clear size hint for correct bucket selection - freelist_pop takes &Self payload to select bucket by size - Type guard in new_ref handles structseq types sharing PyTuple vtable - Add pyinner_layout() helper for custom freelist Drop impls - Update freelist_pop/push signatures across all freelist types --- .cspell.dict/cpython.txt | 1 + crates/vm/src/builtins/complex.rs | 4 +- crates/vm/src/builtins/dict.rs | 4 +- crates/vm/src/builtins/float.rs | 4 +- crates/vm/src/builtins/int.rs | 4 +- crates/vm/src/builtins/list.rs | 4 +- crates/vm/src/builtins/range.rs | 4 +- crates/vm/src/builtins/slice.rs | 4 +- crates/vm/src/builtins/tuple.rs | 99 ++++++++++++++++++++++++++++++- crates/vm/src/object/core.rs | 54 ++++++++++++----- crates/vm/src/object/payload.rs | 17 +++++- 11 files changed, 165 insertions(+), 34 deletions(-) diff --git a/.cspell.dict/cpython.txt b/.cspell.dict/cpython.txt index 684c7a5b614..a9fbc8f4318 100644 --- a/.cspell.dict/cpython.txt +++ b/.cspell.dict/cpython.txt @@ -154,6 +154,7 @@ prec preinitialized pybuilddir pycore +pyinner pydecimal Pyfunc pylifecycle diff --git a/crates/vm/src/builtins/complex.rs b/crates/vm/src/builtins/complex.rs index c36024dcce3..b818e76aa33 100644 --- a/crates/vm/src/builtins/complex.rs +++ b/crates/vm/src/builtins/complex.rs @@ -41,7 +41,7 @@ impl PyPayload for PyComplex { } #[inline] - unsafe fn freelist_push(obj: *mut PyObject) -> bool { + unsafe fn freelist_push(obj: *mut PyObject, _hint: usize, _typ: &Py) -> bool { COMPLEX_FREELIST .try_with(|fl| { let mut list = fl.take(); @@ -58,7 +58,7 @@ impl PyPayload for PyComplex { } #[inline] - unsafe fn freelist_pop() -> Option> { + unsafe fn freelist_pop(_payload: &Self) -> Option> { COMPLEX_FREELIST .try_with(|fl| { let mut list = fl.take(); diff --git a/crates/vm/src/builtins/dict.rs b/crates/vm/src/builtins/dict.rs index f2a7e6a5a29..824c8218e72 100644 --- a/crates/vm/src/builtins/dict.rs +++ b/crates/vm/src/builtins/dict.rs @@ -76,7 +76,7 @@ impl PyPayload for PyDict { } #[inline] - unsafe fn freelist_push(obj: *mut PyObject) -> bool { + unsafe fn freelist_push(obj: *mut PyObject, _hint: usize, _typ: &Py) -> bool { DICT_FREELIST .try_with(|fl| { let mut list = fl.take(); @@ -93,7 +93,7 @@ impl PyPayload for PyDict { } #[inline] - unsafe fn freelist_pop() -> Option> { + unsafe fn freelist_pop(_payload: &Self) -> Option> { DICT_FREELIST .try_with(|fl| { let mut list = fl.take(); diff --git a/crates/vm/src/builtins/float.rs b/crates/vm/src/builtins/float.rs index 4eacfaf45b3..9b8aa4930c3 100644 --- a/crates/vm/src/builtins/float.rs +++ b/crates/vm/src/builtins/float.rs @@ -48,7 +48,7 @@ impl PyPayload for PyFloat { } #[inline] - unsafe fn freelist_push(obj: *mut PyObject) -> bool { + unsafe fn freelist_push(obj: *mut PyObject, _hint: usize, _typ: &Py) -> bool { FLOAT_FREELIST .try_with(|fl| { let mut list = fl.take(); @@ -65,7 +65,7 @@ impl PyPayload for PyFloat { } #[inline] - unsafe fn freelist_pop() -> Option> { + unsafe fn freelist_pop(_payload: &Self) -> Option> { FLOAT_FREELIST .try_with(|fl| { let mut list = fl.take(); diff --git a/crates/vm/src/builtins/int.rs b/crates/vm/src/builtins/int.rs index d5bb3cbecd7..a213a82ed9b 100644 --- a/crates/vm/src/builtins/int.rs +++ b/crates/vm/src/builtins/int.rs @@ -69,7 +69,7 @@ impl PyPayload for PyInt { } #[inline] - unsafe fn freelist_push(obj: *mut PyObject) -> bool { + unsafe fn freelist_push(obj: *mut PyObject, _hint: usize, _typ: &Py) -> bool { INT_FREELIST .try_with(|fl| { let mut list = fl.take(); @@ -86,7 +86,7 @@ impl PyPayload for PyInt { } #[inline] - unsafe fn freelist_pop() -> Option> { + unsafe fn freelist_pop(_payload: &Self) -> Option> { INT_FREELIST .try_with(|fl| { let mut list = fl.take(); diff --git a/crates/vm/src/builtins/list.rs b/crates/vm/src/builtins/list.rs index 34c40bba209..6cc455315a8 100644 --- a/crates/vm/src/builtins/list.rs +++ b/crates/vm/src/builtins/list.rs @@ -88,7 +88,7 @@ impl PyPayload for PyList { } #[inline] - unsafe fn freelist_push(obj: *mut PyObject) -> bool { + unsafe fn freelist_push(obj: *mut PyObject, _hint: usize, _typ: &Py) -> bool { LIST_FREELIST .try_with(|fl| { let mut list = fl.take(); @@ -105,7 +105,7 @@ impl PyPayload for PyList { } #[inline] - unsafe fn freelist_pop() -> Option> { + unsafe fn freelist_pop(_payload: &Self) -> Option> { LIST_FREELIST .try_with(|fl| { let mut list = fl.take(); diff --git a/crates/vm/src/builtins/range.rs b/crates/vm/src/builtins/range.rs index 795ec230ba9..ac236e175ef 100644 --- a/crates/vm/src/builtins/range.rs +++ b/crates/vm/src/builtins/range.rs @@ -84,7 +84,7 @@ impl PyPayload for PyRange { } #[inline] - unsafe fn freelist_push(obj: *mut PyObject) -> bool { + unsafe fn freelist_push(obj: *mut PyObject, _hint: usize, _typ: &Py) -> bool { RANGE_FREELIST .try_with(|fl| { let mut list = fl.take(); @@ -101,7 +101,7 @@ impl PyPayload for PyRange { } #[inline] - unsafe fn freelist_pop() -> Option> { + unsafe fn freelist_pop(_payload: &Self) -> Option> { RANGE_FREELIST .try_with(|fl| { let mut list = fl.take(); diff --git a/crates/vm/src/builtins/slice.rs b/crates/vm/src/builtins/slice.rs index aeb3337c7d8..c25cc152bfc 100644 --- a/crates/vm/src/builtins/slice.rs +++ b/crates/vm/src/builtins/slice.rs @@ -59,7 +59,7 @@ impl PyPayload for PySlice { } #[inline] - unsafe fn freelist_push(obj: *mut PyObject) -> bool { + unsafe fn freelist_push(obj: *mut PyObject, _hint: usize, _typ: &Py) -> bool { SLICE_FREELIST .try_with(|fl| { let mut list = fl.take(); @@ -76,7 +76,7 @@ impl PyPayload for PySlice { } #[inline] - unsafe fn freelist_pop() -> Option> { + unsafe fn freelist_pop(_payload: &Self) -> Option> { SLICE_FREELIST .try_with(|fl| { let mut list = fl.take(); diff --git a/crates/vm/src/builtins/tuple.rs b/crates/vm/src/builtins/tuple.rs index 03f88f1b5fe..6521f9fa08d 100644 --- a/crates/vm/src/builtins/tuple.rs +++ b/crates/vm/src/builtins/tuple.rs @@ -27,6 +27,8 @@ use crate::{ vm::VirtualMachine, }; use alloc::fmt; +use core::cell::Cell; +use core::ptr::NonNull; #[pyclass(module = false, name = "tuple", traverse = "manual")] pub struct PyTuple { @@ -53,14 +55,105 @@ 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. +// spell-checker:ignore MAXFREELIST MAXSAVESIZE +/// Per-size freelist storage for tuples, matching tuples[PyTuple_MAXSAVESIZE]. +/// Each bucket caches tuples of a specific element count (index = len - 1). +struct TupleFreeList { + buckets: [Vec>; Self::MAXSAVESIZE], +} + +impl TupleFreeList { + /// Largest tuple size to cache on the freelist (sizes 1..=20). + const MAXSAVESIZE: usize = 20; + const fn new() -> Self { + Self { + buckets: [const { Vec::new() }; Self::MAXSAVESIZE], + } + } +} + +impl Default for TupleFreeList { + fn default() -> Self { + Self::new() + } +} + +impl Drop for TupleFreeList { + fn drop(&mut self) { + // Same safety pattern as FreeList::drop — free raw allocation + // without running payload destructors to avoid TLS-after-destruction panics. + let layout = crate::object::pyinner_layout::(); + for bucket in &mut self.buckets { + for ptr in bucket.drain(..) { + unsafe { + alloc::alloc::dealloc(ptr.as_ptr() as *mut u8, layout); + } + } + } + } +} + +thread_local! { + static TUPLE_FREELIST: Cell = const { Cell::new(TupleFreeList::new()) }; +} + impl PyPayload for PyTuple { + const MAX_FREELIST: usize = 2000; + const HAS_FREELIST: bool = true; + #[inline] fn class(ctx: &Context) -> &'static Py { ctx.types.tuple_type } + + #[inline] + unsafe fn freelist_hint(obj: *mut PyObject) -> usize { + let py_tuple = unsafe { &*(obj as *const crate::Py) }; + py_tuple.elements.len() + } + + #[inline] + unsafe fn freelist_push(obj: *mut PyObject, hint: usize, typ: &Py) -> bool { + // Py_IS_TYPE(op, &PyTuple_Type): reject structseq and other subtypes + if !core::ptr::eq(typ, Self::class(crate::vm::Context::genesis())) { + return false; + } + let len = hint; + if len == 0 || len > TupleFreeList::MAXSAVESIZE { + return false; + } + TUPLE_FREELIST + .try_with(|fl| { + let mut list = fl.take(); + let bucket = &mut list.buckets[len - 1]; + let stored = if bucket.len() < Self::MAX_FREELIST { + bucket.push(unsafe { NonNull::new_unchecked(obj) }); + true + } else { + false + }; + fl.set(list); + stored + }) + .unwrap_or(false) + } + + #[inline] + unsafe fn freelist_pop(payload: &Self) -> Option> { + let len = payload.elements.len(); + if len == 0 || len > TupleFreeList::MAXSAVESIZE { + return None; + } + TUPLE_FREELIST + .try_with(|fl| { + let mut list = fl.take(); + let result = list.buckets[len - 1].pop(); + fl.set(list); + result + }) + .ok() + .flatten() + } } pub trait IntoPyTuple { diff --git a/crates/vm/src/object/core.rs b/crates/vm/src/object/core.rs index e31470fd12e..dbe4840dab5 100644 --- a/crates/vm/src/object/core.rs +++ b/crates/vm/src/object/core.rs @@ -188,6 +188,15 @@ pub(super) unsafe fn default_dealloc(obj: *mut PyObject) { ); } + // Capture freelist bucket hint before tp_clear empties the payload. + // Size-based freelists (e.g. PyTuple) need the element count for bucket selection, + // but clear() replaces elements with an empty slice. + let freelist_hint = if T::HAS_FREELIST { + unsafe { T::freelist_hint(obj) } + } else { + 0 + }; + // Extract child references before deallocation to break circular refs (tp_clear) let mut edges = Vec::new(); if let Some(clear_fn) = vtable.clear { @@ -195,10 +204,12 @@ pub(super) unsafe fn default_dealloc(obj: *mut PyObject) { } // 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) } + // Only exact types (not heaptype subclasses) go into the freelist. + // The runtime type is passed to freelist_push for Py_IS_TYPE-style + // exact type checking (e.g. reject structseq subtypes of PyTuple). + let typ = obj_ref.class(); + let pushed = if T::HAS_FREELIST && typ.heaptype_ext.is_none() { + unsafe { T::freelist_push(obj, freelist_hint, typ) } } else { false }; @@ -1094,6 +1105,11 @@ impl PyInner { } } +/// Returns the allocation layout for `PyInner`, for use in freelist Drop impls. +pub(crate) const fn pyinner_layout() -> core::alloc::Layout { + core::alloc::Layout::new::>() +} + /// Thread-local freelist storage for reusing object allocations. /// /// Wraps a `Vec<*mut PyObject>`. On thread teardown, `Drop` frees raw @@ -2177,24 +2193,32 @@ impl PyRef { // Try to reuse from freelist (exact type only, no dict, no heaptype) let cached = if !has_dict && !is_heaptype { - unsafe { T::freelist_pop() } + unsafe { T::freelist_pop(&payload) } } 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 + // Push-side exact type check filters subtypes, but subtypes + // sharing the same Rust payload (e.g. structseq) may still + // call freelist_pop. Verify typ matches; if not, deallocate. + let cached_typ = unsafe { (*inner).typ.load_raw() }; + if !core::ptr::eq(cached_typ, &*typ as *const Py) { + unsafe { PyInner::dealloc(inner) }; + let inner = PyInner::new(payload, typ, dict); + unsafe { NonNull::new_unchecked(inner.cast::>()) } + } else { + 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); + } + // Drop the caller's typ since the cached object already holds one + drop(typ); + unsafe { NonNull::new_unchecked(inner.cast::>()) } } - // Drop the caller's typ since the cached object already holds one - drop(typ); - unsafe { NonNull::new_unchecked(inner.cast::>()) } } else { let inner = PyInner::new(payload, typ, dict); unsafe { NonNull::new_unchecked(inner.cast::>()) } diff --git a/crates/vm/src/object/payload.rs b/crates/vm/src/object/payload.rs index 1af954505f7..eb2e8d9b27d 100644 --- a/crates/vm/src/object/payload.rs +++ b/crates/vm/src/object/payload.rs @@ -55,14 +55,27 @@ pub trait PyPayload: MaybeTraverse + PyThreadingConstraint + Sized + 'static { /// Maximum number of objects to keep in the freelist. const MAX_FREELIST: usize = 0; + /// Capture a hint value from the payload before tp_clear runs. + /// Used by size-based freelists (e.g. PyTuple) to remember the element + /// count before clear empties the payload. + /// + /// # Safety + /// `obj` must be a valid pointer to a `PyInner` with the payload still intact. + #[inline] + unsafe fn freelist_hint(_obj: *mut PyObject) -> 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). + /// `hint` is the value returned by `freelist_hint` before tp_clear. + /// `typ` is the runtime type of the object, for exact-type filtering. /// /// # 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 { + unsafe fn freelist_push(_obj: *mut PyObject, _hint: usize, _typ: &Py) -> bool { false } @@ -75,7 +88,7 @@ pub trait PyPayload: MaybeTraverse + PyThreadingConstraint + Sized + 'static { /// whose payload is still initialized from a previous allocation. The caller /// will drop and overwrite `payload` before reuse. #[inline] - unsafe fn freelist_pop() -> Option> { + unsafe fn freelist_pop(_payload: &Self) -> Option> { None } From 62bdfc13c941c6eddba39306bda028f7de5677ae Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sat, 7 Mar 2026 00:27:16 +0900 Subject: [PATCH 2/4] freelist: exact type check at pop call-site Move exact-type filtering from freelist_pop implementations to the single call-site in new_ref. This prevents structseq and other subtypes from popping tuple freelist entries entirely, rather than popping and then deallocating on type mismatch. Add Context::try_genesis() that returns None during bootstrap to avoid deadlock when genesis() is called during Context initialization. --- crates/vm/src/builtins/tuple.rs | 12 ++++++------ crates/vm/src/object/core.rs | 33 +++++++++++++++------------------ 2 files changed, 21 insertions(+), 24 deletions(-) diff --git a/crates/vm/src/builtins/tuple.rs b/crates/vm/src/builtins/tuple.rs index 6521f9fa08d..58ac0979e44 100644 --- a/crates/vm/src/builtins/tuple.rs +++ b/crates/vm/src/builtins/tuple.rs @@ -55,19 +55,19 @@ unsafe impl Traverse for PyTuple { } } -// spell-checker:ignore MAXFREELIST MAXSAVESIZE +// spell-checker:ignore MAXSAVESIZE /// Per-size freelist storage for tuples, matching tuples[PyTuple_MAXSAVESIZE]. /// Each bucket caches tuples of a specific element count (index = len - 1). struct TupleFreeList { - buckets: [Vec>; Self::MAXSAVESIZE], + buckets: [Vec>; Self::MAX_SAVE_SIZE], } impl TupleFreeList { /// Largest tuple size to cache on the freelist (sizes 1..=20). - const MAXSAVESIZE: usize = 20; + const MAX_SAVE_SIZE: usize = 20; const fn new() -> Self { Self { - buckets: [const { Vec::new() }; Self::MAXSAVESIZE], + buckets: [const { Vec::new() }; Self::MAX_SAVE_SIZE], } } } @@ -119,7 +119,7 @@ impl PyPayload for PyTuple { return false; } let len = hint; - if len == 0 || len > TupleFreeList::MAXSAVESIZE { + if len == 0 || len > TupleFreeList::MAX_SAVE_SIZE { return false; } TUPLE_FREELIST @@ -141,7 +141,7 @@ impl PyPayload for PyTuple { #[inline] unsafe fn freelist_pop(payload: &Self) -> Option> { let len = payload.elements.len(); - if len == 0 || len > TupleFreeList::MAXSAVESIZE { + if len == 0 || len > TupleFreeList::MAX_SAVE_SIZE { return None; } TUPLE_FREELIST diff --git a/crates/vm/src/object/core.rs b/crates/vm/src/object/core.rs index dbe4840dab5..fc301907afd 100644 --- a/crates/vm/src/object/core.rs +++ b/crates/vm/src/object/core.rs @@ -2191,7 +2191,7 @@ impl PyRef { let has_dict = dict.is_some(); let is_heaptype = typ.heaptype_ext.is_some(); - // Try to reuse from freelist (exact type only, no dict, no heaptype) + // Try to reuse from freelist (no dict, no heaptype) let cached = if !has_dict && !is_heaptype { unsafe { T::freelist_pop(&payload) } } else { @@ -2200,25 +2200,22 @@ impl PyRef { let ptr = if let Some(cached) = cached { let inner = cached.as_ptr() as *mut PyInner; - // Push-side exact type check filters subtypes, but subtypes - // sharing the same Rust payload (e.g. structseq) may still - // call freelist_pop. Verify typ matches; if not, deallocate. - let cached_typ = unsafe { (*inner).typ.load_raw() }; - if !core::ptr::eq(cached_typ, &*typ as *const Py) { - unsafe { PyInner::dealloc(inner) }; - let inner = PyInner::new(payload, typ, dict); - unsafe { NonNull::new_unchecked(inner.cast::>()) } - } else { - 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); + 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); + // Freelist only stores exact base types (push-side filter), + // but subtypes sharing the same Rust payload (e.g. structseq) + // may pop entries. Update typ if it differs. + let cached_typ: *const Py = &*(*inner).typ; + if core::ptr::eq(cached_typ, &*typ) { + drop(typ); + } else { + let _old = (*inner).typ.swap(typ); } - // Drop the caller's typ since the cached object already holds one - drop(typ); - unsafe { NonNull::new_unchecked(inner.cast::>()) } } + unsafe { NonNull::new_unchecked(inner.cast::>()) } } else { let inner = PyInner::new(payload, typ, dict); unsafe { NonNull::new_unchecked(inner.cast::>()) } From aad6d6d34f78e0895a7e7acd340146e5f32f54b8 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sun, 8 Mar 2026 23:15:59 +0900 Subject: [PATCH 3/4] Move exact type check from freelist_push to call-site in default_dealloc Remove typ parameter from freelist_push trait signature. The exact type check is now done once at the call-site alongside the heaptype check, simplifying all freelist_push implementations. --- crates/vm/src/builtins/complex.rs | 2 +- crates/vm/src/builtins/dict.rs | 2 +- crates/vm/src/builtins/float.rs | 2 +- crates/vm/src/builtins/int.rs | 2 +- crates/vm/src/builtins/list.rs | 2 +- crates/vm/src/builtins/range.rs | 2 +- crates/vm/src/builtins/slice.rs | 2 +- crates/vm/src/builtins/tuple.rs | 6 +----- crates/vm/src/object/core.rs | 11 ++++++----- crates/vm/src/object/payload.rs | 3 +-- 10 files changed, 15 insertions(+), 19 deletions(-) diff --git a/crates/vm/src/builtins/complex.rs b/crates/vm/src/builtins/complex.rs index b818e76aa33..f8667ba0c51 100644 --- a/crates/vm/src/builtins/complex.rs +++ b/crates/vm/src/builtins/complex.rs @@ -41,7 +41,7 @@ impl PyPayload for PyComplex { } #[inline] - unsafe fn freelist_push(obj: *mut PyObject, _hint: usize, _typ: &Py) -> bool { + unsafe fn freelist_push(obj: *mut PyObject, _hint: usize) -> bool { COMPLEX_FREELIST .try_with(|fl| { let mut list = fl.take(); diff --git a/crates/vm/src/builtins/dict.rs b/crates/vm/src/builtins/dict.rs index 824c8218e72..54c524c2b41 100644 --- a/crates/vm/src/builtins/dict.rs +++ b/crates/vm/src/builtins/dict.rs @@ -76,7 +76,7 @@ impl PyPayload for PyDict { } #[inline] - unsafe fn freelist_push(obj: *mut PyObject, _hint: usize, _typ: &Py) -> bool { + unsafe fn freelist_push(obj: *mut PyObject, _hint: usize) -> bool { DICT_FREELIST .try_with(|fl| { let mut list = fl.take(); diff --git a/crates/vm/src/builtins/float.rs b/crates/vm/src/builtins/float.rs index 9b8aa4930c3..a4e8c0b3d6b 100644 --- a/crates/vm/src/builtins/float.rs +++ b/crates/vm/src/builtins/float.rs @@ -48,7 +48,7 @@ impl PyPayload for PyFloat { } #[inline] - unsafe fn freelist_push(obj: *mut PyObject, _hint: usize, _typ: &Py) -> bool { + unsafe fn freelist_push(obj: *mut PyObject, _hint: usize) -> bool { FLOAT_FREELIST .try_with(|fl| { let mut list = fl.take(); diff --git a/crates/vm/src/builtins/int.rs b/crates/vm/src/builtins/int.rs index a213a82ed9b..64badecd1ac 100644 --- a/crates/vm/src/builtins/int.rs +++ b/crates/vm/src/builtins/int.rs @@ -69,7 +69,7 @@ impl PyPayload for PyInt { } #[inline] - unsafe fn freelist_push(obj: *mut PyObject, _hint: usize, _typ: &Py) -> bool { + unsafe fn freelist_push(obj: *mut PyObject, _hint: usize) -> bool { INT_FREELIST .try_with(|fl| { let mut list = fl.take(); diff --git a/crates/vm/src/builtins/list.rs b/crates/vm/src/builtins/list.rs index 6cc455315a8..9f85394729c 100644 --- a/crates/vm/src/builtins/list.rs +++ b/crates/vm/src/builtins/list.rs @@ -88,7 +88,7 @@ impl PyPayload for PyList { } #[inline] - unsafe fn freelist_push(obj: *mut PyObject, _hint: usize, _typ: &Py) -> bool { + unsafe fn freelist_push(obj: *mut PyObject, _hint: usize) -> bool { LIST_FREELIST .try_with(|fl| { let mut list = fl.take(); diff --git a/crates/vm/src/builtins/range.rs b/crates/vm/src/builtins/range.rs index ac236e175ef..21120d8b7ce 100644 --- a/crates/vm/src/builtins/range.rs +++ b/crates/vm/src/builtins/range.rs @@ -84,7 +84,7 @@ impl PyPayload for PyRange { } #[inline] - unsafe fn freelist_push(obj: *mut PyObject, _hint: usize, _typ: &Py) -> bool { + unsafe fn freelist_push(obj: *mut PyObject, _hint: usize) -> bool { RANGE_FREELIST .try_with(|fl| { let mut list = fl.take(); diff --git a/crates/vm/src/builtins/slice.rs b/crates/vm/src/builtins/slice.rs index c25cc152bfc..571500d3254 100644 --- a/crates/vm/src/builtins/slice.rs +++ b/crates/vm/src/builtins/slice.rs @@ -59,7 +59,7 @@ impl PyPayload for PySlice { } #[inline] - unsafe fn freelist_push(obj: *mut PyObject, _hint: usize, _typ: &Py) -> bool { + unsafe fn freelist_push(obj: *mut PyObject, _hint: usize) -> bool { SLICE_FREELIST .try_with(|fl| { let mut list = fl.take(); diff --git a/crates/vm/src/builtins/tuple.rs b/crates/vm/src/builtins/tuple.rs index 58ac0979e44..f980b36ecdb 100644 --- a/crates/vm/src/builtins/tuple.rs +++ b/crates/vm/src/builtins/tuple.rs @@ -113,11 +113,7 @@ impl PyPayload for PyTuple { } #[inline] - unsafe fn freelist_push(obj: *mut PyObject, hint: usize, typ: &Py) -> bool { - // Py_IS_TYPE(op, &PyTuple_Type): reject structseq and other subtypes - if !core::ptr::eq(typ, Self::class(crate::vm::Context::genesis())) { - return false; - } + unsafe fn freelist_push(obj: *mut PyObject, hint: usize) -> bool { let len = hint; if len == 0 || len > TupleFreeList::MAX_SAVE_SIZE { return false; diff --git a/crates/vm/src/object/core.rs b/crates/vm/src/object/core.rs index fc301907afd..ffa038848ba 100644 --- a/crates/vm/src/object/core.rs +++ b/crates/vm/src/object/core.rs @@ -204,12 +204,13 @@ pub(super) unsafe fn default_dealloc(obj: *mut PyObject) { } // Try to store in freelist for reuse; otherwise deallocate. - // Only exact types (not heaptype subclasses) go into the freelist. - // The runtime type is passed to freelist_push for Py_IS_TYPE-style - // exact type checking (e.g. reject structseq subtypes of PyTuple). + // Only exact base types (not heaptype or structseq subtypes) go into the freelist. let typ = obj_ref.class(); - let pushed = if T::HAS_FREELIST && typ.heaptype_ext.is_none() { - unsafe { T::freelist_push(obj, freelist_hint, typ) } + let pushed = if T::HAS_FREELIST + && typ.heaptype_ext.is_none() + && core::ptr::eq(typ, T::class(crate::vm::Context::genesis())) + { + unsafe { T::freelist_push(obj, freelist_hint) } } else { false }; diff --git a/crates/vm/src/object/payload.rs b/crates/vm/src/object/payload.rs index eb2e8d9b27d..820e47fe94f 100644 --- a/crates/vm/src/object/payload.rs +++ b/crates/vm/src/object/payload.rs @@ -69,13 +69,12 @@ pub trait PyPayload: MaybeTraverse + PyThreadingConstraint + Sized + 'static { /// 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). /// `hint` is the value returned by `freelist_hint` before tp_clear. - /// `typ` is the runtime type of the object, for exact-type filtering. /// /// # 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, _hint: usize, _typ: &Py) -> bool { + unsafe fn freelist_push(_obj: *mut PyObject, _hint: usize) -> bool { false } From c32f31bbd6ca96eb01a060948c8233ce7bc2a456 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Mon, 9 Mar 2026 00:20:23 +0900 Subject: [PATCH 4/4] Remove freelist_hint; call freelist_push before tp_clear By calling freelist_push before tp_clear, the payload is still intact and can be read directly (e.g. tuple element count for bucket selection). This eliminates freelist_hint and the hint parameter entirely. --- crates/vm/src/builtins/complex.rs | 2 +- crates/vm/src/builtins/dict.rs | 2 +- crates/vm/src/builtins/float.rs | 2 +- crates/vm/src/builtins/int.rs | 2 +- crates/vm/src/builtins/list.rs | 2 +- crates/vm/src/builtins/range.rs | 2 +- crates/vm/src/builtins/slice.rs | 2 +- crates/vm/src/builtins/tuple.rs | 10 ++-------- crates/vm/src/object/core.rs | 29 +++++++++++------------------ crates/vm/src/object/payload.rs | 19 ++++--------------- 10 files changed, 24 insertions(+), 48 deletions(-) diff --git a/crates/vm/src/builtins/complex.rs b/crates/vm/src/builtins/complex.rs index f8667ba0c51..182962a4b2e 100644 --- a/crates/vm/src/builtins/complex.rs +++ b/crates/vm/src/builtins/complex.rs @@ -41,7 +41,7 @@ impl PyPayload for PyComplex { } #[inline] - unsafe fn freelist_push(obj: *mut PyObject, _hint: usize) -> bool { + unsafe fn freelist_push(obj: *mut PyObject) -> bool { COMPLEX_FREELIST .try_with(|fl| { let mut list = fl.take(); diff --git a/crates/vm/src/builtins/dict.rs b/crates/vm/src/builtins/dict.rs index 54c524c2b41..0e64e9e66ac 100644 --- a/crates/vm/src/builtins/dict.rs +++ b/crates/vm/src/builtins/dict.rs @@ -76,7 +76,7 @@ impl PyPayload for PyDict { } #[inline] - unsafe fn freelist_push(obj: *mut PyObject, _hint: usize) -> bool { + unsafe fn freelist_push(obj: *mut PyObject) -> bool { DICT_FREELIST .try_with(|fl| { let mut list = fl.take(); diff --git a/crates/vm/src/builtins/float.rs b/crates/vm/src/builtins/float.rs index a4e8c0b3d6b..eeddd6b2eb9 100644 --- a/crates/vm/src/builtins/float.rs +++ b/crates/vm/src/builtins/float.rs @@ -48,7 +48,7 @@ impl PyPayload for PyFloat { } #[inline] - unsafe fn freelist_push(obj: *mut PyObject, _hint: usize) -> bool { + unsafe fn freelist_push(obj: *mut PyObject) -> bool { FLOAT_FREELIST .try_with(|fl| { let mut list = fl.take(); diff --git a/crates/vm/src/builtins/int.rs b/crates/vm/src/builtins/int.rs index 64badecd1ac..a253506eba1 100644 --- a/crates/vm/src/builtins/int.rs +++ b/crates/vm/src/builtins/int.rs @@ -69,7 +69,7 @@ impl PyPayload for PyInt { } #[inline] - unsafe fn freelist_push(obj: *mut PyObject, _hint: usize) -> bool { + unsafe fn freelist_push(obj: *mut PyObject) -> bool { INT_FREELIST .try_with(|fl| { let mut list = fl.take(); diff --git a/crates/vm/src/builtins/list.rs b/crates/vm/src/builtins/list.rs index 9f85394729c..cdb8a73ead2 100644 --- a/crates/vm/src/builtins/list.rs +++ b/crates/vm/src/builtins/list.rs @@ -88,7 +88,7 @@ impl PyPayload for PyList { } #[inline] - unsafe fn freelist_push(obj: *mut PyObject, _hint: usize) -> bool { + unsafe fn freelist_push(obj: *mut PyObject) -> bool { LIST_FREELIST .try_with(|fl| { let mut list = fl.take(); diff --git a/crates/vm/src/builtins/range.rs b/crates/vm/src/builtins/range.rs index 21120d8b7ce..153a82bb43b 100644 --- a/crates/vm/src/builtins/range.rs +++ b/crates/vm/src/builtins/range.rs @@ -84,7 +84,7 @@ impl PyPayload for PyRange { } #[inline] - unsafe fn freelist_push(obj: *mut PyObject, _hint: usize) -> bool { + unsafe fn freelist_push(obj: *mut PyObject) -> bool { RANGE_FREELIST .try_with(|fl| { let mut list = fl.take(); diff --git a/crates/vm/src/builtins/slice.rs b/crates/vm/src/builtins/slice.rs index 571500d3254..b46f7a3a56a 100644 --- a/crates/vm/src/builtins/slice.rs +++ b/crates/vm/src/builtins/slice.rs @@ -59,7 +59,7 @@ impl PyPayload for PySlice { } #[inline] - unsafe fn freelist_push(obj: *mut PyObject, _hint: usize) -> bool { + unsafe fn freelist_push(obj: *mut PyObject) -> bool { SLICE_FREELIST .try_with(|fl| { let mut list = fl.take(); diff --git a/crates/vm/src/builtins/tuple.rs b/crates/vm/src/builtins/tuple.rs index f980b36ecdb..e1dc1ef306b 100644 --- a/crates/vm/src/builtins/tuple.rs +++ b/crates/vm/src/builtins/tuple.rs @@ -107,14 +107,8 @@ impl PyPayload for PyTuple { } #[inline] - unsafe fn freelist_hint(obj: *mut PyObject) -> usize { - let py_tuple = unsafe { &*(obj as *const crate::Py) }; - py_tuple.elements.len() - } - - #[inline] - unsafe fn freelist_push(obj: *mut PyObject, hint: usize) -> bool { - let len = hint; + unsafe fn freelist_push(obj: *mut PyObject) -> bool { + let len = unsafe { &*(obj as *const crate::Py) }.elements.len(); if len == 0 || len > TupleFreeList::MAX_SAVE_SIZE { return false; } diff --git a/crates/vm/src/object/core.rs b/crates/vm/src/object/core.rs index ffa038848ba..feaf0508686 100644 --- a/crates/vm/src/object/core.rs +++ b/crates/vm/src/object/core.rs @@ -188,39 +188,32 @@ pub(super) unsafe fn default_dealloc(obj: *mut PyObject) { ); } - // Capture freelist bucket hint before tp_clear empties the payload. - // Size-based freelists (e.g. PyTuple) need the element count for bucket selection, - // but clear() replaces elements with an empty slice. - let freelist_hint = if T::HAS_FREELIST { - unsafe { T::freelist_hint(obj) } - } else { - 0 - }; - - // Extract child references before deallocation to break circular refs (tp_clear) - let mut edges = Vec::new(); - if let Some(clear_fn) = vtable.clear { - unsafe { clear_fn(obj, &mut edges) }; - } - - // Try to store in freelist for reuse; otherwise deallocate. + // Try to store in freelist for reuse BEFORE tp_clear, so that + // size-based freelists (e.g. PyTuple) can read the payload directly. // Only exact base types (not heaptype or structseq subtypes) go into the freelist. let typ = obj_ref.class(); let pushed = if T::HAS_FREELIST && typ.heaptype_ext.is_none() && core::ptr::eq(typ, T::class(crate::vm::Context::genesis())) { - unsafe { T::freelist_push(obj, freelist_hint) } + unsafe { T::freelist_push(obj) } } else { false }; + + // Extract child references to break circular refs (tp_clear). + // This runs regardless of freelist push — the object's children must be released. + let mut edges = Vec::new(); + if let Some(clear_fn) = vtable.clear { + unsafe { clear_fn(obj, &mut edges) }; + } + if !pushed { // Deallocate the object memory (handles ObjExt prefix if present) unsafe { PyInner::dealloc(obj as *mut PyInner) }; } // Drop child references - may trigger recursive destruction. - // The object is already deallocated, so circular refs are broken. drop(edges); // Trashcan: decrement depth and process deferred objects at outermost level diff --git a/crates/vm/src/object/payload.rs b/crates/vm/src/object/payload.rs index 820e47fe94f..a615123c680 100644 --- a/crates/vm/src/object/payload.rs +++ b/crates/vm/src/object/payload.rs @@ -55,26 +55,15 @@ pub trait PyPayload: MaybeTraverse + PyThreadingConstraint + Sized + 'static { /// Maximum number of objects to keep in the freelist. const MAX_FREELIST: usize = 0; - /// Capture a hint value from the payload before tp_clear runs. - /// Used by size-based freelists (e.g. PyTuple) to remember the element - /// count before clear empties the payload. - /// - /// # Safety - /// `obj` must be a valid pointer to a `PyInner` with the payload still intact. - #[inline] - unsafe fn freelist_hint(_obj: *mut PyObject) -> 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). - /// `hint` is the value returned by `freelist_hint` before tp_clear. + /// Called before tp_clear, so the payload is still intact. /// /// # Safety - /// `obj` must be a valid pointer to a `PyInner` with refcount 0, - /// after `drop_slow_inner` and `tp_clear` have already run. + /// `obj` must be a valid pointer to a `PyInner` with refcount 0. + /// The payload is still initialized and can be read for bucket selection. #[inline] - unsafe fn freelist_push(_obj: *mut PyObject, _hint: usize) -> bool { + unsafe fn freelist_push(_obj: *mut PyObject) -> bool { false }