Skip to content

Commit

Permalink
gh-123923: Defer refcounting for f_funcobj in _PyInterpreterFrame (…
Browse files Browse the repository at this point in the history 
…#124026)

Use a `_PyStackRef` and defer the reference to `f_funcobj` when
possible. This avoids some reference count contention in the common case
of executing the same code object from multiple threads concurrently in
the free-threaded build.
  • Loading branch information
@colesbury
colesbury committed Sep 24, 2024
1 parent d3c76df commit f4997bb
Show file tree
Hide file tree
Showing 17 changed files with 143 additions and 137 deletions.
26 changes: 17 additions & 9 deletions Include/internal/pycore_frame.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ enum _frameowner {
typedef struct _PyInterpreterFrame {
_PyStackRef f_executable; /* Deferred or strong reference (code object or None) */
struct _PyInterpreterFrame *previous;
PyObject *f_funcobj; /* Strong reference. Only valid if not on C stack */
_PyStackRef f_funcobj; /* Deferred or strong reference. Only valid if not on C stack */
PyObject *f_globals; /* Borrowed reference. Only valid if not on C stack */
PyObject *f_builtins; /* Borrowed reference. Only valid if not on C stack */
PyObject *f_locals; /* Strong reference, may be NULL. Only valid if not on C stack */
Expand All @@ -84,6 +84,12 @@ static inline PyCodeObject *_PyFrame_GetCode(_PyInterpreterFrame *f) {
return (PyCodeObject *)executable;
}

static inline PyFunctionObject *_PyFrame_GetFunction(_PyInterpreterFrame *f) {
PyObject *func = PyStackRef_AsPyObjectBorrow(f->f_funcobj);
assert(PyFunction_Check(func));
return (PyFunctionObject *)func;
}

static inline _PyStackRef *_PyFrame_Stackbase(_PyInterpreterFrame *f) {
return (f->localsplus + _PyFrame_GetCode(f)->co_nlocalsplus);
}
Expand Down Expand Up @@ -144,14 +150,15 @@ static inline void _PyFrame_Copy(_PyInterpreterFrame *src, _PyInterpreterFrame *
*/
static inline void
_PyFrame_Initialize(
_PyInterpreterFrame *frame, PyFunctionObject *func,
_PyInterpreterFrame *frame, _PyStackRef func,
PyObject *locals, PyCodeObject *code, int null_locals_from, _PyInterpreterFrame *previous)
{
frame->previous = previous;
frame->f_funcobj = (PyObject *)func;
frame->f_funcobj = func;
frame->f_executable = PyStackRef_FromPyObjectNew(code);
frame->f_builtins = func->func_builtins;
frame->f_globals = func->func_globals;
PyFunctionObject *func_obj = (PyFunctionObject *)PyStackRef_AsPyObjectBorrow(func);
frame->f_builtins = func_obj->func_builtins;
frame->f_globals = func_obj->func_globals;
frame->f_locals = locals;
frame->stackpointer = frame->localsplus + code->co_nlocalsplus;
frame->frame_obj = NULL;
Expand Down Expand Up @@ -300,10 +307,11 @@ PyAPI_FUNC(void) _PyThreadState_PopFrame(PyThreadState *tstate, _PyInterpreterFr
* Must be guarded by _PyThreadState_HasStackSpace()
* Consumes reference to func. */
static inline _PyInterpreterFrame *
_PyFrame_PushUnchecked(PyThreadState *tstate, PyFunctionObject *func, int null_locals_from, _PyInterpreterFrame * previous)
_PyFrame_PushUnchecked(PyThreadState *tstate, _PyStackRef func, int null_locals_from, _PyInterpreterFrame * previous)
{
CALL_STAT_INC(frames_pushed);
PyCodeObject *code = (PyCodeObject *)func->func_code;
PyFunctionObject *func_obj = (PyFunctionObject *)PyStackRef_AsPyObjectBorrow(func);
PyCodeObject *code = (PyCodeObject *)func_obj->func_code;
_PyInterpreterFrame *new_frame = (_PyInterpreterFrame *)tstate->datastack_top;
tstate->datastack_top += code->co_framesize;
assert(tstate->datastack_top < tstate->datastack_limit);
Expand All @@ -321,7 +329,7 @@ _PyFrame_PushTrampolineUnchecked(PyThreadState *tstate, PyCodeObject *code, int
tstate->datastack_top += code->co_framesize;
assert(tstate->datastack_top < tstate->datastack_limit);
frame->previous = previous;
frame->f_funcobj = Py_None;
frame->f_funcobj = PyStackRef_None;
frame->f_executable = PyStackRef_FromPyObjectNew(code);
#ifdef Py_DEBUG
frame->f_builtins = NULL;
Expand All @@ -345,7 +353,7 @@ _PyFrame_PushTrampolineUnchecked(PyThreadState *tstate, PyCodeObject *code, int
}

PyAPI_FUNC(_PyInterpreterFrame *)
_PyEvalFramePushAndInit(PyThreadState *tstate, PyFunctionObject *func,
_PyEvalFramePushAndInit(PyThreadState *tstate, _PyStackRef func,
PyObject *locals, _PyStackRef const* args,
size_t argcount, PyObject *kwnames,
_PyInterpreterFrame *previous);
Expand Down
11 changes: 11 additions & 0 deletions Include/internal/pycore_gc.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,17 @@ union _PyStackRef;
extern int _PyGC_VisitFrameStack(struct _PyInterpreterFrame *frame, visitproc visit, void *arg);
extern int _PyGC_VisitStackRef(union _PyStackRef *ref, visitproc visit, void *arg);

// Like Py_VISIT but for _PyStackRef fields
#define _Py_VISIT_STACKREF(ref) \
do { \
if (!PyStackRef_IsNull(ref)) { \
int vret = _PyGC_VisitStackRef(&(ref), visit, arg); \
if (vret) \
return vret; \
} \
} while (0)


#ifdef __cplusplus
}
#endif
Expand Down
6 changes: 3 additions & 3 deletions Modules/_testinternalcapi.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -681,13 +681,13 @@ set_eval_frame_default(PyObject *self, PyObject *Py_UNUSED(args))
static PyObject *
record_eval(PyThreadState *tstate, struct _PyInterpreterFrame *f, int exc)
{
if (PyFunction_Check(f->f_funcobj)) {
if (PyStackRef_FunctionCheck(f->f_funcobj)) {
PyFunctionObject *func = _PyFrame_GetFunction(f);
PyObject *module = _get_current_module();
assert(module != NULL);
module_state *state = get_module_state(module);
Py_DECREF(module);
int res = PyList_Append(state->record_list,
((PyFunctionObject *)f->f_funcobj)->func_name);
int res = PyList_Append(state->record_list, func->func_name);
if (res < 0) {
return NULL;
}
Expand Down
9 changes: 5 additions & 4 deletions Objects/frameobject.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1634,7 +1634,7 @@ frame_dealloc(PyFrameObject *f)
/* Kill all local variables including specials, if we own them */
if (f->f_frame == frame && frame->owner == FRAME_OWNED_BY_FRAME_OBJECT) {
PyStackRef_CLEAR(frame->f_executable);
Py_CLEAR(frame->f_funcobj);
PyStackRef_CLEAR(frame->f_funcobj);
Py_CLEAR(frame->f_locals);
_PyStackRef *locals = _PyFrame_GetLocalsArray(frame);
_PyStackRef *sp = frame->stackpointer;
Expand Down Expand Up @@ -1790,7 +1790,7 @@ static void
init_frame(_PyInterpreterFrame *frame, PyFunctionObject *func, PyObject *locals)
{
PyCodeObject *code = (PyCodeObject *)func->func_code;
_PyFrame_Initialize(frame, (PyFunctionObject*)Py_NewRef(func),
_PyFrame_Initialize(frame, PyStackRef_FromPyObjectNew(func),
Py_XNewRef(locals), code, 0, NULL);
}

Expand Down Expand Up @@ -1861,14 +1861,15 @@ frame_init_get_vars(_PyInterpreterFrame *frame)
PyCodeObject *co = _PyFrame_GetCode(frame);
int lasti = _PyInterpreterFrame_LASTI(frame);
if (!(lasti < 0 && _PyCode_CODE(co)->op.code == COPY_FREE_VARS
&& PyFunction_Check(frame->f_funcobj)))
&& PyStackRef_FunctionCheck(frame->f_funcobj)))
{
/* Free vars are initialized */
return;
}

/* Free vars have not been initialized -- Do that */
PyObject *closure = ((PyFunctionObject *)frame->f_funcobj)->func_closure;
PyFunctionObject *func = _PyFrame_GetFunction(frame);
PyObject *closure = func->func_closure;
int offset = PyUnstable_Code_GetFirstFree(co);
for (int i = 0; i < co->co_nfreevars; ++i) {
PyObject *o = PyTuple_GET_ITEM(closure, i);
Expand Down
5 changes: 1 addition & 4 deletions Objects/genobject.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,7 @@ gen_traverse(PyGenObject *gen, visitproc visit, void *arg)
else {
// We still need to visit the code object when the frame is cleared to
// ensure that it's kept alive if the reference is deferred.
int err = _PyGC_VisitStackRef(&gen->gi_iframe.f_executable, visit, arg);
if (err) {
return err;
}
_Py_VISIT_STACKREF(gen->gi_iframe.f_executable);
}
/* No need to visit cr_origin, because it's just tuples/str/int, so can't
participate in a reference cycle. */
Expand Down
4 changes: 2 additions & 2 deletions Objects/typevarobject.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -372,10 +372,10 @@ caller(void)
if (f == NULL) {
Py_RETURN_NONE;
}
if (f == NULL || f->f_funcobj == NULL) {
if (f == NULL || PyStackRef_IsNull(f->f_funcobj)) {
Py_RETURN_NONE;
}
PyObject *r = PyFunction_GetModule(f->f_funcobj);
PyObject *r = PyFunction_GetModule(PyStackRef_AsPyObjectBorrow(f->f_funcobj));
if (!r) {
PyErr_Clear();
Py_RETURN_NONE;
Expand Down
45 changes: 21 additions & 24 deletions Python/bytecodes.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -808,14 +808,13 @@ dummy_func(
assert(code->co_argcount == 2);
DEOPT_IF(!_PyThreadState_HasStackSpace(tstate, code->co_framesize));
STAT_INC(BINARY_SUBSCR, hit);
Py_INCREF(getitem);
}

op(_BINARY_SUBSCR_INIT_CALL, (container, sub -- new_frame: _PyInterpreterFrame* )) {
PyTypeObject *tp = Py_TYPE(PyStackRef_AsPyObjectBorrow(container));
PyHeapTypeObject *ht = (PyHeapTypeObject *)tp;
PyObject *getitem = ht->_spec_cache.getitem;
new_frame = _PyFrame_PushUnchecked(tstate, (PyFunctionObject *)getitem, 2, frame);
new_frame = _PyFrame_PushUnchecked(tstate, PyStackRef_FromPyObjectNew(getitem), 2, frame);
SYNC_SP();
new_frame->localsplus[0] = container;
new_frame->localsplus[1] = sub;
Expand Down Expand Up @@ -1666,8 +1665,9 @@ dummy_func(
inst(COPY_FREE_VARS, (--)) {
/* Copy closure variables to free variables */
PyCodeObject *co = _PyFrame_GetCode(frame);
assert(PyFunction_Check(frame->f_funcobj));
PyObject *closure = ((PyFunctionObject *)frame->f_funcobj)->func_closure;
assert(PyStackRef_FunctionCheck(frame->f_funcobj));
PyFunctionObject *func = (PyFunctionObject *)PyStackRef_AsPyObjectBorrow(frame->f_funcobj);
PyObject *closure = func->func_closure;
assert(oparg == co->co_nfreevars);
int offset = co->co_nlocalsplus - oparg;
for (int i = 0; i < oparg; ++i) {
Expand Down Expand Up @@ -2170,8 +2170,7 @@ dummy_func(
DEOPT_IF(code->co_argcount != 1);
DEOPT_IF(!_PyThreadState_HasStackSpace(tstate, code->co_framesize));
STAT_INC(LOAD_ATTR, hit);
Py_INCREF(fget);
new_frame = _PyFrame_PushUnchecked(tstate, f, 1, frame);
new_frame = _PyFrame_PushUnchecked(tstate, PyStackRef_FromPyObjectNew(fget), 1, frame);
new_frame->localsplus[0] = owner;
}

Expand Down Expand Up @@ -2202,8 +2201,8 @@ dummy_func(
STAT_INC(LOAD_ATTR, hit);

PyObject *name = GETITEM(FRAME_CO_NAMES, oparg >> 1);
Py_INCREF(f);
_PyInterpreterFrame *new_frame = _PyFrame_PushUnchecked(tstate, f, 2, frame);
_PyInterpreterFrame *new_frame = _PyFrame_PushUnchecked(
tstate, PyStackRef_FromPyObjectNew(f), 2, frame);
// Manipulate stack directly because we exit with DISPATCH_INLINED().
STACK_SHRINK(1);
new_frame->localsplus[0] = owner;
Expand Down Expand Up @@ -3251,7 +3250,7 @@ dummy_func(
int code_flags = ((PyCodeObject*)PyFunction_GET_CODE(callable_o))->co_flags;
PyObject *locals = code_flags & CO_OPTIMIZED ? NULL : Py_NewRef(PyFunction_GET_GLOBALS(callable_o));
_PyInterpreterFrame *new_frame = _PyEvalFramePushAndInit(
tstate, (PyFunctionObject *)PyStackRef_AsPyObjectSteal(callable), locals,
tstate, callable, locals,
args, total_args, NULL, frame
);
// Manipulate stack directly since we leave using DISPATCH_INLINED().
Expand Down Expand Up @@ -3340,7 +3339,7 @@ dummy_func(
int code_flags = ((PyCodeObject*)PyFunction_GET_CODE(callable_o))->co_flags;
PyObject *locals = code_flags & CO_OPTIMIZED ? NULL : Py_NewRef(PyFunction_GET_GLOBALS(callable_o));
new_frame = _PyEvalFramePushAndInit(
tstate, (PyFunctionObject *)PyStackRef_AsPyObjectSteal(callable), locals,
tstate, callable, locals,
args, total_args, NULL, frame
);
// The frame has stolen all the arguments from the stack,
Expand Down Expand Up @@ -3475,11 +3474,9 @@ dummy_func(
}

replicate(5) pure op(_INIT_CALL_PY_EXACT_ARGS, (callable, self_or_null[1], args[oparg] -- new_frame: _PyInterpreterFrame*)) {
PyObject *callable_o = PyStackRef_AsPyObjectBorrow(callable);
int has_self = !PyStackRef_IsNull(self_or_null[0]);
STAT_INC(CALL, hit);
PyFunctionObject *func = (PyFunctionObject *)callable_o;
new_frame = _PyFrame_PushUnchecked(tstate, func, oparg + has_self, frame);
new_frame = _PyFrame_PushUnchecked(tstate, callable, oparg + has_self, frame);
_PyStackRef *first_non_self_local = new_frame->localsplus + has_self;
new_frame->localsplus[0] = self_or_null[0];
for (int i = 0; i < oparg; i++) {
Expand Down Expand Up @@ -3601,10 +3598,9 @@ dummy_func(
assert(_PyCode_CODE(_PyFrame_GetCode(shim))[0].op.code == EXIT_INIT_CHECK);
/* Push self onto stack of shim */
shim->localsplus[0] = PyStackRef_DUP(self);
PyFunctionObject *init_func = (PyFunctionObject *)PyStackRef_AsPyObjectSteal(init);
args[-1] = self;
init_frame = _PyEvalFramePushAndInit(
tstate, init_func, NULL, args-1, oparg+1, NULL, shim);
tstate, init, NULL, args-1, oparg+1, NULL, shim);
SYNC_SP();
if (init_frame == NULL) {
_PyEval_FrameClearAndPop(tstate, shim);
Expand Down Expand Up @@ -4080,7 +4076,7 @@ dummy_func(
int code_flags = ((PyCodeObject*)PyFunction_GET_CODE(callable_o))->co_flags;
PyObject *locals = code_flags & CO_OPTIMIZED ? NULL : Py_NewRef(PyFunction_GET_GLOBALS(callable_o));
_PyInterpreterFrame *new_frame = _PyEvalFramePushAndInit(
tstate, (PyFunctionObject *)PyStackRef_AsPyObjectSteal(callable), locals,
tstate, callable, locals,
args, positional_args, kwnames_o, frame
);
PyStackRef_CLOSE(kwnames);
Expand Down Expand Up @@ -4148,7 +4144,7 @@ dummy_func(
int code_flags = ((PyCodeObject*)PyFunction_GET_CODE(callable_o))->co_flags;
PyObject *locals = code_flags & CO_OPTIMIZED ? NULL : Py_NewRef(PyFunction_GET_GLOBALS(callable_o));
new_frame = _PyEvalFramePushAndInit(
tstate, (PyFunctionObject *)PyStackRef_AsPyObjectSteal(callable), locals,
tstate, callable, locals,
args, positional_args, kwnames_o, frame
);
PyStackRef_CLOSE(kwnames);
Expand Down Expand Up @@ -4332,9 +4328,9 @@ dummy_func(
int code_flags = ((PyCodeObject *)PyFunction_GET_CODE(func))->co_flags;
PyObject *locals = code_flags & CO_OPTIMIZED ? NULL : Py_NewRef(PyFunction_GET_GLOBALS(func));

_PyInterpreterFrame *new_frame = _PyEvalFramePushAndInit_Ex(tstate,
(PyFunctionObject *)PyStackRef_AsPyObjectSteal(func_st), locals,
nargs, callargs, kwargs, frame);
_PyInterpreterFrame *new_frame = _PyEvalFramePushAndInit_Ex(
tstate, func_st, locals,
nargs, callargs, kwargs, frame);
// Need to manually shrink the stack since we exit with DISPATCH_INLINED.
STACK_SHRINK(oparg + 3);
if (new_frame == NULL) {
Expand Down Expand Up @@ -4408,8 +4404,8 @@ dummy_func(
}

inst(RETURN_GENERATOR, (-- res)) {
assert(PyFunction_Check(frame->f_funcobj));
PyFunctionObject *func = (PyFunctionObject *)frame->f_funcobj;
assert(PyStackRef_FunctionCheck(frame->f_funcobj));
PyFunctionObject *func = (PyFunctionObject *)PyStackRef_AsPyObjectBorrow(frame->f_funcobj);
PyGenObject *gen = (PyGenObject *)_Py_MakeCoro(func);
if (gen == NULL) {
ERROR_NO_POP();
Expand Down Expand Up @@ -4771,8 +4767,9 @@ dummy_func(
}

tier2 op(_CHECK_FUNCTION, (func_version/2 -- )) {
assert(PyFunction_Check(frame->f_funcobj));
DEOPT_IF(((PyFunctionObject *)frame->f_funcobj)->func_version != func_version);
assert(PyStackRef_FunctionCheck(frame->f_funcobj));
PyFunctionObject *func = (PyFunctionObject *)PyStackRef_AsPyObjectBorrow(frame->f_funcobj);
DEOPT_IF(func->func_version != func_version);
}

/* Internal -- for testing executors */
Expand Down
Loading

0 comments on commit f4997bb

Please sign in to comment.