Remove Frame mutex and use DataStack bump allocator for LocalsPlus (#7333)

* Remove PyMutex<FrameState> from Frame, use UnsafeCell fields directly

Move stack, cells_frees, prev_line out of the mutex-protected FrameState
into Frame as FrameUnsafeCell fields. This eliminates mutex lock/unlock
overhead on every frame execution (with_exec).

Safety relies on the same single-threaded execution guarantee that
FastLocals already uses.

* Add thread-local DataStack for bump-allocating frame data

Introduce DataStack with linked chunks (16KB initial, doubling) and
push/pop bump allocation. Add datastack field to VirtualMachine.
Not yet wired to frame creation.

* Unify FastLocals and BoxVec stack into LocalsPlus

Replace separate FastLocals (Box<[Option<PyObjectRef>]>) and
BoxVec<Option<PyStackRef>> with a single LocalsPlus struct that
stores both in a contiguous Box<[usize]> array. The first
nlocalsplus slots are fastlocals and the rest is the evaluation
stack. Typed access is provided through transmute-based methods.

Remove BoxVec import from frame.rs.

* Use DataStack for LocalsPlus in non-generator function calls

Normal function calls now bump-allocate LocalsPlus data from the
per-thread DataStack instead of a separate heap allocation.
Generator/coroutine frames continue using heap allocation since
they outlive the call.

On frame exit, data is copied to the heap (materialize_to_heap)
to preserve locals for tracebacks, then the DataStack is popped.

VirtualMachine.datastack is wrapped in UnsafeCell for interior
mutability (safe because frame allocation is single-threaded LIFO).

* Fix clippy: import Layout from core::alloc instead of alloc::alloc

* Fix vectorcall compatibility with LocalsPlus API

Update vectorcall dispatch functions to use localsplus stack
accessors instead of direct stack field access. Add
stack_truncate method to LocalsPlus. Update vectorcall fast
path in function.rs to use datastack and fastlocals_mut().

* Add datastack, nlocalsplus, ncells, tstate to cspell dictionary

* Fix DataStack pop() for non-monotonic allocation addresses

Check both bounds of the current chunk when determining if a
pop base is in the current chunk. The previous check (base >=
chunk_start) fails on Windows where newer chunks may be
allocated at lower addresses than older ones.

* Fix stale comments: release_datastack -> materialize_localsplus

* Fix non-threading mode for parallel test execution

Two fixes for Cell-based types used in static items under non-threading
mode, which cause data races when Rust test runner uses parallel threads:

1. LazyLock: use std::sync::LazyLock when std is available instead of
   wrapping core::cell::LazyCell with a false `unsafe impl Sync`.
   The LazyCell wrapper is kept only for no-std (truly single-threaded).

2. gc_state: use static_cell! (thread-local in non-threading mode)
   instead of OnceLock, so each thread gets its own GcState with
   Cell-based PyRwLock/PyMutex that are not accessed concurrently.

* Fix CallAllocAndEnterInit to use LocalsPlus stack API

* Use checked arithmetic in LocalsPlus and DataStack allocators

* Address code review: checked arithmetic, threading feature deps, Send gate

- Use checked arithmetic for nlocalsplus in Frame::new
- Add "std" to threading feature dependencies in rustpython-common
- Gate GcState Send impl with #[cfg(feature = "threading")]

* Clean up comments: remove redundant/stale remarks, fix CPython references
This commit is contained in:
Jeong, YunWon authored and GitHub committed 2026-03-04 23:27:36 +09:00
1 parent a98c6469d1
commit 68ad332833
12 files changed
+967 -238

No files matched your search

+4
View File
@@ -43,6 +43,7 @@ CONVFUNC
convparam
copyslot
cpucount
datastack
defaultdict
denom
deopt
@@ -118,6 +119,7 @@ mult
multibytecodec
nameobj
nameop
ncells
nconsts
newargs
newfree
@@ -125,6 +127,7 @@ NEWLOCALS
newsemlockobject
nfrees
nkwargs
nlocalsplus
nkwelts
Nondescriptor
noninteger
@@ -192,6 +195,7 @@ testconsole
ticketer
tmptype
tok_oldval
tstate
tvars
typeobject
typeparam
+1 -1
View File
@@ -11,7 +11,7 @@ license.workspace = true
[features]
default = ["std"]
std = []
threading = ["parking_lot"]
threading = ["parking_lot", "std"]
wasm_js = ["getrandom/wasm_js"]
[dependencies]
+14 -4
View File
@@ -10,18 +10,28 @@ cfg_if::cfg_if! {
if #[cfg(feature = "threading")] {
pub use parking_lot::{RawMutex, RawRwLock, RawThreadId};
pub use std::sync::{LazyLock, OnceLock as OnceCell};
pub use std::sync::OnceLock as OnceCell;
pub use core::cell::LazyCell;
} else {
mod cell_lock;
pub use cell_lock::{RawCellMutex as RawMutex, RawCellRwLock as RawRwLock, SingleThreadId as RawThreadId};
pub use core::cell::{LazyCell, OnceCell};
}
}
/// `core::cell::LazyCell` with `Sync` for use in `static` items.
/// SAFETY: Without threading, there can be no concurrent access.
// LazyLock: uses std::sync::LazyLock when std is available (even without
// threading, because Rust test runner uses parallel threads).
// Without std, uses a LazyCell wrapper (truly single-threaded only).
cfg_if::cfg_if! {
if #[cfg(any(feature = "threading", feature = "std"))] {
pub use std::sync::LazyLock;
} else {
pub struct LazyLock<T, F = fn() -> T>(core::cell::LazyCell<T, F>);
// SAFETY: Without threading, there can be no concurrent access.
// SAFETY: This branch is only active when both "std" and "threading"
// features are absent — i.e., truly single-threaded no_std environments
// (e.g., embedded or bare-metal WASM). Without std, the Rust runtime
// cannot spawn threads, so Sync is trivially satisfied.
unsafe impl<T, F> Sync for LazyLock<T, F> {}
impl<T, F: FnOnce() -> T> LazyLock<T, F> {
+1 -1
View File
@@ -692,7 +692,7 @@ impl Py<Frame> {
// Clear fastlocals
// SAFETY: Frame is not executing (detached or stopped).
{
let fastlocals = unsafe { self.fastlocals.borrow_mut() };
let fastlocals = unsafe { self.fastlocals_mut() };
for slot in fastlocals.iter_mut() {
*slot = None;
}
+33 -8
View File
@@ -236,7 +236,7 @@ impl PyFunction {
// https://github.com/python/cpython/blob/main/Python/ceval.c#L3681
// SAFETY: Frame was just created and not yet executing.
let fastlocals = unsafe { frame.fastlocals.borrow_mut() };
let fastlocals = unsafe { frame.fastlocals_mut() };
let mut args_iter = func_args.args.into_iter();
@@ -562,6 +562,7 @@ impl Py<PyFunction> {
let is_gen = code.flags.contains(bytecode::CodeFlags::GENERATOR);
let is_coro = code.flags.contains(bytecode::CodeFlags::COROUTINE);
let use_datastack = !(is_gen || is_coro);
// Construct frame:
let frame = Frame::new(
@@ -570,6 +571,7 @@ impl Py<PyFunction> {
self.builtins.clone(),
self.closure.as_ref().map_or(&[], |c| c.as_slice()),
Some(self.to_owned().into()),
use_datastack,
vm,
)
.into_ref(&vm.ctx);
@@ -594,7 +596,16 @@ impl Py<PyFunction> {
frame.set_generator(&obj);
Ok(obj)
}
(false, false) => vm.run_frame(frame),
(false, false) => {
let result = vm.run_frame(frame.clone());
// Release data stack memory after frame execution completes.
unsafe {
if let Some(base) = frame.materialize_localsplus() {
vm.datastack_pop(base);
}
}
result
}
}
}
@@ -665,13 +676,14 @@ impl Py<PyFunction> {
self.builtins.clone(),
self.closure.as_ref().map_or(&[], |c| c.as_slice()),
Some(self.to_owned().into()),
true, // Always use datastack (invoke_exact_args is never gen/coro)
vm,
)
.into_ref(&vm.ctx);
// Move args directly into fastlocals (no clone/refcount needed)
{
let fastlocals = unsafe { frame.fastlocals.borrow_mut() };
let fastlocals = unsafe { frame.fastlocals_mut() };
for (slot, arg) in fastlocals.iter_mut().zip(args.drain(..)) {
*slot = Some(arg);
}
@@ -679,14 +691,20 @@ impl Py<PyFunction> {
// Handle cell2arg
if let Some(cell2arg) = code.cell2arg.as_deref() {
let fastlocals = unsafe { frame.fastlocals.borrow_mut() };
let fastlocals = unsafe { frame.fastlocals_mut() };
for (cell_idx, arg_idx) in cell2arg.iter().enumerate().filter(|(_, i)| **i != -1) {
let x = fastlocals[*arg_idx as usize].take();
frame.set_cell_contents(cell_idx, x);
}
}
vm.run_frame(frame)
let result = vm.run_frame(frame.clone());
unsafe {
if let Some(base) = frame.materialize_localsplus() {
vm.datastack_pop(base);
}
}
result
}
}
@@ -1291,26 +1309,33 @@ pub(crate) fn vectorcall_function(
zelf.builtins.clone(),
zelf.closure.as_ref().map_or(&[], |c| c.as_slice()),
Some(zelf.to_owned().into()),
true, // Always use datastack (is_simple excludes gen/coro)
vm,
)
.into_ref(&vm.ctx);
{
let fastlocals = unsafe { frame.fastlocals.borrow_mut() };
let fastlocals = unsafe { frame.fastlocals_mut() };
for (slot, arg) in fastlocals.iter_mut().zip(args.drain(..nargs)) {
*slot = Some(arg);
}
}
if let Some(cell2arg) = code.cell2arg.as_deref() {
let fastlocals = unsafe { frame.fastlocals.borrow_mut() };
let fastlocals = unsafe { frame.fastlocals_mut() };
for (cell_idx, arg_idx) in cell2arg.iter().enumerate().filter(|(_, i)| **i != -1) {
let x = fastlocals[*arg_idx as usize].take();
frame.set_cell_contents(cell_idx, x);
}
}
return vm.run_frame(frame);
let result = vm.run_frame(frame.clone());
unsafe {
if let Some(base) = frame.materialize_localsplus() {
vm.datastack_pop(base);
}
}
return result;
}
// SLOW PATH: construct FuncArgs from owned Vec and delegate to invoke()
+1 -1
View File
@@ -86,7 +86,7 @@ impl Initializer for PySuper {
return Err(vm.new_runtime_error("super(): no arguments"));
}
// SAFETY: Frame is current and not concurrently mutated.
let obj = unsafe { frame.fastlocals.borrow() }[0]
let obj = unsafe { frame.fastlocals() }[0]
.clone()
.or_else(|| {
if let Some(cell2arg) = frame.code.cell2arg.as_deref() {
+256
View File
@@ -0,0 +1,256 @@
/// Thread data stack for interpreter frames (`_PyStackChunk` /
/// `tstate->datastack_*`).
///
/// A linked list of chunks providing bump allocation for frame-local data
/// (localsplus arrays). Normal function calls allocate via `push()`
/// (pointer bump). Generators and coroutines use heap-allocated storage.
use alloc::alloc::{alloc, dealloc};
use core::alloc::Layout;
use core::ptr;
/// Minimum chunk size in bytes (`_PY_DATA_STACK_CHUNK_SIZE`).
const MIN_CHUNK_SIZE: usize = 16 * 1024;
/// Extra headroom (in bytes) to avoid allocating a new chunk for the next
/// frame right after growing.
const MINIMUM_OVERHEAD: usize = 1000 * core::mem::size_of::<usize>();
/// Alignment for all data stack allocations.
const ALIGN: usize = 16;
/// Header for a data stack chunk. The usable data region starts right after
/// this header (aligned to `ALIGN`).
#[repr(C)]
struct DataStackChunk {
/// Previous chunk in the linked list (NULL for the root chunk).
previous: *mut DataStackChunk,
/// Total allocation size in bytes (including this header).
size: usize,
/// Saved `top` offset when a newer chunk was pushed. Used to restore
/// `DataStack::top` when popping back to this chunk.
saved_top: usize,
}
impl DataStackChunk {
/// Pointer to the first usable byte after the header (aligned).
#[inline(always)]
fn data_start(&self) -> *mut u8 {
let header_end = (self as *const Self as usize) + core::mem::size_of::<Self>();
let aligned = (header_end + ALIGN - 1) & !(ALIGN - 1);
aligned as *mut u8
}
/// Pointer past the last usable byte.
#[inline(always)]
fn data_limit(&self) -> *mut u8 {
unsafe { (self as *const Self as *mut u8).add(self.size) }
}
}
/// Per-thread data stack for bump-allocating frame-local data.
pub struct DataStack {
/// Current chunk.
chunk: *mut DataStackChunk,
/// Current allocation position within the current chunk.
top: *mut u8,
/// End of usable space in the current chunk.
limit: *mut u8,
}
impl DataStack {
/// Create a new data stack with an initial root chunk.
pub fn new() -> Self {
let chunk = Self::alloc_chunk(MIN_CHUNK_SIZE, ptr::null_mut());
let top = unsafe { (*chunk).data_start() };
let limit = unsafe { (*chunk).data_limit() };
// Skip one ALIGN-sized slot in the root chunk so that `pop()` never
// frees it (`push_chunk` convention).
let top = unsafe { top.add(ALIGN) };
Self { chunk, top, limit }
}
/// Check if the current chunk has at least `size` bytes available.
#[inline(always)]
pub fn has_space(&self, size: usize) -> bool {
let aligned_size = (size + ALIGN - 1) & !(ALIGN - 1);
(self.limit as usize).saturating_sub(self.top as usize) >= aligned_size
}
/// Allocate `size` bytes from the data stack.
///
/// Returns a pointer to the allocated region (aligned to `ALIGN`).
/// The caller must call `pop()` with the returned pointer when done
/// (LIFO order).
#[inline(always)]
pub fn push(&mut self, size: usize) -> *mut u8 {
let aligned_size = (size + ALIGN - 1) & !(ALIGN - 1);
unsafe {
if self.top.add(aligned_size) <= self.limit {
let ptr = self.top;
self.top = self.top.add(aligned_size);
ptr
} else {
self.push_slow(aligned_size)
}
}
}
/// Slow path: allocate a new chunk and push from it.
#[cold]
#[inline(never)]
fn push_slow(&mut self, aligned_size: usize) -> *mut u8 {
let mut chunk_size = MIN_CHUNK_SIZE;
let needed = aligned_size
.checked_add(MINIMUM_OVERHEAD)
.and_then(|v| v.checked_add(core::mem::size_of::<DataStackChunk>()))
.and_then(|v| v.checked_add(ALIGN))
.expect("DataStack chunk size overflow");
while chunk_size < needed {
chunk_size = chunk_size
.checked_mul(2)
.expect("DataStack chunk size overflow");
}
// Save current position in old chunk.
unsafe {
(*self.chunk).saved_top = self.top as usize - self.chunk as usize;
}
let new_chunk = Self::alloc_chunk(chunk_size, self.chunk);
self.chunk = new_chunk;
let start = unsafe { (*new_chunk).data_start() };
self.limit = unsafe { (*new_chunk).data_limit() };
self.top = unsafe { start.add(aligned_size) };
start
}
/// Pop a previous allocation. `base` must be the pointer returned by
/// `push()`. Calls must be in LIFO order.
///
/// # Safety
/// `base` must be a valid pointer returned by `push()` on this data stack,
/// and all allocations made after it must already have been popped.
#[inline(always)]
pub unsafe fn pop(&mut self, base: *mut u8) {
debug_assert!(!base.is_null());
if self.is_in_current_chunk(base) {
// Common case: base is within the current chunk.
self.top = base;
} else {
// base is in a previous chunk — free the current chunk.
unsafe { self.pop_slow(base) };
}
}
/// Check if `ptr` falls within the current chunk's data area.
/// Both bounds are checked to handle non-monotonic allocation addresses
/// (e.g. on Windows where newer chunks may be at lower addresses).
#[inline(always)]
fn is_in_current_chunk(&self, ptr: *mut u8) -> bool {
let chunk_start = unsafe { (*self.chunk).data_start() };
ptr >= chunk_start && ptr <= self.limit
}
/// Slow path: pop back to a previous chunk.
#[cold]
#[inline(never)]
unsafe fn pop_slow(&mut self, base: *mut u8) {
loop {
let old_chunk = self.chunk;
let prev = unsafe { (*old_chunk).previous };
debug_assert!(!prev.is_null(), "tried to pop past the root chunk");
unsafe { Self::free_chunk(old_chunk) };
self.chunk = prev;
self.limit = unsafe { (*prev).data_limit() };
if self.is_in_current_chunk(base) {
self.top = base;
return;
}
}
}
/// Allocate a new chunk.
fn alloc_chunk(size: usize, previous: *mut DataStackChunk) -> *mut DataStackChunk {
let layout = Layout::from_size_align(size, ALIGN).expect("invalid chunk layout");
let ptr = unsafe { alloc(layout) };
if ptr.is_null() {
alloc::alloc::handle_alloc_error(layout);
}
let chunk = ptr as *mut DataStackChunk;
unsafe {
(*chunk).previous = previous;
(*chunk).size = size;
(*chunk).saved_top = 0;
}
chunk
}
/// Free a chunk.
unsafe fn free_chunk(chunk: *mut DataStackChunk) {
let size = unsafe { (*chunk).size };
let layout = Layout::from_size_align(size, ALIGN).expect("invalid chunk layout");
unsafe { dealloc(chunk as *mut u8, layout) };
}
}
// SAFETY: DataStack is per-thread and not shared. The raw pointers
// it contains point to memory exclusively owned by this DataStack.
unsafe impl Send for DataStack {}
impl Default for DataStack {
fn default() -> Self {
Self::new()
}
}
impl Drop for DataStack {
fn drop(&mut self) {
let mut chunk = self.chunk;
while !chunk.is_null() {
let prev = unsafe { (*chunk).previous };
unsafe { Self::free_chunk(chunk) };
chunk = prev;
}
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn basic_push_pop() {
let mut ds = DataStack::new();
let p1 = ds.push(64);
assert!(!p1.is_null());
let p2 = ds.push(128);
assert!(!p2.is_null());
assert!(p2 > p1);
unsafe {
ds.pop(p2);
ds.pop(p1);
}
}
#[test]
fn cross_chunk_push_pop() {
let mut ds = DataStack::new();
// Push enough to force a new chunk
let mut ptrs = Vec::new();
for _ in 0..100 {
ptrs.push(ds.push(1024));
}
// Pop all in reverse
for p in ptrs.into_iter().rev() {
unsafe { ds.pop(p) };
}
}
#[test]
fn alignment() {
let mut ds = DataStack::new();
for size in [1, 7, 15, 16, 17, 31, 32, 33, 64, 100] {
let p = ds.push(size);
assert_eq!(p as usize % ALIGN, 0, "alignment violated for size {size}");
unsafe { ds.pop(p) };
}
}
}
+617 -212
View File
File diff suppressed because it is too large. Load diff
+14 -10
View File
@@ -134,10 +134,13 @@ pub struct GcState {
finalized_objects: PyRwLock<HashSet<GcObjectPtr>>,
}
// SAFETY: All fields are either inherently Send/Sync (atomics, RwLock, Mutex) or protected by PyMutex.
// PyMutex<Vec<PyObjectRef>> is safe to share/send across threads because access is synchronized.
// PyObjectRef itself is Send, and interior mutability is guarded by the mutex.
// SAFETY: GcObjectPtr wraps NonNull which is !Send/!Sync, but we only use it as an opaque
// hash key. PyObjectRef is Send. In threading mode, PyRwLock/PyMutex are parking_lot based
// and genuinely Sync. In non-threading mode, gc_state() is thread-local so neither Send
// nor Sync is needed.
#[cfg(feature = "threading")]
unsafe impl Send for GcState {}
#[cfg(feature = "threading")]
unsafe impl Sync for GcState {}
impl Default for GcState {
@@ -827,14 +830,15 @@ impl GcState {
}
}
use std::sync::OnceLock;
/// Global GC state instance
/// Using a static because GC needs to be accessible from object allocation/deallocation
static GC_STATE: OnceLock<GcState> = OnceLock::new();
/// Get a reference to the global GC state
/// Get a reference to the GC state.
///
/// In threading mode this is a true global (OnceLock).
/// In non-threading mode this is thread-local, because PyRwLock/PyMutex
/// use Cell-based locks that are not Sync.
pub fn gc_state() -> &'static GcState {
rustpython_common::static_cell! {
static GC_STATE: GcState;
}
GC_STATE.get_or_init(GcState::new)
}
+1
View File
@@ -51,6 +51,7 @@ mod codecs;
pub mod compiler;
pub mod convert;
mod coroutine;
pub mod datastack;
mod dict_inner;
#[cfg(feature = "rustpython-compiler")]
+24 -1
View File
@@ -74,6 +74,9 @@ pub struct VirtualMachine {
pub sys_module: PyRef<PyModule>,
pub ctx: PyRc<Context>,
pub frames: RefCell<Vec<FramePtr>>,
/// Thread-local data stack for bump-allocating frame-local data
/// (localsplus arrays for non-generator frames).
datastack: core::cell::UnsafeCell<crate::datastack::DataStack>,
pub wasm_id: Option<String>,
exceptions: RefCell<ExceptionStack>,
pub import_func: PyObjectRef,
@@ -172,6 +175,25 @@ pub fn process_hash_secret_seed() -> u32 {
}
impl VirtualMachine {
/// Bump-allocate `size` bytes from the thread data stack.
///
/// # Safety
/// The returned pointer must be freed by calling `datastack_pop` in LIFO order.
#[inline(always)]
pub(crate) fn datastack_push(&self, size: usize) -> *mut u8 {
unsafe { (*self.datastack.get()).push(size) }
}
/// Pop a previous data stack allocation.
///
/// # Safety
/// `base` must be a pointer returned by `datastack_push` on this VM,
/// and all allocations made after it must already have been popped.
#[inline(always)]
pub(crate) unsafe fn datastack_pop(&self, base: *mut u8) {
unsafe { (*self.datastack.get()).pop(base) }
}
/// Check whether the current thread is the main thread.
/// Mirrors `_Py_ThreadCanHandleSignals`.
#[allow(dead_code)]
@@ -215,6 +237,7 @@ impl VirtualMachine {
sys_module,
ctx,
frames: RefCell::new(vec![]),
datastack: core::cell::UnsafeCell::new(crate::datastack::DataStack::new()),
wasm_id: None,
exceptions: RefCell::default(),
import_func,
@@ -591,7 +614,7 @@ impl VirtualMachine {
};
let frame =
Frame::new(code, scope, builtins, &[], Some(func_obj), self).into_ref(&self.ctx);
Frame::new(code, scope, builtins, &[], Some(func_obj), false, self).into_ref(&self.ctx);
self.run_frame(frame)
}
+1
View File
@@ -309,6 +309,7 @@ impl VirtualMachine {
sys_module: self.sys_module.clone(),
ctx: self.ctx.clone(),
frames: RefCell::new(vec![]),
datastack: core::cell::UnsafeCell::new(crate::datastack::DataStack::new()),
wasm_id: self.wasm_id.clone(),
exceptions: RefCell::default(),
import_func: self.import_func.clone(),