Skip to content

Commit d474353

Browse files
youknowoneyouknowone
authored
Address slot members by byte offset (#8811)
* Keep slot values freelist-eligible Member cells now sit in the object allocation ahead of the extension header, so a slot read is one pointer instead of a boxed slice. A value stored in a slot is not marked published. With threading, only the value that leaves the slot is, so its free can wait out a reader. Without threading a slot read is a single incref. Assisted-by: Grok:4.7 * Address slot members by byte offset A slot member's offset is the byte distance from the object to its inline pointer cell. Loads and stores, including the specialized instructions, use that address. A relative C offset is resolved by adding the object header and the type's basic size. Assisted-by: Grok:4.7 * Keep base slot offsets stable on subclasses Cell 0 sits directly in front of the extension header. Further slots extend away from the object, so a member offset computed for a base class still addresses the same cell on a subclass. Assisted-by: Grok:4.7 * Pack slot cells flush against the extension header On 32-bit targets an odd slot count is not a multiple of ObjExt's alignment. The padding now sits in front of the cells, so cell 0 stays immediately before the header. The non-threading slot load is also the function's return value. Assisted-by: Grok:4.7 * Place weakref lists in front of slot cells A subclass that adds a weakref list no longer inserts it between ObjExt and the object, so inherited member offsets stay valid. Assisted-by: Grok:4.7
1 parent e56dfa1 commit d474353

8 files changed

Lines changed: 210 additions & 114 deletions

File tree

‎crates/capi/src/descrobject.rs‎

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ use crate::pystate::with_vm;
55
use crate::util::{CStrExt, FfiPtrExt};
66
use core::ffi::{c_char, c_int, c_void};
77
use rustpython_vm::builtins::{
8-
DescriptorMemberDef, MemberAccess, MemberKind, PY_READONLY, PY_RELATIVE_OFFSET,
9-
PyDescriptorOwned, PyGetSet, PyMappingProxy, PyMemberDescriptor, PyType,
8+
DescriptorMemberDef, MemberAccess, MemberKind, PY_RELATIVE_OFFSET, PyDescriptorOwned, PyGetSet,
9+
PyMappingProxy, PyMemberDescriptor, PyType,
1010
};
1111
use rustpython_vm::common::lock::PyRwLock;
1212
use rustpython_vm::function::PySetterValue;
@@ -126,17 +126,15 @@ impl PyMemberDef {
126126
self.type_code
127127
)));
128128
};
129-
if self.offset < 0 {
130-
return Err(vm.new_system_error("PyDescr_NewMember does not support negative offsets"));
131-
}
132-
if self.flags & PY_RELATIVE_OFFSET != 0 {
133-
return Err(
134-
vm.new_system_error("PyDescr_NewMember does not support Py_RELATIVE_OFFSET")
135-
);
129+
let mut offset = self.offset;
130+
let mut flags = self.flags;
131+
if flags & PY_RELATIVE_OFFSET != 0 {
132+
// type creation adds tp_basicsize and clears the flag before GetOne.
133+
offset += (rustpython_vm::object::SIZEOF_PYOBJECT_HEAD + ty.slots.basicsize) as isize;
134+
flags &= !PY_RELATIVE_OFFSET;
136135
}
137136

138137
let doc = unsafe { self.doc.try_as_str_opt(vm) }?.map(str::to_owned);
139-
let readonly = self.flags & PY_READONLY != 0;
140138

141139
let descriptor = PyMemberDescriptor {
142140
common: PyDescriptorOwned {
@@ -147,11 +145,11 @@ impl PyMemberDef {
147145
member: DescriptorMemberDef {
148146
name: name.to_owned(),
149147
kind,
150-
offset: self.offset,
151-
flags: if readonly { PY_READONLY } else { 0 },
148+
offset,
149+
flags,
152150
doc,
153151
},
154-
// Instance members live in the slot array. `offset` is that index.
152+
// `offset` is a byte offset from the object to a pointer cell.
155153
access: MemberAccess::Slot,
156154
};
157155

‎crates/vm/src/builtins/descriptor.rs‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -355,9 +355,9 @@ pub(crate) type MemberSetterFunc =
355355

356356
/// Where `PyMemberDef.offset` points.
357357
///
358-
/// Builtin payloads keep fields behind locks, so a byte offset from the
359-
/// object header does not address them. Slot members use `offset` as an
360-
/// index into the instance slot array. Function members ignore `offset`.
358+
/// Slot members use a byte offset from the object to an inline pointer cell.
359+
/// Builtin payloads keep fields behind locks, so those members use a function
360+
/// and ignore `offset`.
361361
#[derive(Clone, Copy)]
362362
pub enum MemberAccess {
363363
Func {
@@ -415,10 +415,10 @@ pub struct PyMemberDescriptor {
415415
}
416416

417417
impl PyMemberDescriptor {
418-
/// Slot index for an instance-slot member. `None` when the offset is not a slot.
419-
pub(crate) fn slot_offset(&self) -> Option<usize> {
418+
/// Byte offset of an instance-slot member. `None` when this is not a slot.
419+
pub(crate) fn slot_offset(&self) -> Option<isize> {
420420
match self.access {
421-
MemberAccess::Slot => Some(self.member.offset as usize),
421+
MemberAccess::Slot => Some(self.member.offset),
422422
_ => None,
423423
}
424424
}
@@ -436,9 +436,7 @@ impl PyMemberDescriptor {
436436
}
437437
match self.access {
438438
MemberAccess::Func { get, .. } => get(vm, obj),
439-
MemberAccess::Slot => {
440-
get_slot_from_object(&obj, self.member.offset as usize, &self.member, vm)
441-
}
439+
MemberAccess::Slot => get_slot_from_object(&obj, self.member.offset, &self.member, vm),
442440
MemberAccess::TupleItem => {
443441
let index = self.member.offset as usize;
444442
let tuple = obj.downcast_ref::<PyTuple>().ok_or_else(|| {
@@ -468,7 +466,7 @@ impl PyMemberDescriptor {
468466
None => Err(vm.new_attribute_error("readonly attribute")),
469467
},
470468
MemberAccess::Slot => {
471-
set_slot_at_object(&obj, self.member.offset as usize, &self.member, value, vm)
469+
set_slot_at_object(&obj, self.member.offset, &self.member, value, vm)
472470
}
473471
MemberAccess::TupleItem => Err(vm.new_attribute_error("readonly attribute")),
474472
}
@@ -566,7 +564,7 @@ impl PyMemberDescriptor {
566564
// PyMember_GetOne for a value stored in the instance slot array.
567565
fn get_slot_from_object(
568566
obj: &PyObject,
569-
offset: usize,
567+
offset: isize,
570568
member: &PyMemberDef,
571569
vm: &VirtualMachine,
572570
) -> PyResult {
@@ -589,7 +587,7 @@ fn get_slot_from_object(
589587
// PyMember_SetOne for a value stored in the instance slot array.
590588
fn set_slot_at_object(
591589
obj: &PyObject,
592-
offset: usize,
590+
offset: isize,
593591
member: &PyMemberDef,
594592
value: PySetterValue,
595593
vm: &VirtualMachine,

‎crates/vm/src/builtins/type.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2782,7 +2782,7 @@ impl Constructor for PyType {
27822782
let member_def = PyMemberDef {
27832783
name: mangled_name.clone(),
27842784
kind: MemberKind::ObjectEx,
2785-
offset: offset as isize,
2785+
offset: crate::object::slot_member_offset(offset),
27862786
flags: 0,
27872787
doc: None,
27882788
};

‎crates/vm/src/frame.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6291,7 +6291,7 @@ impl ExecutingFrame<'_> {
62916291

62926292
if type_version != 0 && owner.class().tp_version_tag.load(Acquire) == type_version {
62936293
let slot_offset =
6294-
self.code.instructions.read_cache_u32(cache_base + 3) as usize;
6294+
self.code.instructions.read_cache_u32(cache_base + 3) as i32 as isize;
62956295
if let Some(value) = owner.get_slot(slot_offset) {
62966296
self.pop_stackref();
62976297
if oparg.is_method() {
@@ -6381,7 +6381,7 @@ impl ExecutingFrame<'_> {
63816381

63826382
if version_match {
63836383
let slot_offset =
6384-
self.code.instructions.read_cache_u16(cache_base + 3) as usize;
6384+
self.code.instructions.read_cache_u16(cache_base + 3) as i16 as isize;
63856385
let owner = self.pop_value();
63866386
let value = self.pop_value();
63876387
owner.set_slot(slot_offset, Some(value));

0 commit comments

Comments
 (0)