Skip to content
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-118926: Deferred reference counting GC changes for free threading #121318

Closed
wants to merge 28 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
34e6b9f
Fix a few wrong steals in bytecodes.c
Fidget-Spinner Jun 28, 2024
b3b6851
Deferred refcount GC
Fidget-Spinner Jun 28, 2024
a90a074
add a steal
Fidget-Spinner Jun 29, 2024
e82c2dc
Conversion function should have steal variant
Fidget-Spinner Jun 29, 2024
5df23bf
remove a steal
Fidget-Spinner Jun 29, 2024
6b63066
Fix double decref in error path
Fidget-Spinner Jun 30, 2024
634adcc
Revert "Fix double decref in error path"
Fidget-Spinner Jun 30, 2024
5b55760
Merge remote-tracking branch 'upstream/main' into deferred_gc
Fidget-Spinner Jul 3, 2024
8cb139f
Remove immortalize visitor
Fidget-Spinner Jul 3, 2024
c260fc9
fix the JIT builds
Fidget-Spinner Jul 3, 2024
700c2fd
fix buildbot bug
Fidget-Spinner Jul 3, 2024
cde931d
don't deref NULL
Fidget-Spinner Jul 3, 2024
4e59420
Remove steal conversion, add list_fromstackrefsteal
Fidget-Spinner Jul 3, 2024
98f9c0f
Silence warnings
Fidget-Spinner Jul 3, 2024
32aaf2b
Fix error case in CALL
Fidget-Spinner Jul 3, 2024
41c6218
remove all temporary immortalization
Fidget-Spinner Jul 3, 2024
dca2ec3
Revert "remove all temporary immortalization"
Fidget-Spinner Jul 3, 2024
39e8a56
Apply suggestions
Fidget-Spinner Jul 4, 2024
3e14b1c
fix non-free-threaded
Fidget-Spinner Jul 4, 2024
2bfe017
Merge remote-tracking branch 'upstream/main' into deferred_gc
Fidget-Spinner Jul 9, 2024
07f78ce
fix bytecodes.c to use arrays instead of pointers
Fidget-Spinner Jul 9, 2024
34b047e
Merge remote-tracking branch 'upstream/main' into deferred_gc
Fidget-Spinner Jul 18, 2024
f4b4022
Automatically flush to stack new
Fidget-Spinner Jul 18, 2024
4c0afa1
lint
Fidget-Spinner Jul 18, 2024
71aed5c
Simpler implementation
Fidget-Spinner Jul 18, 2024
1d19a97
reduce diff
Fidget-Spinner Jul 18, 2024
2b383b1
reduce diff
Fidget-Spinner Jul 18, 2024
2d2cd7c
reduce diff again (sorry)
Fidget-Spinner Jul 18, 2024
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
2 changes: 1 addition & 1 deletion Include/internal/pycore_ceval.h
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ PyAPI_FUNC(PyObject *)_PyEval_MatchClass(PyThreadState *tstate, PyObject *subjec
PyAPI_FUNC(PyObject *)_PyEval_MatchKeys(PyThreadState *tstate, PyObject *map, PyObject *keys);
PyAPI_FUNC(int) _PyEval_UnpackIterableStackRef(PyThreadState *tstate, _PyStackRef v, int argcnt, int argcntafter, _PyStackRef *sp);
PyAPI_FUNC(void) _PyEval_FrameClearAndPop(PyThreadState *tstate, _PyInterpreterFrame *frame);
PyAPI_FUNC(PyObject **) _PyObjectArray_FromStackRefArray(_PyStackRef *input, Py_ssize_t nargs, PyObject **scratch);
PyAPI_FUNC(PyObject **) _PyObjectArray_FromStackRefArrayBorrow(_PyStackRef *input, Py_ssize_t nargs, PyObject **scratch);

PyAPI_FUNC(void) _PyObjectArray_Free(PyObject **array, PyObject **scratch);

Expand Down
24 changes: 24 additions & 0 deletions Include/internal/pycore_frame.h
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,13 @@ static inline void _PyFrame_Copy(_PyInterpreterFrame *src, _PyInterpreterFrame *
// Don't leave a dangling pointer to the old frame when creating generators
// and coroutines:
dest->previous = NULL;

#ifdef Py_GIL_DISABLED
PyCodeObject *co = (PyCodeObject *)dest->f_executable;
for (int i = stacktop; i < co->co_nlocalsplus + co->co_stacksize; i++) {
dest->localsplus[i] = PyStackRef_NULL;
}
#endif
}

/* Consumes reference to func and locals.
Expand All @@ -153,6 +160,16 @@ _PyFrame_Initialize(
for (int i = null_locals_from; i < code->co_nlocalsplus; i++) {
frame->localsplus[i] = PyStackRef_NULL;
}

#ifdef Py_GIL_DISABLED
// On GIL disabled, we walk the entire stack in GC. Since stacktop
// is not always in sync with the real stack pointer, we have
// no choice but to traverse the entire stack.
// This just makes sure we don't pass the GC invalid stack values.
for (int i = code->co_nlocalsplus; i < code->co_nlocalsplus + code->co_stacksize; i++) {
frame->localsplus[i] = PyStackRef_NULL;
}
#endif
}

/* Gets the pointer to the locals array
Expand Down Expand Up @@ -314,6 +331,13 @@ _PyFrame_PushTrampolineUnchecked(PyThreadState *tstate, PyCodeObject *code, int
frame->instr_ptr = _PyCode_CODE(code);
frame->owner = FRAME_OWNED_BY_THREAD;
frame->return_offset = 0;

#ifdef Py_GIL_DISABLED
assert(code->co_nlocalsplus == 0);
for (int i = 0; i < code->co_stacksize; i++) {
frame->localsplus[i] = PyStackRef_NULL;
}
#endif
return frame;
}

Expand Down
3 changes: 3 additions & 0 deletions Include/internal/pycore_gc.h
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,9 @@ extern void _Py_RunGC(PyThreadState *tstate);
extern void _PyGC_ImmortalizeDeferredObjects(PyInterpreterState *interp);
#endif

// Functions to clear generator frames
extern int _PyGC_VisitFrameStack(struct _PyInterpreterFrame *frame, visitproc visit, void *arg);

#ifdef __cplusplus
}
#endif
Expand Down
3 changes: 3 additions & 0 deletions Include/internal/pycore_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ extern "C" {
#endif

#include "pycore_freelist.h" // _PyFreeListState
#include "pycore_stackref.h" // _PyStackRef

PyAPI_FUNC(PyObject*) _PyList_Extend(PyListObject *, PyObject *);
extern void _PyList_DebugMallocStats(FILE *out);
Expand Down Expand Up @@ -59,6 +60,8 @@ typedef struct {
} _PyListIterObject;

PyAPI_FUNC(PyObject *)_PyList_FromArraySteal(PyObject *const *src, Py_ssize_t n);
PyAPI_FUNC(PyObject *)_PyList_FromStackRefSteal(const _PyStackRef *src, Py_ssize_t n);


#ifdef __cplusplus
}
Expand Down
8 changes: 5 additions & 3 deletions Include/internal/pycore_stackref.h
Original file line number Diff line number Diff line change
Expand Up @@ -142,15 +142,16 @@ _PyStackRef_FromPyObjectSteal(PyObject *obj)


// Converts a PyObject * to a PyStackRef, with a new reference
// IMPORTANT: The result of this operation must be immediately assigned to localsplus.
// There must be no interfering Py_DECREF calls or such between this operation and that.
#ifdef Py_GIL_DISABLED
static inline _PyStackRef
PyStackRef_FromPyObjectNew(PyObject *obj)
{
// Make sure we don't take an already tagged value.
assert(((uintptr_t)obj & Py_TAG_BITS) == 0);
assert(obj != NULL);
// TODO (gh-117139): Add deferred objects later.
if (_Py_IsImmortal(obj)) {
if (_Py_IsImmortal(obj) || _PyObject_HasDeferredRefcount(obj)) {
return (_PyStackRef){ .bits = (uintptr_t)obj | Py_TAG_DEFERRED };
}
else {
Expand Down Expand Up @@ -219,7 +220,8 @@ PyStackRef_DUP(_PyStackRef stackref)
{
if (PyStackRef_IsDeferred(stackref)) {
assert(PyStackRef_IsNull(stackref) ||
_Py_IsImmortal(PyStackRef_AsPyObjectBorrow(stackref)));
_Py_IsImmortal(PyStackRef_AsPyObjectBorrow(stackref)) ||
_PyObject_HasDeferredRefcount(PyStackRef_AsPyObjectBorrow(stackref)));
return stackref;
}
Py_INCREF(PyStackRef_AsPyObjectBorrow(stackref));
Expand Down
23 changes: 23 additions & 0 deletions Objects/listobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -3203,6 +3203,29 @@ _PyList_FromArraySteal(PyObject *const *src, Py_ssize_t n)
return (PyObject *)list;
}

PyObject *
_PyList_FromStackRefSteal(const _PyStackRef *src, Py_ssize_t n)
{
if (n == 0) {
return PyList_New(0);
}

PyListObject *list = (PyListObject *)PyList_New(n);
if (list == NULL) {
for (Py_ssize_t i = 0; i < n; i++) {
PyStackRef_CLOSE(src[i]);
}
return NULL;
}

PyObject **dst = list->ob_item;
for (Py_ssize_t i = 0; i < n; i++) {
dst[i] = PyStackRef_AsPyObjectSteal(src[i]);
}

return (PyObject *)list;
}

/*[clinic input]
list.index

Expand Down
7 changes: 0 additions & 7 deletions Objects/object.c
Original file line number Diff line number Diff line change
Expand Up @@ -2432,13 +2432,6 @@ _PyObject_SetDeferredRefcount(PyObject *op)
assert(_Py_IsOwnedByCurrentThread(op));
assert(op->ob_ref_shared == 0);
_PyObject_SET_GC_BITS(op, _PyGC_BITS_DEFERRED);
PyInterpreterState *interp = _PyInterpreterState_GET();
Copy link
Contributor

Choose a reason for hiding this comment

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

🎉

if (_Py_atomic_load_int_relaxed(&interp->gc.immortalize) == 1) {
// gh-117696: immortalize objects instead of using deferred reference
// counting for now.
_Py_SetImmortal(op);
return;
}
op->ob_ref_local += 1;
op->ob_ref_shared = _Py_REF_QUEUED;
#endif
Expand Down
44 changes: 20 additions & 24 deletions Python/bytecodes.c
Original file line number Diff line number Diff line change
Expand Up @@ -843,12 +843,11 @@ dummy_func(
}

inst(STORE_SUBSCR_DICT, (unused/1, value, dict_st, sub_st -- )) {
PyObject *sub = PyStackRef_AsPyObjectBorrow(sub_st);
PyObject *dict = PyStackRef_AsPyObjectBorrow(dict_st);

DEOPT_IF(!PyDict_CheckExact(dict));
STAT_INC(STORE_SUBSCR, hit);
int err = _PyDict_SetItem_Take2((PyDictObject *)dict, sub, PyStackRef_AsPyObjectSteal(value));
int err = _PyDict_SetItem_Take2((PyDictObject *)dict, PyStackRef_AsPyObjectSteal(sub_st), PyStackRef_AsPyObjectSteal(value));
PyStackRef_CLOSE(dict_st);
ERROR_IF(err, error);
}
Expand Down Expand Up @@ -1767,7 +1766,7 @@ dummy_func(
}

inst(BUILD_STRING, (pieces[oparg] -- str)) {
STACKREFS_TO_PYOBJECTS(pieces, oparg, pieces_o);
STACKREFS_TO_PYOBJECTS_BORROW(pieces, oparg, pieces_o);
if (CONVERSION_FAILED(pieces_o)) {
DECREF_INPUTS();
ERROR_IF(true, error);
Expand All @@ -1786,13 +1785,7 @@ dummy_func(
}

inst(BUILD_LIST, (values[oparg] -- list)) {
STACKREFS_TO_PYOBJECTS(values, oparg, values_o);
if (CONVERSION_FAILED(values_o)) {
DECREF_INPUTS();
ERROR_IF(true, error);
}
PyObject *list_o = _PyList_FromArraySteal(values_o, oparg);
STACKREFS_TO_PYOBJECTS_CLEANUP(values_o);
PyObject *list_o = _PyList_FromStackRefSteal(values, oparg);
ERROR_IF(list_o == NULL, error);
list = PyStackRef_FromPyObjectSteal(list_o);
}
Expand Down Expand Up @@ -1832,11 +1825,11 @@ dummy_func(
}
int err = 0;
for (int i = 0; i < oparg; i++) {
PyObject *item = PyStackRef_AsPyObjectSteal(values[i]);
PyObject *item = PyStackRef_AsPyObjectBorrow(values[i]);
if (err == 0) {
err = PySet_Add(set_o, item);
}
Py_DECREF(item);
PyStackRef_CLOSE(values[i]);
}
if (err != 0) {
Py_DECREF(set_o);
Expand All @@ -1846,7 +1839,7 @@ dummy_func(
}

inst(BUILD_MAP, (values[oparg*2] -- map)) {
STACKREFS_TO_PYOBJECTS(values, oparg*2, values_o);
STACKREFS_TO_PYOBJECTS_BORROW(values, oparg*2, values_o);
if (CONVERSION_FAILED(values_o)) {
DECREF_INPUTS();
ERROR_IF(true, error);
Expand Down Expand Up @@ -1889,7 +1882,7 @@ dummy_func(

assert(PyTuple_CheckExact(keys_o));
assert(PyTuple_GET_SIZE(keys_o) == (Py_ssize_t)oparg);
STACKREFS_TO_PYOBJECTS(values, oparg, values_o);
STACKREFS_TO_PYOBJECTS_BORROW(values, oparg, values_o);
if (CONVERSION_FAILED(values_o)) {
DECREF_INPUTS();
ERROR_IF(true, error);
Expand Down Expand Up @@ -3103,7 +3096,7 @@ dummy_func(

inst(LOAD_SPECIAL, (owner -- attr, self_or_null)) {
assert(oparg <= SPECIAL_MAX);
PyObject *owner_o = PyStackRef_AsPyObjectSteal(owner);
PyObject *owner_o = PyStackRef_AsPyObjectBorrow(owner);
PyObject *name = _Py_SpecialMethods[oparg].name;
PyObject *self_or_null_o;
attr = PyStackRef_FromPyObjectSteal(_PyObject_LookupSpecialMethod(owner_o, name, &self_or_null_o));
Expand Down Expand Up @@ -3381,9 +3374,12 @@ dummy_func(
DISPATCH_INLINED(new_frame);
}
/* Callable is not a normal Python function */
STACKREFS_TO_PYOBJECTS(args, total_args, args_o);
STACKREFS_TO_PYOBJECTS_BORROW(args, total_args, args_o);
if (CONVERSION_FAILED(args_o)) {
DECREF_INPUTS();
PyStackRef_CLOSE(callable);
for (int i = 0; i < total_args; i++) {
PyStackRef_CLOSE(args[i]);
}
ERROR_IF(true, error);
}
PyObject *res_o = PyObject_Vectorcall(
Expand Down Expand Up @@ -3513,7 +3509,7 @@ dummy_func(
total_args++;
}
/* Callable is not a normal Python function */
STACKREFS_TO_PYOBJECTS(args, total_args, args_o);
STACKREFS_TO_PYOBJECTS_BORROW(args, total_args, args_o);
if (CONVERSION_FAILED(args_o)) {
DECREF_INPUTS();
ERROR_IF(true, error);
Expand Down Expand Up @@ -3747,7 +3743,7 @@ dummy_func(
PyTypeObject *tp = (PyTypeObject *)callable_o;
DEOPT_IF(tp->tp_vectorcall == NULL);
STAT_INC(CALL, hit);
STACKREFS_TO_PYOBJECTS(args, total_args, args_o);
STACKREFS_TO_PYOBJECTS_BORROW(args, total_args, args_o);
if (CONVERSION_FAILED(args_o)) {
DECREF_INPUTS();
ERROR_IF(true, error);
Expand Down Expand Up @@ -3817,7 +3813,7 @@ dummy_func(
STAT_INC(CALL, hit);
PyCFunction cfunc = PyCFunction_GET_FUNCTION(callable_o);
/* res = func(self, args, nargs) */
STACKREFS_TO_PYOBJECTS(args, total_args, args_o);
STACKREFS_TO_PYOBJECTS_BORROW(args, total_args, args_o);
if (CONVERSION_FAILED(args_o)) {
DECREF_INPUTS();
ERROR_IF(true, error);
Expand Down Expand Up @@ -3861,7 +3857,7 @@ dummy_func(
(PyCFunctionFastWithKeywords)(void(*)(void))
PyCFunction_GET_FUNCTION(callable_o);

STACKREFS_TO_PYOBJECTS(args, total_args, args_o);
STACKREFS_TO_PYOBJECTS_BORROW(args, total_args, args_o);
if (CONVERSION_FAILED(args_o)) {
DECREF_INPUTS();
ERROR_IF(true, error);
Expand Down Expand Up @@ -4025,7 +4021,7 @@ dummy_func(
PyCFunctionFastWithKeywords cfunc =
(PyCFunctionFastWithKeywords)(void(*)(void))meth->ml_meth;

STACKREFS_TO_PYOBJECTS(args, nargs, args_o);
STACKREFS_TO_PYOBJECTS_BORROW(args, nargs, args_o);
if (CONVERSION_FAILED(args_o)) {
DECREF_INPUTS();
ERROR_IF(true, error);
Expand Down Expand Up @@ -4106,7 +4102,7 @@ dummy_func(
(PyCFunctionFast)(void(*)(void))meth->ml_meth;
int nargs = total_args - 1;

STACKREFS_TO_PYOBJECTS(args, nargs, args_o);
STACKREFS_TO_PYOBJECTS_BORROW(args, nargs, args_o);
if (CONVERSION_FAILED(args_o)) {
DECREF_INPUTS();
ERROR_IF(true, error);
Expand Down Expand Up @@ -4190,7 +4186,7 @@ dummy_func(
DISPATCH_INLINED(new_frame);
}
/* Callable is not a normal Python function */
STACKREFS_TO_PYOBJECTS(args, total_args, args_o);
STACKREFS_TO_PYOBJECTS_BORROW(args, total_args, args_o);
if (CONVERSION_FAILED(args_o)) {
DECREF_INPUTS();
ERROR_IF(true, error);
Expand Down
2 changes: 1 addition & 1 deletion Python/ceval.c
Original file line number Diff line number Diff line change
Expand Up @@ -698,7 +698,7 @@ extern void _PyUOpPrint(const _PyUOpInstruction *uop);


PyObject **
_PyObjectArray_FromStackRefArray(_PyStackRef *input, Py_ssize_t nargs, PyObject **scratch)
_PyObjectArray_FromStackRefArrayBorrow(_PyStackRef *input, Py_ssize_t nargs, PyObject **scratch)
{
PyObject **result;
if (nargs > MAX_STACKREF_SCRATCH) {
Expand Down
6 changes: 3 additions & 3 deletions Python/ceval_macros.h
Original file line number Diff line number Diff line change
Expand Up @@ -455,12 +455,12 @@ do { \
#define MAX_STACKREF_SCRATCH 10

#ifdef Py_GIL_DISABLED
#define STACKREFS_TO_PYOBJECTS(ARGS, ARG_COUNT, NAME) \
#define STACKREFS_TO_PYOBJECTS_BORROW(ARGS, ARG_COUNT, NAME) \
/* +1 because vectorcall might use -1 to write self */ \
PyObject *NAME##_temp[MAX_STACKREF_SCRATCH+1]; \
PyObject **NAME = _PyObjectArray_FromStackRefArray(ARGS, ARG_COUNT, NAME##_temp + 1);
PyObject **NAME = _PyObjectArray_FromStackRefArrayBorrow(ARGS, ARG_COUNT, NAME##_temp + 1);
#else
#define STACKREFS_TO_PYOBJECTS(ARGS, ARG_COUNT, NAME) \
#define STACKREFS_TO_PYOBJECTS_BORROW(ARGS, ARG_COUNT, NAME) \
PyObject **NAME = (PyObject **)ARGS; \
assert(NAME != NULL);
#endif
Expand Down
Loading
Loading