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.

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

Merged
merged 10 commits into from
Feb 23, 2023
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
24c9400
Remove redundant gi_code field from generator object.
markshannon Jan 4, 2023
d68ce2c
Add news
markshannon Jan 4, 2023
3a9df11
Add API function to access code object of generator.
markshannon Jan 4, 2023
d891db9
Merge branch 'main' into trim-code-object-2
markshannon Jan 4, 2023
d81c042
Merge branch 'main' into trim-code-object-2
markshannon Feb 13, 2023
f7e9c71
Add back gi_code field to PyGenObject and deprecate it.
markshannon Feb 13, 2023
453f804
Merge branch 'main' into trim-code-object-2
markshannon Feb 13, 2023
a3e6ac6
Merge branch 'main' into trim-code-object-2
markshannon Feb 21, 2023
1357cf0
Remove gi_code field
markshannon Feb 21, 2023
47858c3
Merge branch 'main' into trim-code-object-2
markshannon Feb 21, 2023
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
5 changes: 2 additions & 3 deletions Include/cpython/genobject.h
View file
Open in desktop
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);


/* --- PyCoroObject ------------------------------------------------------- */
Expand Down
2 changes: 1 addition & 1 deletion Include/internal/pycore_frame.h
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
3 changes: 3 additions & 0 deletions Misc/NEWS.d/next/Core and Builtins/2023-01-04-12-49-33.gh-issue-100719.uRPccL.rst
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
0