Skip to content

Commit db12634

Browse files
colesburycolesbury
committed
gh-123923: Defer refcounting for f_funcobj in _PyInterpreterFrame
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.
1 parent 6e06e01 commit db12634

File tree

15 files changed

+144
-138
lines changed

15 files changed

+144
-138
lines changed

‎Include/internal/pycore_frame.h‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ enum _frameowner {
6262
typedef struct _PyInterpreterFrame {
6363
_PyStackRef f_executable; /* Deferred or strong reference (code object or None) */
6464
struct _PyInterpreterFrame *previous;
65-
PyObject *f_funcobj; /* Strong reference. Only valid if not on C stack */
65+
_PyStackRef f_funcobj; /* Deferred or strong reference. Only valid if not on C stack */
6666
PyObject *f_globals; /* Borrowed reference. Only valid if not on C stack */
6767
PyObject *f_builtins; /* Borrowed reference. Only valid if not on C stack */
6868
PyObject *f_locals; /* Strong reference, may be NULL. Only valid if not on C stack */
@@ -144,14 +144,15 @@ static inline void _PyFrame_Copy(_PyInterpreterFrame *src, _PyInterpreterFrame *
144144
*/
145145
static inline void
146146
_PyFrame_Initialize(
147-
_PyInterpreterFrame *frame, PyFunctionObject *func,
147+
_PyInterpreterFrame *frame, _PyStackRef func,
148148
PyObject *locals, PyCodeObject *code, int null_locals_from, _PyInterpreterFrame *previous)
149149
{
150150
frame->previous = previous;
151-
frame->f_funcobj = (PyObject *)func;
151+
frame->f_funcobj = func;
152152
frame->f_executable = PyStackRef_FromPyObjectNew(code);
153-
frame->f_builtins = func->func_builtins;
154-
frame->f_globals = func->func_globals;
153+
PyFunctionObject *func_obj = (PyFunctionObject *)PyStackRef_AsPyObjectBorrow(func);
154+
frame->f_builtins = func_obj->func_builtins;
155+
frame->f_globals = func_obj->func_globals;
155156
frame->f_locals = locals;
156157
frame->stackpointer = frame->localsplus + code->co_nlocalsplus;
157158
frame->frame_obj = NULL;
@@ -300,10 +301,11 @@ PyAPI_FUNC(void) _PyThreadState_PopFrame(PyThreadState *tstate, _PyInterpreterFr
300301
* Must be guarded by _PyThreadState_HasStackSpace()
301302
* Consumes reference to func. */
302303
static inline _PyInterpreterFrame *
303-
_PyFrame_PushUnchecked(PyThreadState *tstate, PyFunctionObject *func, int null_locals_from, _PyInterpreterFrame * previous)
304+
_PyFrame_PushUnchecked(PyThreadState *tstate, _PyStackRef func, int null_locals_from, _PyInterpreterFrame * previous)
304305
{
305306
CALL_STAT_INC(frames_pushed);
306-
PyCodeObject *code = (PyCodeObject *)func->func_code;
307+
PyFunctionObject *func_obj = (PyFunctionObject *)PyStackRef_AsPyObjectBorrow(func);
308+
PyCodeObject *code = (PyCodeObject *)func_obj->func_code;
307309
_PyInterpreterFrame *new_frame = (_PyInterpreterFrame *)tstate->datastack_top;
308310
tstate->datastack_top += code->co_framesize;
309311
assert(tstate->datastack_top < tstate->datastack_limit);
@@ -321,7 +323,7 @@ _PyFrame_PushTrampolineUnchecked(PyThreadState *tstate, PyCodeObject *code, int
321323
tstate->datastack_top += code->co_framesize;
322324
assert(tstate->datastack_top < tstate->datastack_limit);
323325
frame->previous = previous;
324-
frame->f_funcobj = Py_None;
326+
frame->f_funcobj = PyStackRef_None;
325327
frame->f_executable = PyStackRef_FromPyObjectNew(code);
326328
#ifdef Py_DEBUG
327329
frame->f_builtins = NULL;
@@ -345,7 +347,7 @@ _PyFrame_PushTrampolineUnchecked(PyThreadState *tstate, PyCodeObject *code, int
345347
}
346348

347349
PyAPI_FUNC(_PyInterpreterFrame *)
348-
_PyEvalFramePushAndInit(PyThreadState *tstate, PyFunctionObject *func,
350+
_PyEvalFramePushAndInit(PyThreadState *tstate, _PyStackRef func,
349351
PyObject *locals, _PyStackRef const* args,
350352
size_t argcount, PyObject *kwnames,
351353
_PyInterpreterFrame *previous);

‎Include/internal/pycore_gc.h‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,17 @@ union _PyStackRef;
387387
extern int _PyGC_VisitFrameStack(struct _PyInterpreterFrame *frame, visitproc visit, void *arg);
388388
extern int _PyGC_VisitStackRef(union _PyStackRef *ref, visitproc visit, void *arg);
389389

390+
// Like Py_VISIT but for _PyStackRef fields
391+
#define _Py_VISIT_STACKREF(ref) \
392+
do { \
393+
if (!PyStackRef_IsNull(ref)) { \
394+
int vret = _PyGC_VisitStackRef(&(ref), visit, arg); \
395+
if (vret) \
396+
return vret; \
397+
} \
398+
} while (0)
399+
400+
390401
#ifdef __cplusplus
391402
}
392403
#endif

‎Modules/_testinternalcapi.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -681,13 +681,13 @@ set_eval_frame_default(PyObject *self, PyObject *Py_UNUSED(args))
681681
static PyObject *
682682
record_eval(PyThreadState *tstate, struct _PyInterpreterFrame *f, int exc)
683683
{
684-
if (PyFunction_Check(f->f_funcobj)) {
684+
if (PyStackRef_FunctionCheck(f->f_funcobj)) {
685+
PyFunctionObject *func = (PyFunctionObject *)PyStackRef_AsPyObjectBorrow(f->f_funcobj);
685686
PyObject *module = _get_current_module();
686687
assert(module != NULL);
687688
module_state *state = get_module_state(module);
688689
Py_DECREF(module);
689-
int res = PyList_Append(state->record_list,
690-
((PyFunctionObject *)f->f_funcobj)->func_name);
690+
int res = PyList_Append(state->record_list, func->func_name);
691691
if (res < 0) {
692692
return NULL;
693693
}

‎Objects/frameobject.c‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1634,7 +1634,7 @@ frame_dealloc(PyFrameObject *f)
16341634
/* Kill all local variables including specials, if we own them */
16351635
if (f->f_frame == frame && frame->owner == FRAME_OWNED_BY_FRAME_OBJECT) {
16361636
PyStackRef_CLEAR(frame->f_executable);
1637-
Py_CLEAR(frame->f_funcobj);
1637+
PyStackRef_CLEAR(frame->f_funcobj);
16381638
Py_CLEAR(frame->f_locals);
16391639
_PyStackRef *locals = _PyFrame_GetLocalsArray(frame);
16401640
_PyStackRef *sp = frame->stackpointer;
@@ -1790,7 +1790,7 @@ static void
17901790
init_frame(_PyInterpreterFrame *frame, PyFunctionObject *func, PyObject *locals)
17911791
{
17921792
PyCodeObject *code = (PyCodeObject *)func->func_code;
1793-
_PyFrame_Initialize(frame, (PyFunctionObject*)Py_NewRef(func),
1793+
_PyFrame_Initialize(frame, PyStackRef_FromPyObjectNew(func),
17941794
Py_XNewRef(locals), code, 0, NULL);
17951795
}
17961796

@@ -1861,14 +1861,15 @@ frame_init_get_vars(_PyInterpreterFrame *frame)
18611861
PyCodeObject *co = _PyFrame_GetCode(frame);
18621862
int lasti = _PyInterpreterFrame_LASTI(frame);
18631863
if (!(lasti < 0 && _PyCode_CODE(co)->op.code == COPY_FREE_VARS
1864-
&& PyFunction_Check(frame->f_funcobj)))
1864+
&& PyStackRef_FunctionCheck(frame->f_funcobj)))
18651865
{
18661866
/* Free vars are initialized */
18671867
return;
18681868
}
18691869

18701870
/* Free vars have not been initialized -- Do that */
1871-
PyObject *closure = ((PyFunctionObject *)frame->f_funcobj)->func_closure;
1871+
PyFunctionObject *func = (PyFunctionObject *)PyStackRef_AsPyObjectBorrow(frame->f_funcobj);
1872+
PyObject *closure = func->func_closure;
18721873
int offset = PyUnstable_Code_GetFirstFree(co);
18731874
for (int i = 0; i < co->co_nfreevars; ++i) {
18741875
PyObject *o = PyTuple_GET_ITEM(closure, i);

‎Objects/genobject.c‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,7 @@ gen_traverse(PyGenObject *gen, visitproc visit, void *arg)
5858
else {
5959
// We still need to visit the code object when the frame is cleared to
6060
// ensure that it's kept alive if the reference is deferred.
61-
int err = _PyGC_VisitStackRef(&gen->gi_iframe.f_executable, visit, arg);
62-
if (err) {
63-
return err;
64-
}
61+
_Py_VISIT_STACKREF(gen->gi_iframe.f_executable);
6562
}
6663
/* No need to visit cr_origin, because it's just tuples/str/int, so can't
6764
participate in a reference cycle. */

‎Objects/typevarobject.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -372,10 +372,10 @@ caller(void)
372372
if (f == NULL) {
373373
Py_RETURN_NONE;
374374
}
375-
if (f == NULL || f->f_funcobj == NULL) {
375+
if (f == NULL || PyStackRef_IsNull(f->f_funcobj)) {
376376
Py_RETURN_NONE;
377377
}
378-
PyObject *r = PyFunction_GetModule(f->f_funcobj);
378+
PyObject *r = PyFunction_GetModule(PyStackRef_AsPyObjectBorrow(f->f_funcobj));
379379
if (!r) {
380380
PyErr_Clear();
381381
Py_RETURN_NONE;

‎Python/bytecodes.c‎

Lines changed: 21 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -808,14 +808,13 @@ dummy_func(
808808
assert(code->co_argcount == 2);
809809
DEOPT_IF(!_PyThreadState_HasStackSpace(tstate, code->co_framesize));
810810
STAT_INC(BINARY_SUBSCR, hit);
811-
Py_INCREF(getitem);
812811
}
813812

814813
op(_BINARY_SUBSCR_INIT_CALL, (container, sub -- new_frame: _PyInterpreterFrame* )) {
815814
PyTypeObject *tp = Py_TYPE(PyStackRef_AsPyObjectBorrow(container));
816815
PyHeapTypeObject *ht = (PyHeapTypeObject *)tp;
817816
PyObject *getitem = ht->_spec_cache.getitem;
818-
new_frame = _PyFrame_PushUnchecked(tstate, (PyFunctionObject *)getitem, 2, frame);
817+
new_frame = _PyFrame_PushUnchecked(tstate, PyStackRef_FromPyObjectNew(getitem), 2, frame);
819818
SYNC_SP();
820819
new_frame->localsplus[0] = container;
821820
new_frame->localsplus[1] = sub;
@@ -1666,8 +1665,9 @@ dummy_func(
16661665
inst(COPY_FREE_VARS, (--)) {
16671666
/* Copy closure variables to free variables */
16681667
PyCodeObject *co = _PyFrame_GetCode(frame);
1669-
assert(PyFunction_Check(frame->f_funcobj));
1670-
PyObject *closure = ((PyFunctionObject *)frame->f_funcobj)->func_closure;
1668+
assert(PyStackRef_FunctionCheck(frame->f_funcobj));
1669+
PyFunctionObject *func = (PyFunctionObject *)PyStackRef_AsPyObjectBorrow(frame->f_funcobj);
1670+
PyObject *closure = func->func_closure;
16711671
assert(oparg == co->co_nfreevars);
16721672
int offset = co->co_nlocalsplus - oparg;
16731673
for (int i = 0; i < oparg; ++i) {
@@ -2170,8 +2170,7 @@ dummy_func(
21702170
DEOPT_IF(code->co_argcount != 1);
21712171
DEOPT_IF(!_PyThreadState_HasStackSpace(tstate, code->co_framesize));
21722172
STAT_INC(LOAD_ATTR, hit);
2173-
Py_INCREF(fget);
2174-
new_frame = _PyFrame_PushUnchecked(tstate, f, 1, frame);
2173+
new_frame = _PyFrame_PushUnchecked(tstate, PyStackRef_FromPyObjectNew(fget), 1, frame);
21752174
new_frame->localsplus[0] = owner;
21762175
}
21772176

@@ -2202,8 +2201,8 @@ dummy_func(
22022201
STAT_INC(LOAD_ATTR, hit);
22032202

22042203
PyObject *name = GETITEM(FRAME_CO_NAMES, oparg >> 1);
2205-
Py_INCREF(f);
2206-
_PyInterpreterFrame *new_frame = _PyFrame_PushUnchecked(tstate, f, 2, frame);
2204+
_PyInterpreterFrame *new_frame = _PyFrame_PushUnchecked(
2205+
tstate, PyStackRef_FromPyObjectNew(f), 2, frame);
22072206
// Manipulate stack directly because we exit with DISPATCH_INLINED().
22082207
STACK_SHRINK(1);
22092208
new_frame->localsplus[0] = owner;
@@ -3251,7 +3250,7 @@ dummy_func(
32513250
int code_flags = ((PyCodeObject*)PyFunction_GET_CODE(callable_o))->co_flags;
32523251
PyObject *locals = code_flags & CO_OPTIMIZED ? NULL : Py_NewRef(PyFunction_GET_GLOBALS(callable_o));
32533252
_PyInterpreterFrame *new_frame = _PyEvalFramePushAndInit(
3254-
tstate, (PyFunctionObject *)PyStackRef_AsPyObjectSteal(callable), locals,
3253+
tstate, callable, locals,
32553254
args, total_args, NULL, frame
32563255
);
32573256
// Manipulate stack directly since we leave using DISPATCH_INLINED().
@@ -3340,7 +3339,7 @@ dummy_func(
33403339
int code_flags = ((PyCodeObject*)PyFunction_GET_CODE(callable_o))->co_flags;
33413340
PyObject *locals = code_flags & CO_OPTIMIZED ? NULL : Py_NewRef(PyFunction_GET_GLOBALS(callable_o));
33423341
new_frame = _PyEvalFramePushAndInit(
3343-
tstate, (PyFunctionObject *)PyStackRef_AsPyObjectSteal(callable), locals,
3342+
tstate, callable, locals,
33443343
args, total_args, NULL, frame
33453344
);
33463345
// The frame has stolen all the arguments from the stack,
@@ -3475,11 +3474,9 @@ dummy_func(
34753474
}
34763475

34773476
replicate(5) pure op(_INIT_CALL_PY_EXACT_ARGS, (callable, self_or_null[1], args[oparg] -- new_frame: _PyInterpreterFrame*)) {
3478-
PyObject *callable_o = PyStackRef_AsPyObjectBorrow(callable);
34793477
int has_self = !PyStackRef_IsNull(self_or_null[0]);
34803478
STAT_INC(CALL, hit);
3481-
PyFunctionObject *func = (PyFunctionObject *)callable_o;
3482-
new_frame = _PyFrame_PushUnchecked(tstate, func, oparg + has_self, frame);
3479+
new_frame = _PyFrame_PushUnchecked(tstate, callable, oparg + has_self, frame);
34833480
_PyStackRef *first_non_self_local = new_frame->localsplus + has_self;
34843481
new_frame->localsplus[0] = self_or_null[0];
34853482
for (int i = 0; i < oparg; i++) {
@@ -3601,10 +3598,9 @@ dummy_func(
36013598
assert(_PyCode_CODE(_PyFrame_GetCode(shim))[0].op.code == EXIT_INIT_CHECK);
36023599
/* Push self onto stack of shim */
36033600
shim->localsplus[0] = PyStackRef_DUP(self);
3604-
PyFunctionObject *init_func = (PyFunctionObject *)PyStackRef_AsPyObjectSteal(init);
36053601
args[-1] = self;
36063602
init_frame = _PyEvalFramePushAndInit(
3607-
tstate, init_func, NULL, args-1, oparg+1, NULL, shim);
3603+
tstate, init, NULL, args-1, oparg+1, NULL, shim);
36083604
SYNC_SP();
36093605
if (init_frame == NULL) {
36103606
_PyEval_FrameClearAndPop(tstate, shim);
@@ -4080,7 +4076,7 @@ dummy_func(
40804076
int code_flags = ((PyCodeObject*)PyFunction_GET_CODE(callable_o))->co_flags;
40814077
PyObject *locals = code_flags & CO_OPTIMIZED ? NULL : Py_NewRef(PyFunction_GET_GLOBALS(callable_o));
40824078
_PyInterpreterFrame *new_frame = _PyEvalFramePushAndInit(
4083-
tstate, (PyFunctionObject *)PyStackRef_AsPyObjectSteal(callable), locals,
4079+
tstate, callable, locals,
40844080
args, positional_args, kwnames_o, frame
40854081
);
40864082
PyStackRef_CLOSE(kwnames);
@@ -4148,7 +4144,7 @@ dummy_func(
41484144
int code_flags = ((PyCodeObject*)PyFunction_GET_CODE(callable_o))->co_flags;
41494145
PyObject *locals = code_flags & CO_OPTIMIZED ? NULL : Py_NewRef(PyFunction_GET_GLOBALS(callable_o));
41504146
new_frame = _PyEvalFramePushAndInit(
4151-
tstate, (PyFunctionObject *)PyStackRef_AsPyObjectSteal(callable), locals,
4147+
tstate, callable, locals,
41524148
args, positional_args, kwnames_o, frame
41534149
);
41544150
PyStackRef_CLOSE(kwnames);
@@ -4332,9 +4328,9 @@ dummy_func(
43324328
int code_flags = ((PyCodeObject *)PyFunction_GET_CODE(func))->co_flags;
43334329
PyObject *locals = code_flags & CO_OPTIMIZED ? NULL : Py_NewRef(PyFunction_GET_GLOBALS(func));
43344330

4335-
_PyInterpreterFrame *new_frame = _PyEvalFramePushAndInit_Ex(tstate,
4336-
(PyFunctionObject *)PyStackRef_AsPyObjectSteal(func_st), locals,
4337-
nargs, callargs, kwargs, frame);
4331+
_PyInterpreterFrame *new_frame = _PyEvalFramePushAndInit_Ex(
4332+
tstate, func_st, locals,
4333+
nargs, callargs, kwargs, frame);
43384334
// Need to manually shrink the stack since we exit with DISPATCH_INLINED.
43394335
STACK_SHRINK(oparg + 3);
43404336
if (new_frame == NULL) {
@@ -4408,8 +4404,8 @@ dummy_func(
44084404
}
44094405

44104406
inst(RETURN_GENERATOR, (-- res)) {
4411-
assert(PyFunction_Check(frame->f_funcobj));
4412-
PyFunctionObject *func = (PyFunctionObject *)frame->f_funcobj;
4407+
assert(PyStackRef_FunctionCheck(frame->f_funcobj));
4408+
PyFunctionObject *func = (PyFunctionObject *)PyStackRef_AsPyObjectBorrow(frame->f_funcobj);
44134409
PyGenObject *gen = (PyGenObject *)_Py_MakeCoro(func);
44144410
if (gen == NULL) {
44154411
ERROR_NO_POP();
@@ -4771,8 +4767,9 @@ dummy_func(
47714767
}
47724768

47734769
tier2 op(_CHECK_FUNCTION, (func_version/2 -- )) {
4774-
assert(PyFunction_Check(frame->f_funcobj));
4775-
DEOPT_IF(((PyFunctionObject *)frame->f_funcobj)->func_version != func_version);
4770+
assert(PyStackRef_FunctionCheck(frame->f_funcobj));
4771+
PyFunctionObject *func = (PyFunctionObject *)PyStackRef_AsPyObjectBorrow(frame->f_funcobj);
4772+
DEOPT_IF(func->func_version != func_version);
47764773
}
47774774

47784775
/* Internal -- for testing executors */

0 commit comments

Comments
 (0)