8000 GH-100719: Remove redundant `gi_code` field from generator object. by markshannon · Pull Request #100749 · python/cpython · GitHub
[go: up one dir, main page]

Skip to content

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

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

Merged
merged 10 commits into from
Feb 23, 2023
Next Next commit
Remove redundant gi_code field from generator object.
  • Loading branch information
markshannon committed Jan 4, 2023
commit 24c94002473f82a076b97d565930a47a9bab90a0
2 changes: 0 additions & 2 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 Down
2 changes: 1 addition & 1 deletion Include/internal/pycore_frame.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
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 @@ -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
Expand Down
64 changes: 47 additions & 17 deletions Objects/genobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -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")},
Expand All @@ -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 */
};

Expand All @@ -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);
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -935,20 +955,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 @@ -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;
}
Expand Down Expand Up @@ -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,
Expand All @@ -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 */
};
Expand Down Expand Up @@ -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")},
Expand All @@ -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 */
};

Expand Down
80 changes: 40 additions & 40 deletions Python/ceval.c
Original file line number Diff line number Diff line change
Expand Up @@ -1962,44 +1962,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_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)
{
Expand All @@ -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);
}
Expand All @@ -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;
}
Expand All @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions Python/frame.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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. */
Expand All @@ -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
Expand Down
0