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.

Sign up for GitHub

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

Jump to bottom

gh-123923: Defer refcounting for f_funcobj in _PyInterpreterFrame #124026

Merged
merged 6 commits into from
Sep 24, 2024
Merged

gh-123923: Defer refcounting for f_funcobj in _PyInterpreterFrame #124026

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
db12634
gh-123923: Defer refcounting for `f_funcobj` in `_PyInterpreterFrame`
colesbury Sep 11, 2024
963896a
Fix tier2 optimizer
colesbury Sep 12, 2024
68d360e
Merge branch 'main' into gh-123923-defer-func
colesbury Sep 14, 2024
cd3d83d
Simplify code after merge
colesbury Sep 14, 2024
6a1014f
Merge branch 'main' into gh-123923-defer-func
colesbury Sep 20, 2024
2fde56d
Merge branch 'main' into gh-123923-defer-func
colesbury Sep 24, 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
26 changes: 17 additions & 9 deletions Include/internal/pycore_frame.h
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading