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.

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

GH-100719: Remove redundant gi_code field from generator object. #100749

Merged
merged 10 commits into from
Feb 23, 2023
5 changes: 2 additions & 3 deletions Include/cpython/genobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -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. */ \
Expand All @@ -28,7 +26,7 @@ extern "C" {
char prefix##_running_async; \
/* The frame */ \
int8_t prefix##_frame_state; \
PyObject *prefix##_iframe[1];
PyObject *prefix##_iframe[1]; \

typedef struct {
/* The gi_ prefix is intended to remind of generator-iterator. */
Expand All @@ -46,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);
ericsnowcurrently marked this conversation as resolved.
Show resolved Hide resolved


/* --- PyCoroObject ------------------------------------------------------- */
Expand Down
2 changes: 1 addition & 1 deletion Include/internal/pycore_frame.h
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,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);
Expand Down
5 changes: 5 additions & 0 deletions Lib/test/test_capi/test_misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -1213,6 +1213,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):

Expand Down
2 changes: 1 addition & 1 deletion Lib/test/test_sys.py
Original file line number Diff line number Diff line change
Expand Up @@ -1460,7 +1460,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
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
11 changes: 11 additions & 0 deletions Modules/_testcapimodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -3076,6 +3076,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 *
eval_eval_code_ex(PyObject *mod, PyObject *pos_args)
{
Expand Down Expand Up @@ -3657,6 +3667,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},
Expand Down
72 changes: 55 additions & 17 deletions Objects/genobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,21 @@ 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;
}

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)
{
Expand All @@ -34,7 +49,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) {
Expand Down Expand Up @@ -88,8 +102,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);
Expand Down Expand Up @@ -137,12 +151,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);
Expand Down Expand Up @@ -332,7 +346,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(_PyCode_CODE(gen->gi_code)[0].op.code != SEND);
assert(_PyCode_CODE(_PyGen_GetCode(gen))[0].op.code != SEND);
return NULL;
}
_Py_CODEUNIT next = frame->prev_instr[1];
Expand Down Expand Up @@ -767,6 +781,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")},
Expand All @@ -777,11 +806,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 */
};

Expand All @@ -790,7 +819,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 += _PyFrame_NumSlotsForCodeObject(code) * sizeof(PyObject *);
return PyLong_FromSsize_t(res);
}
Expand Down Expand Up @@ -878,7 +907,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;
Expand Down Expand Up @@ -960,20 +988,18 @@ 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;
gen->gi_exc_state.previous_item = NULL;
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;
}
Expand Down Expand Up @@ -1001,7 +1027,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;
}
Expand Down Expand Up @@ -1110,6 +1136,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,
Expand All @@ -1120,12 +1152,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 */
};
Expand Down Expand Up @@ -1514,6 +1546,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")},
Expand All @@ -1522,13 +1560,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 */
};

Expand Down
73 changes: 36 additions & 37 deletions Python/ceval.c
Original file line number Diff line number Diff line change
Expand Up @@ -1604,41 +1604,6 @@ initialize_locals(PyThreadState *tstate, PyFunctionObject *func,
return -1;
}

/* Consumes references to func, locals and all the args */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was this moved downwards?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was a small change, it now calls clear_thread_frame() rather than _PyEvalFrameClearAndPop(), so now needs to come after clear_thread_frame

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_Initialize(frame, func, locals, code, 0);
PyObject **localsarray = &frame->localsplus[0];
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)
{
Expand All @@ -1649,7 +1614,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);
}
Expand All @@ -1665,7 +1631,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;
}
Expand All @@ -1681,6 +1647,39 @@ _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_Initialize(frame, func, locals, code, 0);
if (initialize_locals(tstate, func, frame->localsplus, 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,
Expand Down
Loading