8000 gh-123923: Defer refcounting for `f_executable` in `_PyInterpreterFrame` · python/cpython@24e9b4b · GitHub
[go: up one dir, main page]
Skip to content

Commit 24e9b4b

Browse files
colesburycolesbury
committed
gh-123923: Defer refcounting for f_executable in _PyInterpreterFrame
Use a `_PyStackRef` and defer the reference to `f_executable` 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 00ffdf2 commit 24e9b4b

18 files changed

+175
-97
lines changed

‎Include/internal/pycore_frame.h

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ enum _frameowner {
6060
};
6161

6262
typedef struct _PyInterpreterFrame {
63-
PyObject *f_executable; /* Strong reference (code object or None) */
63+
_PyStackRef f_executable; /* Deferred or strong reference (code object or None) */
6464
struct _PyInterpreterFrame *previous;
6565
PyObject *f_funcobj; /* Strong reference. Only valid if not on C stack */
6666
PyObject *f_globals; /* Borrowed reference. Only valid if not on C stack */
@@ -79,8 +79,9 @@ typedef struct _PyInterpreterFrame {
7979
((int)((IF)->instr_ptr - _PyCode_CODE(_PyFrame_GetCode(IF))))
8080

8181
static inline PyCodeObject *_PyFrame_GetCode(_PyInterpreterFrame *f) {
82-
assert(PyCode_Check(f->f_executable));
83-
return (PyCodeObject *)f->f_executable;
82+
PyObject *executable = PyStackRef_AsPyObjectBorrow(f->f_executable);
83+
assert(PyCode_Check(executable));
84+
return (PyCodeObject *)executable;
8485
}
8586

8687
static inline _PyStackRef *_PyFrame_Stackbase(_PyInterpreterFrame *f) {
@@ -130,7 +131,7 @@ static inline void _PyFrame_Copy(_PyInterpreterFrame *src, _PyInterpreterFrame *
130131
dest->previous = NULL;
131132

132133
#ifdef Py_GIL_DISABLED
133-
PyCodeObject *co = (PyCodeObject *)dest->f_executable;
134+
PyCodeObject *co = _PyFrame_GetCode(dest);
134135
for (int i = stacktop; i < co->co_nlocalsplus + co->co_stacksize; i++) {
135136
dest->localsplus[i] = PyStackRef_NULL;
136137
}
@@ -148,7 +149,7 @@ _PyFrame_Initialize(
148149
{
149150
frame->previous = previous;
150151
frame->f_funcobj = (PyObject *)func;
151-
frame->f_executable = Py_NewRef(code);
152+
frame->f_executable = PyStackRef_FromPyObjectNew(code);
152153
frame->f_builtins = func->func_builtins;
153154
frame->f_globals = func->func_globals;
154155
frame->f_locals = locals;
@@ -321,7 +322,7 @@ _PyFrame_PushTrampolineUnchecked(PyThreadState *tstate, PyCodeObject *code, int
321322
assert(tstate->datastack_top < tstate->datastack_limit);
322323
frame->previous = previous;
323324
frame->f_funcobj = Py_None;
324-
frame->f_executable = Py_NewRef(code);
325+
frame->f_executable = PyStackRef_FromPyObjectNew(code);
325326
#ifdef Py_DEBUG
326327
frame->f_builtins = NULL;
327328
frame->f_globals = NULL;

‎Include/internal/pycore_gc.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -381,8 +381,11 @@ extern void _PyGC_ClearAllFreeLists(PyInterpreterState *interp);
381381
extern void _Py_ScheduleGC(PyThreadState *tstate);
382382
extern void _Py_RunGC(PyThreadState *tstate);
383383

384+
union _PyStackRef;
385+
384386
// GC visit callback for tracked interpreter frames
385387
extern int _PyGC_VisitFrameStack(struct _PyInterpreterFrame *frame, visitproc visit, void *arg);
388+
extern int _PyGC_VisitStackRef(union _PyStackRef *ref, visitproc visit, void *arg);
386389

387390
#ifdef __cplusplus
388391
}

‎Include/internal/pycore_stackref.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,13 @@ PyStackRef_DUP(_PyStackRef stackref)
227227
# define PyStackRef_DUP(stackref) PyStackRef_FromPyObjectSteal(Py_NewRef(PyStackRef_AsPyObjectBorrow(stackref)))
228228
#endif
229229

230+
// Convert a possibly deferred reference to a strong reference.
231+
static inline _PyStackRef
232+
PyStackRef_AsStrongReference(_PyStackRef stackref)
233+
{
234+
return PyStackRef_FromPyObjectSteal(PyStackRef_AsPyObjectSteal(stackref));
235+
}
236+
230237
static inline void
231238
_PyObjectStack_FromStackRefStack(PyObject **dst, const _PyStackRef *src, size_t length)
232239
{
@@ -261,6 +268,11 @@ PyStackRef_ExceptionInstanceCheck(_PyStackRef stackref)
261268
return PyExceptionInstance_Check(PyStackRef_AsPyObjectBorrow(stackref));
262269
}
263270

271+
static inline bool
272+
PyStackRef_CodeCheck(_PyStackRef stackref)
273+
{
274+
return PyCode_Check(PyStackRef_AsPyObjectBorrow(stackref));
275+
}
264276

265277
static inline bool
266278
PyStackRef_FunctionCheck(_PyStackRef stackref)

‎Lib/test/test_generators.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import unittest
77
import weakref
88
import inspect
9+
import textwrap
910
import types
1011

1112
from test import support
@@ -112,6 +113,27 @@ def g3(): return (yield from f())
112113
gen.send(2)
113114
self.assertEqual(cm.exception.value, 2)
114115

116+
def test_generator_resurrect(self):
117+
# Test that a resurrected generator still has a valid gi_code
118+
resurrected = []
119+
120+
# Resurrect a generator in a finalizer
121+
exec(textwrap.dedent("""
122+
def gen():
123+
try:
124+
yield
125+
except:
126+
resurrected.append(g)
127+
128+
g = gen()
129+
next(g)
130+
"""), {"resurrected": resurrected})
131+
132+
support.gc_collect()
133+
134+
self.assertEqual(len(resurrected), 1)
135+
self.assertIsInstance(resurrected[0].gi_code, types.CodeType)
136+
115137

116138
class GeneratorTest(unittest.TestCase):
117139

‎Misc/NEWS.d/next/Core_and_Builtins/2024-09-10-20-25-00.gh-issue-123923.A7uxqa.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
The ``f_executable`` field in the internal :c:struct:`_PyInterpreterFrame`
2+
struct now uses a tagged pointer. Profilers and debuggers that uses this
3+
field should clear the least significant bit to recover the
4+
:c:expr:`PyObject*` pointer.

‎Modules/_testexternalinspection.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -510,7 +510,7 @@ parse_frame_object(
510510
return 0;
511511
}
512512

513-
void* address_of_code_object;
513+
uintptr_t address_of_code_object;
514514
bytes_read = read_memory(
515515
pid,
516516
(void*)(address + offsets->interpreter_frame.executable),
@@ -520,10 +520,11 @@ parse_frame_object(
520520
return -1;
521521
}
522522

523-
if (address_of_code_object == NULL) {
523+
if (address_of_code_object == 0) {
524524
return 0;
525525
}
526-
return parse_code_object(pid, result, offsets, address_of_code_object, previous_frame);
526+
address_of_code_object &= ~1;
527+
return parse_code_object(pid, result, offsets, (void *)address_of_code_object, previous_frame);
527528
}
528529

529530
static PyObject*

‎Objects/frameobject.c

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1625,8 +1625,6 @@ frame_dealloc(PyFrameObject *f)
16251625
}
16261626

16271627
Py_TRASHCAN_BEGIN(f, frame_dealloc);
1628-
PyObject *co = NULL;
1629-
16301628
/* GH-106092: If f->f_frame was on the stack and we reached the maximum
16311629
* nesting depth for deallocations, the trashcan may have delayed this
16321630
* deallocation until after f->f_frame is freed. Avoid dereferencing
@@ -1635,9 +1633,7 @@ frame_dealloc(PyFrameObject *f)
16351633

16361634
/* Kill all local variables including specials, if we own them */
16371635
if (f->f_frame == frame && frame->owner == FRAME_OWNED_BY_FRAME_OBJECT) {
1638-
/* Don't clear code object until the end */
1639-
co = frame->f_executable;
1640-
frame->f_executable = NULL;
1636+
PyStackRef_CLEAR(frame->f_executable);
16411637
Py_CLEAR(frame->f_funcobj);
16421638
Py_CLEAR(frame->f_locals);
16431639
_PyStackRef *locals = _PyFrame_GetLocalsArray(frame);
@@ -1652,7 +1648,6 @@ frame_dealloc(PyFrameObject *f)
16521648
Py_CLEAR(f->f_extra_locals);
16531649
Py_CLEAR(f->f_locals_cache);
16541650
PyObject_GC_Del(f);
1655-
Py_XDECREF(co);
16561651
Py_TRASHCAN_END;
16571652
}
16581653

‎Objects/genobject.c

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,14 @@ gen_traverse(PyGenObject *gen, visitproc visit, void *arg)
5555
return err;
5656
}
5757
}
58+
else {
59+
// We still need to visit the code object when the frame is cleared to
60+
// 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+
}
65+
}
5866
/* No need to visit cr_origin, because it's just tuples/str/int, so can't
5967
participate in a reference cycle. */
6068
Py_VISIT(gen->gi_exc_state.exc_value);
@@ -139,6 +147,9 @@ gen_dealloc(PyGenObject *gen)
139147
and GC_Del. */
140148
Py_CLEAR(((PyAsyncGenObject*)gen)->ag_origin_or_finalizer);
141149
}
150+
if (PyCoro_CheckExact(gen)) {
151+
Py_CLEAR(((PyCoroObject *)gen)->cr_origin_or_finalizer);
152+
}
142153
if (gen->gi_frame_state != FRAME_CLEARED) {
143154
_PyInterpreterFrame *frame = &gen->gi_iframe;
144155
gen->gi_frame_state = FRAME_CLEARED;
@@ -147,10 +158,7 @@ gen_dealloc(PyGenObject *gen)
147158
_PyErr_ClearExcState(&gen->gi_exc_state);
148159
}
149160
assert(gen->gi_exc_state.exc_value == NULL);
150-
if (_PyGen_GetCode(gen)->co_flags & CO_COROUTINE) {
151-
Py_CLEAR(((PyCoroObject *)gen)->cr_origin_or_finalizer);
152-
}
153-
Py_DECREF(_PyGen_GetCode(gen));
161+
PyStackRef_CLEAR(gen->gi_iframe.f_executable);
154162
Py_CLEAR(gen->gi_name);
155163
Py_CLEAR(gen->gi_qualname);
156164

‎Python/bytecodes.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3604,7 +3604,7 @@ dummy_func(
36043604
op(_CREATE_INIT_FRAME, (self, init, args[oparg] -- init_frame: _PyInterpreterFrame *)) {
36053605
_PyInterpreterFrame *shim = _PyFrame_PushTrampolineUnchecked(
36063606
tstate, (PyCodeObject *)&_Py_InitCleanup, 1, frame);
3607-
assert(_PyCode_CODE((PyCodeObject *)shim->f_executable)[0].op.code == EXIT_INIT_CHECK);
3607+
assert(_PyCode_CODE(_PyFrame_GetCode(shim))[0].op.code == EXIT_INIT_CHECK);
36083608
/* Push self onto stack of shim */
36093609
shim->localsplus[0] = PyStackRef_DUP(self);
36103610
PyFunctionObject *init_func = (PyFunctionObject *)PyStackRef_AsPyObjectSteal(init);

‎Python/ceval.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ static void
194194
lltrace_resume_frame(_PyInterpreterFrame *frame)
195195
{
196196
PyObject *fobj = frame->f_funcobj;
197-
if (!PyCode_Check(frame->f_executable) ||
197+
if (!PyStackRef_CodeCheck(frame->f_executable) ||
198198
fobj == NULL ||
199199
!PyFunction_Check(fobj)
200200
) {
@@ -784,7 +784,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
784784
entry_frame.f_globals = (PyObject*)0xaaa3;
785785
entry_frame.f_builtins = (PyObject*)0xaaa4;
786786
#endif
787-
entry_frame.f_executable = Py_None;
787+
entry_frame.f_executable = PyStackRef_None;
788788
entry_frame.instr_ptr = (_Py_CODEUNIT *)_Py_INTERPRETER_TRAMPOLINE_INSTRUCTIONS + 1;
789789
entry_frame.stackpointer = entry_frame.localsplus;
790790
entry_frame.owner = FRAME_OWNED_BY_CSTACK;
@@ -1681,7 +1681,7 @@ clear_thread_frame(PyThreadState *tstate, _PyInterpreterFrame * frame)
16811681
tstate->c_recursion_remaining--;
16821682
assert(frame->frame_obj == NULL || frame->frame_obj->f_frame == frame);
16831683
_PyFrame_ClearExceptCode(frame);
1684-
Py_DECREF(frame->f_executable);
1684+
PyStackRef_CLEAR(frame->f_executable);
16851685
tstate->c_recursion_remaining++;
16861686
_PyThreadState_PopFrame(tstate, frame);
16871687
}

‎Python/executor_cases.c.h

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Python/frame.c

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,10 @@ _PyFrame_Traverse(_PyInterpreterFrame *frame, visitproc visit, void *arg)
1414
Py_VISIT(frame->frame_obj);
1515
Py_VISIT(frame->f_locals);
1616
Py_VISIT(frame->f_funcobj);
17-
Py_VISIT(_PyFrame_GetCode(frame));
17+
int err = _PyGC_VisitStackRef(&frame->f_executable, visit, arg);
18+
if (err) {
19+
return err;
20+
}
1821
return _PyGC_VisitFrameStack(frame, visit, arg);
1922
}
2023

@@ -53,10 +56,10 @@ take_ownership(PyFrameObject *f, _PyInterpreterFrame *frame)
5356
assert(frame->owner != FRAME_OWNED_BY_FRAME_OBJECT);
5457
assert(frame->owner != FRAME_CLEARED);
5558
Py_ssize_t size = ((char*)frame->stackpointer) - (char *)frame;
56-
Py_INCREF(_PyFrame_GetCode(frame));
5759
memcpy((_PyInterpreterFrame *)f->_f_frame_data, frame, size);
5860
frame = (_PyInterpreterFrame *)f->_f_frame_data;
5961
frame->stackpointer = (_PyStackRef *)(((char *)frame) + size);
62+
frame->f_executable = PyStackRef_DUP(frame->f_executable);
6063
f->f_frame = frame;
6164
frame->owner = FRAME_OWNED_BY_FRAME_OBJECT;
6265
if (_PyFrame_IsIncomplete(frame)) {
@@ -131,9 +134,7 @@ _PyFrame_ClearExceptCode(_PyInterpreterFrame *frame)
131134
PyObject *
132135
PyUnstable_InterpreterFrame_GetCode(struct _PyInterpreterFrame *frame)
133136
{
134-
PyObject *code = frame->f_executable;
135-
Py_INCREF(code);
136-
return code;
137+
return PyStackRef_AsPyObjectNew(frame->f_executable);
137138
}
138139

139140
int

‎Python/gc.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -534,6 +534,13 @@ visit_decref(PyObject *op, void *parent)
534534
return 0;
535535
}
536536

537+
int
538+
_PyGC_VisitStackRef(_PyStackRef *ref, visitproc visit, void *arg)
539+
{
540+
Py_VISIT(PyStackRef_AsPyObjectBorrow(*ref));
541+
return 0;
542+
}
543+
537544
int
538545
_PyGC_VisitFrameStack(_PyInterpreterFrame *frame, visitproc visit, void *arg)
539546
{

0 commit comments

Comments
 (0)
0