-
-
Notifications
You must be signed in to change notification settings - Fork 30.1k
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
gh-122417: Implement per-thread heap type refcounts #122418
Changes from 2 commits
103bd0e
4c3ba8e
c33e6fa
e185840
883f8d3
15229c2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -14,10 +14,19 @@ extern "C" { | |||||
#include "pycore_interp.h" // PyInterpreterState.gc | ||||||
#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_STORE_PTR_RELAXED | ||||||
#include "pycore_pystate.h" // _PyInterpreterState_GET() | ||||||
#include "pycore_typeid.h" // _PyType_IncrefSlow | ||||||
|
||||||
|
||||||
#define _Py_IMMORTAL_REFCNT_LOOSE ((_Py_IMMORTAL_REFCNT >> 1) + 1) | ||||||
|
||||||
// This value is added to `ob_ref_shared` for objects that use deferred | ||||||
// reference counting so that they are not immediately deallocated when the | ||||||
// non-deferred reference count drops to zero. | ||||||
// | ||||||
// The value is half the maximum shared refcount because the low two bits of | ||||||
// `ob_ref_shared` are used for flags. | ||||||
#define _Py_REF_DEFERRED (PY_SSIZE_T_MAX / 8) | ||||||
|
||||||
// gh-121528, gh-118997: Similar to _Py_IsImmortal() but be more loose when | ||||||
// comparing the reference count to stay compatible with C extensions built | ||||||
// with the stable ABI 3.11 or older. Such extensions implement INCREF/DECREF | ||||||
|
@@ -280,6 +289,64 @@ extern PyStatus _PyObject_InitState(PyInterpreterState *interp); | |||||
extern void _PyObject_FiniState(PyInterpreterState *interp); | ||||||
extern bool _PyRefchain_IsTraced(PyInterpreterState *interp, PyObject *obj); | ||||||
|
||||||
static inline void | ||||||
markshannon marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
_Py_INCREF_TYPE(PyTypeObject *type) | ||||||
{ | ||||||
#ifndef Py_GIL_DISABLED | ||||||
Py_INCREF(type); | ||||||
#else | ||||||
if (!_PyType_HasFeature(type, Py_TPFLAGS_HEAPTYPE)) { | ||||||
assert(_Py_IsImmortal(type)); | ||||||
return; | ||||||
} | ||||||
|
||||||
_PyThreadStateImpl *tstate = (_PyThreadStateImpl *)_PyThreadState_GET(); | ||||||
PyHeapTypeObject *ht = (PyHeapTypeObject *)type; | ||||||
|
||||||
// Unsigned comparison so that `ht_id=-1` is treated as out-of-bounds. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or just simply
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've updated the comment and moved the "fast path" to the "if" branch instead of the "else" |
||||||
Py_ssize_t ht_id = ht->_ht_id; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit comment:
It will be helpful to understand what's happening in here for people like me :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. By the way, you can ignore this, but some people may need to understand the mechanism when they have to write similar features. At that time it will be helpful to verify ht_id There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see a great way of refactoring out this check -- I think a function name that accurately describes the check will be awkwardly long. It's not just that the heap type has an assigned id (i.e., |
||||||
if ((size_t)ht_id >= (size_t)tstate->types.size) { | ||||||
_PyType_IncrefSlow(ht); | ||||||
} | ||||||
else { | ||||||
# ifdef Py_REF_DEBUG | ||||||
_Py_INCREF_IncRefTotal(); | ||||||
# endif | ||||||
_Py_INCREF_STAT_INC(); | ||||||
tstate->types.refcounts[ht_id]++; | ||||||
} | ||||||
#endif | ||||||
} | ||||||
|
||||||
static inline void | ||||||
_Py_DECREF_TYPE(PyTypeObject *type) | ||||||
markshannon marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
{ | ||||||
#ifndef Py_GIL_DISABLED | ||||||
Py_DECREF(type); | ||||||
#else | ||||||
if (!_PyType_HasFeature(type, Py_TPFLAGS_HEAPTYPE)) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Immortal heap types are allowed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Immortal heap types are fine. They either go through the The assumption in the assert is that static types are immortal. We already assert that elsewhere (in We could also write: if (_Py_IsImmortal(type)) {
return;
}
assert(_PyType_HasFeature(type, Py_TPFLAGS_HEAPTYPE)); That seems a bit less robust to me in cases where the assumptions are violated, but also fine. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Either works. I think they equivalent in terms of checking assumptions. |
||||||
assert(_Py_IsImmortal(type)); | ||||||
return; | ||||||
} | ||||||
|
||||||
_PyThreadStateImpl *tstate = (_PyThreadStateImpl *)_PyThreadState_GET(); | ||||||
PyHeapTypeObject *ht = (PyHeapTypeObject *)type; | ||||||
|
||||||
// Unsigned comparison so that `ht_id=-1` is treated as out-of-bounds. | ||||||
Py_ssize_t ht_id = ht->_ht_id; | ||||||
if ((size_t)ht_id >= (size_t)tstate->types.size) { | ||||||
Py_DECREF(type); | ||||||
} | ||||||
else { | ||||||
# ifdef Py_REF_DEBUG | ||||||
_Py_DECREF_DecRefTotal(); | ||||||
# endif | ||||||
_Py_DECREF_STAT_INC(); | ||||||
tstate->types.refcounts[ht_id]--; | ||||||
} | ||||||
#endif | ||||||
} | ||||||
|
||||||
/* Inline functions trading binary compatibility for speed: | ||||||
_PyObject_Init() is the fast version of PyObject_Init(), and | ||||||
_PyObject_InitVar() is the fast version of PyObject_InitVar(). | ||||||
|
@@ -291,7 +358,7 @@ _PyObject_Init(PyObject *op, PyTypeObject *typeobj) | |||||
assert(op != NULL); | ||||||
Py_SET_TYPE(op, typeobj); | ||||||
assert(_PyType_HasFeature(typeobj, Py_TPFLAGS_HEAPTYPE) || _Py_IsImmortalLoose(typeobj)); | ||||||
Py_INCREF(typeobj); | ||||||
_Py_INCREF_TYPE(typeobj); | ||||||
_Py_NewReference(op); | ||||||
} | ||||||
|
||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
#ifndef Py_INTERNAL_TYPEID_H | ||
#define Py_INTERNAL_TYPEID_H | ||
#ifdef __cplusplus | ||
extern "C" { | ||
#endif | ||
|
||
#ifndef Py_BUILD_CORE | ||
# error "this header requires Py_BUILD_CORE define" | ||
#endif | ||
|
||
#ifdef Py_GIL_DISABLED | ||
|
||
// This contains code for allocating unique ids to heap type objects | ||
// and re-using those ids when the type is deallocated. | ||
// | ||
// The type ids are used to implement per-thread reference counts of | ||
// heap type objects to avoid contention on the reference count fields | ||
// of heap type objects. (Non-heap type objects are immortal, so contention | ||
// is not an issue.) | ||
// | ||
// Type id of -1 is used to indicate a type doesn't use thread-local | ||
// refcounting. | ||
// | ||
// Each entry implicitly represents a type id based on it's offset in the | ||
// table. Non-allocated entries form a free-list via the 'next' pointer. | ||
// Allocated entries store the corresponding PyTypeObject. | ||
typedef union _Py_type_id_entry { | ||
// Points to the next free type id, when part of the freelist | ||
union _Py_type_id_entry *next; | ||
|
||
// Stores the type object when the id is assigned | ||
PyHeapTypeObject *type; | ||
} _Py_type_id_entry; | ||
|
||
struct _Py_type_id_pool { | ||
PyMutex mutex; | ||
|
||
// combined table of types with allocated type ids and unallocated | ||
// type ids. | ||
_Py_type_id_entry *table; | ||
|
||
// Next entry to allocate inside 'table' or NULL | ||
_Py_type_id_entry *freelist; | ||
|
||
// size of 'table' | ||
Py_ssize_t size; | ||
}; | ||
|
||
// Assigns the next id from the pool of type ids. | ||
extern void _PyType_AssignId(PyHeapTypeObject *type); | ||
|
||
// Releases the allocated type id back to the pool. | ||
extern void _PyType_ReleaseId(PyHeapTypeObject *type); | ||
|
||
// Merges the thread-local reference counts into the corresponding types. | ||
extern void _PyType_MergeThreadLocalRefcounts(_PyThreadStateImpl *tstate); | ||
|
||
// Like _PyType_MergeThreadLocalRefcounts, but also frees the thread-local | ||
// array of refcounts. | ||
extern void _PyType_FinalizeThreadLocalRefcounts(_PyThreadStateImpl *tstate); | ||
|
||
// Frees the interpreter's pool of type ids. | ||
extern void _PyType_FinalizeIdPool(PyInterpreterState *interp); | ||
|
||
// Increfs the type, resizing the thread-local refcount array if necessary. | ||
PyAPI_FUNC(void) _PyType_IncrefSlow(PyHeapTypeObject *type); | ||
|
||
#endif /* Py_GIL_DISABLED */ | ||
|
||
#ifdef __cplusplus | ||
} | ||
#endif | ||
#endif /* !Py_INTERNAL_TYPEID_H */ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
In the free-threaded build, the reference counts for heap type objects are now | ||
partially stored in a distributed manner in per-thread arrays. This reduces | ||
contention on the heap type's reference count fields when creating or | ||
destroying instances of the same type from multiple threads concurrently. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems excessive if you are reusing them. Maybe use a 32 bit number?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also,
_ht_id
is not the clearest name.unique_id
perhaps (since we know this is a heap type, theht
prefix adds little).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More than 2^31-1 heap types seems unlikely, but not implausible. It also doesn't save any space, at least not for now, due to padding and alignment.