Skip to content

gh-112529: Remove PyGC_Head from object pre-header in free-threaded build #114564

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 18 additions & 9 deletions Include/internal/pycore_object.h
Original file line number Diff line number Diff line change
Expand Up @@ -315,16 +315,15 @@ static inline void _PyObject_GC_TRACK(
_PyObject_ASSERT_FROM(op, !_PyObject_GC_IS_TRACKED(op),
"object already tracked by the garbage collector",
filename, lineno, __func__);

#ifdef Py_GIL_DISABLED
op->ob_gc_bits |= _PyGC_BITS_TRACKED;
#else
PyGC_Head *gc = _Py_AS_GC(op);
_PyObject_ASSERT_FROM(op,
(gc->_gc_prev & _PyGC_PREV_MASK_COLLECTING) == 0,
"object is in generation which is garbage collected",
filename, lineno, __func__);

#ifdef Py_GIL_DISABLED
op->ob_gc_bits |= _PyGC_BITS_TRACKED;
#else
PyInterpreterState *interp = _PyInterpreterState_GET();
PyGC_Head *generation0 = interp->gc.generation0;
PyGC_Head *last = (PyGC_Head*)(generation0->_gc_prev);
Expand Down Expand Up @@ -594,8 +593,12 @@ _PyObject_IS_GC(PyObject *obj)
static inline size_t
_PyType_PreHeaderSize(PyTypeObject *tp)
{
return _PyType_IS_GC(tp) * sizeof(PyGC_Head) +
_PyType_HasFeature(tp, Py_TPFLAGS_PREHEADER) * 2 * sizeof(PyObject *);
return (
#ifndef Py_GIL_DISABLED
_PyType_IS_GC(tp) * sizeof(PyGC_Head) +
#endif
_PyType_HasFeature(tp, Py_TPFLAGS_PREHEADER) * 2 * sizeof(PyObject *)
);
}

void _PyObject_GC_Link(PyObject *op);
Expand Down Expand Up @@ -625,6 +628,14 @@ extern int _PyObject_StoreInstanceAttribute(PyObject *obj, PyDictValues *values,
PyObject * _PyObject_GetInstanceAttribute(PyObject *obj, PyDictValues *values,
PyObject *name);

#ifdef Py_GIL_DISABLED
# define MANAGED_DICT_OFFSET (((Py_ssize_t)sizeof(PyObject *))*-1)
# define MANAGED_WEAKREF_OFFSET (((Py_ssize_t)sizeof(PyObject *))*-2)
#else
# define MANAGED_DICT_OFFSET (((Py_ssize_t)sizeof(PyObject *))*-3)
# define MANAGED_WEAKREF_OFFSET (((Py_ssize_t)sizeof(PyObject *))*-4)
#endif

typedef union {
PyObject *dict;
/* Use a char* to generate a warning if directly assigning a PyDictValues */
Expand All @@ -635,7 +646,7 @@ static inline PyDictOrValues *
_PyObject_DictOrValuesPointer(PyObject *obj)
{
assert(Py_TYPE(obj)->tp_flags & Py_TPFLAGS_MANAGED_DICT);
return ((PyDictOrValues *)obj)-3;
return (PyDictOrValues *)((char *)obj + MANAGED_DICT_OFFSET);
}

static inline int
Expand Down Expand Up @@ -664,8 +675,6 @@ _PyDictOrValues_SetValues(PyDictOrValues *ptr, PyDictValues *values)
ptr->values = ((char *)values) - 1;
}

#define MANAGED_WEAKREF_OFFSET (((Py_ssize_t)sizeof(PyObject *))*-4)

extern PyObject ** _PyObject_ComputedDictPointer(PyObject *);
extern void _PyObject_FreeInstanceAttributes(PyObject *obj);
extern int _PyObject_IsInstanceDictEmpty(PyObject *);
Expand Down
5 changes: 3 additions & 2 deletions Include/object.h
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,9 @@ struct _object {
struct _PyMutex { uint8_t v; };

struct _object {
// ob_tid stores the thread id (or zero). It is also used by the GC to
// store linked lists and the computed "gc_refs" refcount.
// ob_tid stores the thread id (or zero). It is also used by the GC and the
// trashcan mechanism as a linked list pointer and by the GC to store the
// computed "gc_refs" refcount.
uintptr_t ob_tid;
uint16_t _padding;
struct _PyMutex ob_mutex; // per-object lock
Expand Down
5 changes: 3 additions & 2 deletions Lib/test/test_sys.py
Original file line number Diff line number Diff line change
Expand Up @@ -1392,6 +1392,7 @@ def setUp(self):
self.longdigit = sys.int_info.sizeof_digit
import _testinternalcapi
self.gc_headsize = _testinternalcapi.SIZEOF_PYGC_HEAD
self.managed_pre_header_size = _testinternalcapi.SIZEOF_MANAGED_PRE_HEADER

check_sizeof = test.support.check_sizeof

Expand Down Expand Up @@ -1427,7 +1428,7 @@ class OverflowSizeof(int):
def __sizeof__(self):
return int(self)
self.assertEqual(sys.getsizeof(OverflowSizeof(sys.maxsize)),
sys.maxsize + self.gc_headsize*2)
sys.maxsize + self.gc_headsize + self.managed_pre_header_size)
with self.assertRaises(OverflowError):
sys.getsizeof(OverflowSizeof(sys.maxsize + 1))
with self.assertRaises(ValueError):
Expand Down Expand Up @@ -1650,7 +1651,7 @@ def delx(self): del self.__x
# type
# static type: PyTypeObject
fmt = 'P2nPI13Pl4Pn9Pn12PIPc'
s = vsize('2P' + fmt)
s = vsize(fmt)
check(int, s)
# class
s = vsize(fmt + # PyTypeObject
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
The free-threaded build no longer allocates space for the ``PyGC_Head``
structure in objects that support cyclic garbage collection. A number of
other fields and data structures are used as replacements, including
``ob_gc_bits``, ``ob_tid``, and mimalloc internal data structures.
12 changes: 11 additions & 1 deletion Modules/_testinternalcapi.c
Original file line number Diff line number Diff line change
Expand Up @@ -1752,8 +1752,18 @@ module_exec(PyObject *module)
return 1;
}

Py_ssize_t sizeof_gc_head = 0;
#ifndef Py_GIL_DISABLED
sizeof_gc_head = sizeof(PyGC_Head);
#endif

if (PyModule_Add(module, "SIZEOF_PYGC_HEAD",
PyLong_FromSsize_t(sizeof(PyGC_Head))) < 0) {
PyLong_FromSsize_t(sizeof_gc_head)) < 0) {
return 1;
}

if (PyModule_Add(module, "SIZEOF_MANAGED_PRE_HEADER",
PyLong_FromSsize_t(2 * sizeof(PyObject*))) < 0) {
return 1;
}

Expand Down
13 changes: 11 additions & 2 deletions Objects/object.c
Original file line number Diff line number Diff line change
Expand Up @@ -2671,7 +2671,12 @@ _PyTrash_thread_deposit_object(struct _py_trashcan *trash, PyObject *op)
_PyObject_ASSERT(op, _PyObject_IS_GC(op));
_PyObject_ASSERT(op, !_PyObject_GC_IS_TRACKED(op));
_PyObject_ASSERT(op, Py_REFCNT(op) == 0);
#ifdef Py_GIL_DISABLED
_PyObject_ASSERT(op, op->ob_tid == 0);
op->ob_tid = (uintptr_t)trash->delete_later;
#else
_PyGCHead_SET_PREV(_Py_AS_GC(op), (PyGC_Head*)trash->delete_later);
#endif
trash->delete_later = op;
}

Expand All @@ -2697,8 +2702,12 @@ _PyTrash_thread_destroy_chain(struct _py_trashcan *trash)
PyObject *op = trash->delete_later;
destructor dealloc = Py_TYPE(op)->tp_dealloc;

trash->delete_later =
(PyObject*) _PyGCHead_PREV(_Py_AS_GC(op));
#ifdef Py_GIL_DISABLED
trash->delete_later = (PyObject*) op->ob_tid;
op->ob_tid = 0;
#else
trash->delete_later = (PyObject*) _PyGCHead_PREV(_Py_AS_GC(op));
#endif

/* Call the deallocator directly. This used to try to
* fool Py_DECREF into calling it indirectly, but
Expand Down
21 changes: 16 additions & 5 deletions Python/gc_free_threading.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ typedef struct _gc_runtime_state GCState;
// Automatically choose the generation that needs collecting.
#define GENERATION_AUTO (-1)

// A linked-list of objects using the `ob_tid` field as the next pointer.
// A linked list of objects using the `ob_tid` field as the next pointer.
// The linked list pointers are distinct from any real thread ids, because the
// thread ids returned by _Py_ThreadId() are also pointers to distinct objects.
// No thread will confuse its own id with a linked list pointer.
struct worklist {
uintptr_t head;
};
Expand Down Expand Up @@ -220,7 +223,7 @@ gc_visit_heaps_lock_held(PyInterpreterState *interp, mi_block_visit_fun *visitor
struct visitor_args *arg)
{
// Offset of PyObject header from start of memory block.
Py_ssize_t offset_base = sizeof(PyGC_Head);
Py_ssize_t offset_base = 0;
if (_PyMem_DebugEnabled()) {
// The debug allocator adds two words at the beginning of each block.
offset_base += 2 * sizeof(size_t);
Expand Down Expand Up @@ -330,8 +333,14 @@ update_refs(const mi_heap_t *heap, const mi_heap_area_t *area,
Py_ssize_t refcount = Py_REFCNT(op);
_PyObject_ASSERT(op, refcount >= 0);

// Add the actual refcount to ob_tid.
// We repurpose ob_tid to compute "gc_refs", the number of external
// references to the object (i.e., from outside the GC heaps). This means
// that ob_tid is no longer a valid thread id until it is restored by
// scan_heap_visitor(). Until then, we cannot use the standard reference
// counting functions or allow other threads to run Python code.
gc_maybe_init_refs(op);

// Add the actual refcount to ob_tid.
gc_add_refs(op, refcount);

// Subtract internal references from ob_tid. Objects with ob_tid > 0
Expand Down Expand Up @@ -1552,8 +1561,10 @@ gc_alloc(PyTypeObject *tp, size_t basicsize, size_t presize)
if (mem == NULL) {
return _PyErr_NoMemory(tstate);
}
((PyObject **)mem)[0] = NULL;
((PyObject **)mem)[1] = NULL;
if (presize) {
((PyObject **)mem)[0] = NULL;
((PyObject **)mem)[1] = NULL;
}
PyObject *op = (PyObject *)(mem + presize);
_PyObject_GC_Link(op);
return op;
Expand Down
10 changes: 9 additions & 1 deletion Python/sysmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -1878,7 +1878,15 @@ _PySys_GetSizeOf(PyObject *o)
return (size_t)-1;
}

return (size_t)size + _PyType_PreHeaderSize(Py_TYPE(o));
size_t presize = 0;
if (!Py_IS_TYPE(o, &PyType_Type) ||
PyType_HasFeature((PyTypeObject *)o, Py_TPFLAGS_HEAPTYPE))
{
/* Add the size of the pre-header if "o" is not a static type */
presize = _PyType_PreHeaderSize(Py_TYPE(o));
}
Comment on lines +1882 to +1887
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This now more closely matches the behavior of the Python check_sizeof function, although that function does not account for space for managed dict/weakref.


return (size_t)size + presize;
}

static PyObject *
Expand Down
15 changes: 11 additions & 4 deletions Tools/gdb/libpython.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,14 @@ def _type_unsigned_int_ptr():
def _sizeof_void_p():
return gdb.lookup_type('void').pointer().sizeof

def _managed_dict_offset():
# See pycore_object.h
pyobj = gdb.lookup_type("PyObject")
if any(field.name == "ob_ref_local" for field in pyobj.fields()):
return -1 * _sizeof_void_p()
else:
return -3 * _sizeof_void_p()


Py_TPFLAGS_MANAGED_DICT = (1 << 4)
Py_TPFLAGS_HEAPTYPE = (1 << 9)
Expand Down Expand Up @@ -457,7 +465,7 @@ def get_attr_dict(self):
if dictoffset < 0:
if int_from_int(typeobj.field('tp_flags')) & Py_TPFLAGS_MANAGED_DICT:
assert dictoffset == -1
dictoffset = -3 * _sizeof_void_p()
dictoffset = _managed_dict_offset()
else:
type_PyVarObject_ptr = gdb.lookup_type('PyVarObject').pointer()
tsize = int_from_int(self._gdbval.cast(type_PyVarObject_ptr)['ob_size'])
Expand Down Expand Up @@ -485,9 +493,8 @@ def get_keys_values(self):
has_values = int_from_int(typeobj.field('tp_flags')) & Py_TPFLAGS_MANAGED_DICT
if not has_values:
return None
charptrptr_t = _type_char_ptr().pointer()
ptr = self._gdbval.cast(charptrptr_t) - 3
char_ptr = ptr.dereference()
ptr = self._gdbval.cast(_type_char_ptr()) + _managed_dict_offset()
char_ptr = ptr.cast(_type_char_ptr().pointer()).dereference()
if (int(char_ptr) & 1) == 0:
return None
char_ptr += 1
Expand Down