From 24c94002473f82a076b97d565930a47a9bab90a0 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 4 Jan 2023 12:40:09 +0000 Subject: [PATCH 1/5] Remove redundant gi_code field from generator object. --- Include/cpython/genobject.h | 2 - Include/internal/pycore_frame.h | 2 +- Lib/test/test_sys.py | 2 +- Objects/genobject.c | 64 +++++++++++++++++++------- Python/ceval.c | 80 ++++++++++++++++----------------- Python/frame.c | 4 +- 6 files changed, 91 insertions(+), 63 deletions(-) diff --git a/Include/cpython/genobject.h b/Include/cpython/genobject.h index 6127ba7babb80f..5a1cf312250c8d 100644 --- a/Include/cpython/genobject.h +++ b/Include/cpython/genobject.h @@ -13,8 +13,6 @@ extern "C" { and coroutine objects. */ #define _PyGenObject_HEAD(prefix) \ PyObject_HEAD \ - /* The code object backing the generator */ \ - PyCodeObject *prefix##_code; \ /* List of weak reference. */ \ PyObject *prefix##_weakreflist; \ /* Name of the generator. */ \ diff --git a/Include/internal/pycore_frame.h b/Include/internal/pycore_frame.h index f18723b303224f..8553babcee4ece 100644 --- a/Include/internal/pycore_frame.h +++ b/Include/internal/pycore_frame.h @@ -183,7 +183,7 @@ _PyFrame_GetFrameObject(_PyInterpreterFrame *frame) * frames like the ones in generators and coroutines. */ void -_PyFrame_Clear(_PyInterpreterFrame * frame); +_PyFrame_ClearExceptCode(_PyInterpreterFrame * frame); int _PyFrame_Traverse(_PyInterpreterFrame *frame, visitproc visit, void *arg); diff --git a/Lib/test/test_sys.py b/Lib/test/test_sys.py index 232b79971dc2b7..d7646e19d3712a 100644 --- a/Lib/test/test_sys.py +++ b/Lib/test/test_sys.py @@ -1440,7 +1440,7 @@ def bar(cls): check(bar, size('PP')) # generator def get_gen(): yield 1 - check(get_gen(), size('P2P4P4c7P2ic??2P')) + check(get_gen(), size('PP4P4c7P2ic??2P')) # iterator check(iter('abc'), size('lP')) # callable-iterator diff --git a/Objects/genobject.c b/Objects/genobject.c index c006f1af2177f9..a7f16e0ea09a23 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -24,6 +24,13 @@ static const char *NON_INIT_CORO_MSG = "can't send non-None value to a " static const char *ASYNC_GEN_IGNORED_EXIT_MSG = "async generator ignored GeneratorExit"; +/* Returns a borrowed reference */ +static inline PyCodeObject * +_PyGen_GetCode(PyGenObject *gen) { + _PyInterpreterFrame *frame = (_PyInterpreterFrame *)(gen->gi_iframe); + return frame->f_code; +} + static inline int exc_state_traverse(_PyErr_StackItem *exc_state, visitproc visit, void *arg) { @@ -34,7 +41,6 @@ exc_state_traverse(_PyErr_StackItem *exc_state, visitproc visit, void *arg) static int gen_traverse(PyGenObject *gen, visitproc visit, void *arg) { - Py_VISIT(gen->gi_code); Py_VISIT(gen->gi_name); Py_VISIT(gen->gi_qualname); if (gen->gi_frame_state < FRAME_CLEARED) { @@ -88,8 +94,8 @@ _PyGen_Finalize(PyObject *self) /* If `gen` is a coroutine, and if it was never awaited on, issue a RuntimeWarning. */ - if (gen->gi_code != NULL && - ((PyCodeObject *)gen->gi_code)->co_flags & CO_COROUTINE && + assert(_PyGen_GetCode(gen) != NULL); + if (_PyGen_GetCode(gen)->co_flags & CO_COROUTINE && gen->gi_frame_state == FRAME_CREATED) { _PyErr_WarnUnawaitedCoroutine((PyObject *)gen); @@ -137,12 +143,12 @@ gen_dealloc(PyGenObject *gen) _PyInterpreterFrame *frame = (_PyInterpreterFrame *)gen->gi_iframe; gen->gi_frame_state = FRAME_CLEARED; frame->previous = NULL; - _PyFrame_Clear(frame); + _PyFrame_ClearExceptCode(frame); } - if (((PyCodeObject *)gen->gi_code)->co_flags & CO_COROUTINE) { + if (_PyGen_GetCode(gen)->co_flags & CO_COROUTINE) { Py_CLEAR(((PyCoroObject *)gen)->cr_origin_or_finalizer); } - Py_CLEAR(gen->gi_code); + Py_DECREF(_PyGen_GetCode(gen)); Py_CLEAR(gen->gi_name); Py_CLEAR(gen->gi_qualname); _PyErr_ClearExcState(&gen->gi_exc_state); @@ -332,7 +338,7 @@ _PyGen_yf(PyGenObject *gen) /* Return immediately if the frame didn't start yet. SEND always come after LOAD_CONST: a code object should not start with SEND */ - assert(_Py_OPCODE(_PyCode_CODE(gen->gi_code)[0]) != SEND); + assert(_Py_OPCODE(_PyCode_CODE(_PyGen_GetCode(gen))[0]) != SEND); return NULL; } _Py_CODEUNIT next = frame->prev_instr[1]; @@ -745,6 +751,21 @@ gen_getframe(PyGenObject *gen, void *Py_UNUSED(ignored)) return _gen_getframe(gen, "gi_frame"); } +static PyObject * +_gen_getcode(PyGenObject *gen, const char *const name) +{ + if (PySys_Audit("object.__getattr__", "Os", gen, name) < 0) { + return NULL; + } + return Py_NewRef(_PyGen_GetCode(gen)); +} + +static PyObject * +gen_getcode(PyGenObject *gen, void *Py_UNUSED(ignored)) +{ + return _gen_getcode(gen, "gi_code"); +} + static PyGetSetDef gen_getsetlist[] = { {"__name__", (getter)gen_get_name, (setter)gen_set_name, PyDoc_STR("name of the generator")}, @@ -755,11 +776,11 @@ static PyGetSetDef gen_getsetlist[] = { {"gi_running", (getter)gen_getrunning, NULL, NULL}, {"gi_frame", (getter)gen_getframe, NULL, NULL}, {"gi_suspended", (getter)gen_getsuspended, NULL, NULL}, + {"gi_code", (getter)gen_getcode, NULL, NULL}, {NULL} /* Sentinel */ }; static PyMemberDef gen_memberlist[] = { - {"gi_code", T_OBJECT, offsetof(PyGenObject, gi_code), READONLY|PY_AUDIT_READ}, {NULL} /* Sentinel */ }; @@ -768,7 +789,7 @@ gen_sizeof(PyGenObject *gen, PyObject *Py_UNUSED(ignored)) { Py_ssize_t res; res = offsetof(PyGenObject, gi_iframe) + offsetof(_PyInterpreterFrame, localsplus); - PyCodeObject *code = gen->gi_code; + PyCodeObject *code = _PyGen_GetCode(gen); res += (code->co_nlocalsplus+code->co_stacksize) * sizeof(PyObject *); return PyLong_FromSsize_t(res); } @@ -856,7 +877,6 @@ make_gen(PyTypeObject *type, PyFunctionObject *func) return NULL; } gen->gi_frame_state = FRAME_CLEARED; - gen->gi_code = (PyCodeObject *)Py_NewRef(func->func_code); gen->gi_weakreflist = NULL; gen->gi_exc_state.exc_value = NULL; gen->gi_exc_state.previous_item = NULL; @@ -935,8 +955,6 @@ gen_new_with_qualname(PyTypeObject *type, PyFrameObject *f, f->f_frame = frame; frame->owner = FRAME_OWNED_BY_GENERATOR; assert(PyObject_GC_IsTracked((PyObject *)f)); - gen->gi_code = PyFrame_GetCode(f); - Py_INCREF(gen->gi_code); Py_DECREF(f); gen->gi_weakreflist = NULL; gen->gi_exc_state.exc_value = NULL; @@ -944,11 +962,11 @@ gen_new_with_qualname(PyTypeObject *type, PyFrameObject *f, if (name != NULL) gen->gi_name = Py_NewRef(name); else - gen->gi_name = Py_NewRef(gen->gi_code->co_name); + gen->gi_name = Py_NewRef(_PyGen_GetCode(gen)->co_name); if (qualname != NULL) gen->gi_qualname = Py_NewRef(qualname); else - gen->gi_qualname = Py_NewRef(gen->gi_code->co_qualname); + gen->gi_qualname = Py_NewRef(_PyGen_GetCode(gen)->co_qualname); _PyObject_GC_TRACK(gen); return (PyObject *)gen; } @@ -976,7 +994,7 @@ static int gen_is_coroutine(PyObject *o) { if (PyGen_CheckExact(o)) { - PyCodeObject *code = (PyCodeObject *)((PyGenObject*)o)->gi_code; + PyCodeObject *code = _PyGen_GetCode((PyGenObject*)o); if (code->co_flags & CO_ITERABLE_COROUTINE) { return 1; } @@ -1085,6 +1103,12 @@ cr_getframe(PyCoroObject *coro, void *Py_UNUSED(ignored)) return _gen_getframe((PyGenObject *)coro, "cr_frame"); } +static PyObject * +cr_getcode(PyCoroObject *coro, void *Py_UNUSED(ignored)) +{ + return _gen_getcode((PyGenObject *)coro, "cr_code"); +} + static PyGetSetDef coro_getsetlist[] = { {"__name__", (getter)gen_get_name, (setter)gen_set_name, @@ -1095,12 +1119,12 @@ static PyGetSetDef coro_getsetlist[] = { PyDoc_STR("object being awaited on, or None")}, {"cr_running", (getter)cr_getrunning, NULL, NULL}, {"cr_frame", (getter)cr_getframe, NULL, NULL}, + {"cr_code", (getter)cr_getcode, NULL, NULL}, {"cr_suspended", (getter)cr_getsuspended, NULL, NULL}, {NULL} /* Sentinel */ }; static PyMemberDef coro_memberlist[] = { - {"cr_code", T_OBJECT, offsetof(PyCoroObject, cr_code), READONLY|PY_AUDIT_READ}, {"cr_origin", T_OBJECT, offsetof(PyCoroObject, cr_origin_or_finalizer), READONLY}, {NULL} /* Sentinel */ }; @@ -1489,6 +1513,12 @@ ag_getframe(PyAsyncGenObject *ag, void *Py_UNUSED(ignored)) return _gen_getframe((PyGenObject *)ag, "ag_frame"); } +static PyObject * +ag_getcode(PyGenObject *gen, void *Py_UNUSED(ignored)) +{ + return _gen_getcode(gen, "ag__code"); +} + static PyGetSetDef async_gen_getsetlist[] = { {"__name__", (getter)gen_get_name, (setter)gen_set_name, PyDoc_STR("name of the async generator")}, @@ -1497,13 +1527,13 @@ static PyGetSetDef async_gen_getsetlist[] = { {"ag_await", (getter)coro_get_cr_await, NULL, PyDoc_STR("object being awaited on, or None")}, {"ag_frame", (getter)ag_getframe, NULL, NULL}, + {"ag_code", (getter)ag_getcode, NULL, NULL}, {NULL} /* Sentinel */ }; static PyMemberDef async_gen_memberlist[] = { {"ag_running", T_BOOL, offsetof(PyAsyncGenObject, ag_running_async), READONLY}, - {"ag_code", T_OBJECT, offsetof(PyAsyncGenObject, ag_code), READONLY|PY_AUDIT_READ}, {NULL} /* Sentinel */ }; diff --git a/Python/ceval.c b/Python/ceval.c index 45f42800d7ce58..6df20e93e97daa 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -1962,44 +1962,6 @@ initialize_locals(PyThreadState *tstate, PyFunctionObject *func, return -1; } -/* Consumes references to func, locals and all the args */ -static _PyInterpreterFrame * -_PyEvalFramePushAndInit(PyThreadState *tstate, PyFunctionObject *func, - PyObject *locals, PyObject* const* args, - size_t argcount, PyObject *kwnames) -{ - PyCodeObject * code = (PyCodeObject *)func->func_code; - CALL_STAT_INC(frames_pushed); - _PyInterpreterFrame *frame = _PyThreadState_PushFrame(tstate, code->co_framesize); - if (frame == NULL) { - goto fail; - } - _PyFrame_InitializeSpecials(frame, func, locals, code); - PyObject **localsarray = &frame->localsplus[0]; - for (int i = 0; i < code->co_nlocalsplus; i++) { - localsarray[i] = NULL; - } - if (initialize_locals(tstate, func, localsarray, args, argcount, kwnames)) { - assert(frame->owner != FRAME_OWNED_BY_GENERATOR); - _PyEvalFrameClearAndPop(tstate, frame); - return NULL; - } - return frame; -fail: - /* Consume the references */ - for (size_t i = 0; i < argcount; i++) { - Py_DECREF(args[i]); - } - if (kwnames) { - Py_ssize_t kwcount = PyTuple_GET_SIZE(kwnames); - for (Py_ssize_t i = 0; i < kwcount; i++) { - Py_DECREF(args[i+argcount]); - } - } - PyErr_NoMemory(); - return NULL; -} - static void clear_thread_frame(PyThreadState *tstate, _PyInterpreterFrame * frame) { @@ -2010,7 +1972,8 @@ clear_thread_frame(PyThreadState *tstate, _PyInterpreterFrame * frame) tstate->datastack_top); tstate->c_recursion_remaining--; assert(frame->frame_obj == NULL || frame->frame_obj->f_frame == frame); - _PyFrame_Clear(frame); + _PyFrame_ClearExceptCode(frame); + Py_DECREF(frame->f_code); tstate->c_recursion_remaining++; _PyThreadState_PopFrame(tstate, frame); } @@ -2026,7 +1989,7 @@ clear_gen_frame(PyThreadState *tstate, _PyInterpreterFrame * frame) gen->gi_exc_state.previous_item = NULL; tstate->c_recursion_remaining--; assert(frame->frame_obj == NULL || frame->frame_obj->f_frame == frame); - _PyFrame_Clear(frame); + _PyFrame_ClearExceptCode(frame); tstate->c_recursion_remaining++; frame->previous = NULL; } @@ -2042,6 +2005,43 @@ _PyEvalFrameClearAndPop(PyThreadState *tstate, _PyInterpreterFrame * frame) } } +/* Consumes references to func, locals and all the args */ +static _PyInterpreterFrame * +_PyEvalFramePushAndInit(PyThreadState *tstate, PyFunctionObject *func, + PyObject *locals, PyObject* const* args, + size_t argcount, PyObject *kwnames) +{ + PyCodeObject * code = (PyCodeObject *)func->func_code; + CALL_STAT_INC(frames_pushed); + _PyInterpreterFrame *frame = _PyThreadState_PushFrame(tstate, code->co_framesize); + if (frame == NULL) { + goto fail; + } + _PyFrame_InitializeSpecials(frame, func, locals, code); + PyObject **localsarray = &frame->localsplus[0]; + for (int i = 0; i < code->co_nlocalsplus; i++) { + localsarray[i] = NULL; + } + if (initialize_locals(tstate, func, localsarray, args, argcount, kwnames)) { + assert(frame->owner == FRAME_OWNED_BY_THREAD); + clear_thread_frame(tstate, frame); + return NULL; + } + return frame; +fail: + /* Consume the references */ + for (size_t i = 0; i < argcount; i++) { + Py_DECREF(args[i]); + } + if (kwnames) { + Py_ssize_t kwcount = PyTuple_GET_SIZE(kwnames); + for (Py_ssize_t i = 0; i < kwcount; i++) { + Py_DECREF(args[i+argcount]); + } + } + PyErr_NoMemory(); + return NULL; +} PyObject * _PyEval_Vector(PyThreadState *tstate, PyFunctionObject *func, diff --git a/Python/frame.c b/Python/frame.c index b1525cca511224..3ee4df3ca179fa 100644 --- a/Python/frame.c +++ b/Python/frame.c @@ -84,6 +84,7 @@ take_ownership(PyFrameObject *f, _PyInterpreterFrame *frame) assert(frame->owner != FRAME_OWNED_BY_FRAME_OBJECT); assert(frame->owner != FRAME_CLEARED); Py_ssize_t size = ((char*)&frame->localsplus[frame->stacktop]) - (char *)frame; + Py_INCREF(frame->f_code); memcpy((_PyInterpreterFrame *)f->_f_frame_data, frame, size); frame = (_PyInterpreterFrame *)f->_f_frame_data; f->f_frame = frame; @@ -121,7 +122,7 @@ take_ownership(PyFrameObject *f, _PyInterpreterFrame *frame) } void -_PyFrame_Clear(_PyInterpreterFrame *frame) +_PyFrame_ClearExceptCode(_PyInterpreterFrame *frame) { /* It is the responsibility of the owning generator/coroutine * to have cleared the enclosing generator, if any. */ @@ -147,7 +148,6 @@ _PyFrame_Clear(_PyInterpreterFrame *frame) Py_XDECREF(frame->frame_obj); Py_XDECREF(frame->f_locals); Py_DECREF(frame->f_funcobj); - Py_DECREF(frame->f_code); } int From d68ce2cf094f57d1cd14e726ffdfbf233fe1ab54 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 4 Jan 2023 12:49:39 +0000 Subject: [PATCH 2/5] Add news --- .../2023-01-04-12-49-33.gh-issue-100719.uRPccL.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2023-01-04-12-49-33.gh-issue-100719.uRPccL.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-01-04-12-49-33.gh-issue-100719.uRPccL.rst b/Misc/NEWS.d/next/Core and Builtins/2023-01-04-12-49-33.gh-issue-100719.uRPccL.rst new file mode 100644 index 00000000000000..2addef27b8ea4d --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2023-01-04-12-49-33.gh-issue-100719.uRPccL.rst @@ -0,0 +1,3 @@ +Remove gi_code field from generator (and coroutine and async generator) +objects as it is redundant. The frame already includes a reference to the +code object. From 3a9df11651c8d2cf5090f26f7e62bd32da6a9813 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 4 Jan 2023 15:31:32 +0000 Subject: [PATCH 3/5] Add API function to access code object of generator. --- Include/cpython/genobject.h | 1 + Lib/test/test_capi/test_misc.py | 5 +++++ Modules/_testcapimodule.c | 11 +++++++++++ Objects/genobject.c | 8 ++++++++ 4 files changed, 25 insertions(+) diff --git a/Include/cpython/genobject.h b/Include/cpython/genobject.h index 5a1cf312250c8d..7eecdcf76d885e 100644 --- a/Include/cpython/genobject.h +++ b/Include/cpython/genobject.h @@ -44,6 +44,7 @@ PyAPI_FUNC(PyObject *) PyGen_NewWithQualName(PyFrameObject *, PyAPI_FUNC(int) _PyGen_SetStopIterationValue(PyObject *); PyAPI_FUNC(int) _PyGen_FetchStopIterationValue(PyObject **); PyAPI_FUNC(void) _PyGen_Finalize(PyObject *self); +PyAPI_FUNC(PyCodeObject *) PyGen_GetCode(PyGenObject *gen); /* --- PyCoroObject ------------------------------------------------------- */ diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py index dace37c362e569..226d87ca7b6ca5 100644 --- a/Lib/test/test_capi/test_misc.py +++ b/Lib/test/test_capi/test_misc.py @@ -1214,6 +1214,11 @@ def test_pendingcalls_non_threaded(self): self.pendingcalls_submit(l, n) self.pendingcalls_wait(l, n) + def test_gen_get_code(self): + def genf(): yield + gen = genf() + self.assertEqual(_testcapi.gen_get_code(gen), gen.gi_code) + class SubinterpreterTest(unittest.TestCase): diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index c777c3efc4923b..b06d28952f18eb 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -2943,6 +2943,16 @@ eval_get_func_desc(PyObject *self, PyObject *func) return PyUnicode_FromString(PyEval_GetFuncDesc(func)); } +static PyObject * +gen_get_code(PyObject *self, PyObject *gen) +{ + if (!PyGen_Check(gen)) { + PyErr_SetString(PyExc_TypeError, "argument must be a generator object"); + return NULL; + } + return (PyObject *)PyGen_GetCode((PyGenObject *)gen); +} + static PyObject * get_feature_macros(PyObject *self, PyObject *Py_UNUSED(args)) { @@ -3346,6 +3356,7 @@ static PyMethodDef TestMethods[] = { {"frame_getvarstring", test_frame_getvarstring, METH_VARARGS, NULL}, {"eval_get_func_name", eval_get_func_name, METH_O, NULL}, {"eval_get_func_desc", eval_get_func_desc, METH_O, NULL}, + {"gen_get_code", gen_get_code, METH_O, NULL}, {"get_feature_macros", get_feature_macros, METH_NOARGS, NULL}, {"test_code_api", test_code_api, METH_NOARGS, NULL}, {"settrace_to_record", settrace_to_record, METH_O, NULL}, diff --git a/Objects/genobject.c b/Objects/genobject.c index a7f16e0ea09a23..f97dbf4b9315c3 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -31,6 +31,14 @@ _PyGen_GetCode(PyGenObject *gen) { return frame->f_code; } +PyCodeObject * +PyGen_GetCode(PyGenObject *gen) { + assert(PyGen_Check(gen)); + PyCodeObject *res = _PyGen_GetCode(gen); + Py_INCREF(res); + return res; +} + static inline int exc_state_traverse(_PyErr_StackItem *exc_state, visitproc visit, void *arg) { From f7e9c7133e1982110cb078d6eb76b4752dd75015 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Mon, 13 Feb 2023 12:10:15 +0000 Subject: [PATCH 4/5] Add back gi_code field to PyGenObject and deprecate it. --- Include/cpython/genobject.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Include/cpython/genobject.h b/Include/cpython/genobject.h index 7eecdcf76d885e..fa3f4f232169d1 100644 --- a/Include/cpython/genobject.h +++ b/Include/cpython/genobject.h @@ -26,7 +26,10 @@ extern "C" { char prefix##_running_async; \ /* The frame */ \ int8_t prefix##_frame_state; \ - PyObject *prefix##_iframe[1]; + union { \ + Py_DEPRECATED(3.12) PyCodeObject *prefix##_code; \ + PyObject *prefix##_iframe[1]; \ + }; typedef struct { /* The gi_ prefix is intended to remind of generator-iterator. */ From 1357cf0449409343f0215b74f820195dd2ede8a3 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Tue, 21 Feb 2023 15:26:06 +0000 Subject: [PATCH 5/5] Remove gi_code field --- Include/cpython/genobject.h | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Include/cpython/genobject.h b/Include/cpython/genobject.h index fa3f4f232169d1..18b8ce913e6e31 100644 --- a/Include/cpython/genobject.h +++ b/Include/cpython/genobject.h @@ -26,10 +26,7 @@ extern "C" { char prefix##_running_async; \ /* The frame */ \ int8_t prefix##_frame_state; \ - union { \ - Py_DEPRECATED(3.12) PyCodeObject *prefix##_code; \ - PyObject *prefix##_iframe[1]; \ - }; + PyObject *prefix##_iframe[1]; \ typedef struct { /* The gi_ prefix is intended to remind of generator-iterator. */