From 367f54806c191febd96439601bec02f0c06adefc Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Fri, 3 Nov 2023 17:15:33 +0000 Subject: [PATCH 1/7] inline exc_state_traverse --- Objects/genobject.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/Objects/genobject.c b/Objects/genobject.c index a32ad74ecd800a..a2036c088f03f6 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -43,13 +43,6 @@ PyGen_GetCode(PyGenObject *gen) { return res; } -static inline int -exc_state_traverse(_PyErr_StackItem *exc_state, visitproc visit, void *arg) -{ - Py_VISIT(exc_state->exc_value); - return 0; -} - static int gen_traverse(PyGenObject *gen, visitproc visit, void *arg) { @@ -66,7 +59,8 @@ gen_traverse(PyGenObject *gen, visitproc visit, void *arg) } /* No need to visit cr_origin, because it's just tuples/str/int, so can't participate in a reference cycle. */ - return exc_state_traverse(&gen->gi_exc_state, visit, arg); + Py_VISIT(gen->gi_exc_state.exc_value); + return 0; } void From 470f82eb2c5409d9c6bdb51098961af467d37123 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Fri, 3 Nov 2023 17:40:38 +0000 Subject: [PATCH 2/7] remove comparisons with frame states --- Include/internal/pycore_frame.h | 1 + Objects/genobject.c | 16 +++++++++------- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/Include/internal/pycore_frame.h b/Include/internal/pycore_frame.h index f95352fd007c6c..21d874f16aa00e 100644 --- a/Include/internal/pycore_frame.h +++ b/Include/internal/pycore_frame.h @@ -45,6 +45,7 @@ typedef enum _framestate { } PyFrameState; #define FRAME_STATE_SUSPENDED(S) ((S) == FRAME_SUSPENDED || (S) == FRAME_SUSPENDED_YIELD_FROM) +#define FRAME_STATE_CLOSED(S) ((S) >= FRAME_COMPLETED) enum _frameowner { FRAME_OWNED_BY_THREAD = 0, diff --git a/Objects/genobject.c b/Objects/genobject.c index a2036c088f03f6..bb4c57d336d041 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -48,7 +48,7 @@ gen_traverse(PyGenObject *gen, visitproc visit, void *arg) { Py_VISIT(gen->gi_name); Py_VISIT(gen->gi_qualname); - if (gen->gi_frame_state < FRAME_CLEARED) { + if (gen->gi_frame_state != FRAME_CLEARED) { _PyInterpreterFrame *frame = (_PyInterpreterFrame *)(gen->gi_iframe); assert(frame->frame_obj == NULL || frame->frame_obj->f_frame->owner == FRAME_OWNED_BY_GENERATOR); @@ -68,7 +68,7 @@ _PyGen_Finalize(PyObject *self) { PyGenObject *gen = (PyGenObject *)self; - if (gen->gi_frame_state >= FRAME_COMPLETED) { + if (FRAME_STATE_CLOSED(gen->gi_frame_state)) { /* Generator isn't paused, so no need to close */ return; } @@ -141,7 +141,7 @@ gen_dealloc(PyGenObject *gen) and GC_Del. */ Py_CLEAR(((PyAsyncGenObject*)gen)->ag_origin_or_finalizer); } - if (gen->gi_frame_state < FRAME_CLEARED) { + if (gen->gi_frame_state != FRAME_CLEARED) { _PyInterpreterFrame *frame = (_PyInterpreterFrame *)gen->gi_iframe; gen->gi_frame_state = FRAME_CLEARED; frame->previous = NULL; @@ -192,7 +192,7 @@ gen_send_ex2(PyGenObject *gen, PyObject *arg, PyObject **presult, PyErr_SetString(PyExc_ValueError, msg); return PYGEN_ERROR; } - if (gen->gi_frame_state >= FRAME_COMPLETED) { + if (FRAME_STATE_CLOSED(gen->gi_frame_state)) { if (PyCoro_CheckExact(gen) && !closing) { /* `gen` is an exhausted coroutine: raise an error, except when called from gen_close(), which should @@ -210,7 +210,9 @@ gen_send_ex2(PyGenObject *gen, PyObject *arg, PyObject **presult, return PYGEN_ERROR; } - assert(gen->gi_frame_state < FRAME_EXECUTING); + assert((gen->gi_frame_state == FRAME_CREATED) || + FRAME_STATE_SUSPENDED(gen->gi_frame_state)); + /* Push arg onto the frame's value stack */ result = arg ? arg : Py_None; _PyFrame_StackPush(frame, Py_NewRef(result)); @@ -360,7 +362,7 @@ gen_close(PyGenObject *gen, PyObject *args) gen->gi_frame_state = FRAME_COMPLETED; Py_RETURN_NONE; } - if (gen->gi_frame_state >= FRAME_COMPLETED) { + if (FRAME_STATE_CLOSED(gen->gi_frame_state)) { Py_RETURN_NONE; } PyObject *yf = _PyGen_yf(gen); @@ -2089,7 +2091,7 @@ async_gen_athrow_send(PyAsyncGenAThrow *o, PyObject *arg) return NULL; } - if (gen->gi_frame_state >= FRAME_COMPLETED) { + if (FRAME_STATE_CLOSED(gen->gi_frame_state)) { o->agt_state = AWAITABLE_STATE_CLOSED; PyErr_SetNone(PyExc_StopIteration); return NULL; From 3dc75863f2f7d3c9a8335c6035ffaf15cb1c8af1 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Fri, 3 Nov 2023 18:04:59 +0000 Subject: [PATCH 3/7] variable reused for two different purposes --- Objects/genobject.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Objects/genobject.c b/Objects/genobject.c index bb4c57d336d041..c506baeb768ca0 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -165,7 +165,6 @@ gen_send_ex2(PyGenObject *gen, PyObject *arg, PyObject **presult, { PyThreadState *tstate = _PyThreadState_GET(); _PyInterpreterFrame *frame = (_PyInterpreterFrame *)gen->gi_iframe; - PyObject *result; *presult = NULL; if (gen->gi_frame_state == FRAME_CREATED && arg && arg != Py_None) { @@ -214,8 +213,8 @@ gen_send_ex2(PyGenObject *gen, PyObject *arg, PyObject **presult, FRAME_STATE_SUSPENDED(gen->gi_frame_state)); /* Push arg onto the frame's value stack */ - result = arg ? arg : Py_None; - _PyFrame_StackPush(frame, Py_NewRef(result)); + PyObject *arg_obj = arg ? arg : Py_None; + _PyFrame_StackPush(frame, Py_NewRef(arg_obj)); _PyErr_StackItem *prev_exc_info = tstate->exc_info; gen->gi_exc_state.previous_item = prev_exc_info; @@ -228,7 +227,7 @@ gen_send_ex2(PyGenObject *gen, PyObject *arg, PyObject **presult, gen->gi_frame_state = FRAME_EXECUTING; EVAL_CALL_STAT_INC(EVAL_CALL_GENERATOR); - result = _PyEval_EvalFrame(tstate, frame, exc); + PyObject *result = _PyEval_EvalFrame(tstate, frame, exc); assert(tstate->exc_info == prev_exc_info); assert(gen->gi_exc_state.previous_item == NULL); assert(gen->gi_frame_state != FRAME_EXECUTING); From 677fc3efb6ffee47440359157849e47785199154 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Fri, 3 Nov 2023 18:11:27 +0000 Subject: [PATCH 4/7] remove unused #includes --- Objects/genobject.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/Objects/genobject.c b/Objects/genobject.c index c506baeb768ca0..ef63485a394a9c 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -9,13 +9,10 @@ #include "pycore_genobject.h" // struct _Py_async_gen_state #include "pycore_modsupport.h" // _PyArg_CheckPositional() #include "pycore_object.h" // _PyObject_GC_UNTRACK() -#include "pycore_opcode_metadata.h" // _PyOpcode_Caches #include "pycore_opcode_utils.h" // RESUME_AFTER_YIELD_FROM #include "pycore_pyerrors.h" // _PyErr_ClearExcState() #include "pycore_pystate.h" // _PyThreadState_GET() -#include "opcode.h" // SEND -#include "frameobject.h" // _PyInterpreterFrame_GetLine #include "pystats.h" static PyObject *gen_close(PyGenObject *, PyObject *); From 0936160c2b3fbb9c6902e20ec8af8ff7c51f3bd2 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Fri, 3 Nov 2023 18:45:14 +0000 Subject: [PATCH 5/7] add RESUME_WITH_SUBITERATOR macro --- Include/internal/pycore_opcode_utils.h | 4 ++++ Objects/genobject.c | 4 ++-- Python/bytecodes.c | 4 ++-- Python/generated_cases.c.h | 4 ++-- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/Include/internal/pycore_opcode_utils.h b/Include/internal/pycore_opcode_utils.h index 208bfb2f75308b..9b50c993df6d01 100644 --- a/Include/internal/pycore_opcode_utils.h +++ b/Include/internal/pycore_opcode_utils.h @@ -67,6 +67,10 @@ extern "C" { #define RESUME_OPARG_LOCATION_MASK 0x3 #define RESUME_OPARG_DEPTH1_MASK 0x4 + +#define RESUME_WITH_SUBITERATOR(V) \ + (((V) & RESUME_OPARG_LOCATION_MASK) >= RESUME_AFTER_YIELD_FROM) + #ifdef __cplusplus } #endif diff --git a/Objects/genobject.c b/Objects/genobject.c index ef63485a394a9c..bc5b5a292ca1e6 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -9,7 +9,7 @@ #include "pycore_genobject.h" // struct _Py_async_gen_state #include "pycore_modsupport.h" // _PyArg_CheckPositional() #include "pycore_object.h" // _PyObject_GC_UNTRACK() -#include "pycore_opcode_utils.h" // RESUME_AFTER_YIELD_FROM +#include "pycore_opcode_utils.h" // RESUME_WITH_SUBITERATOR #include "pycore_pyerrors.h" // _PyErr_ClearExcState() #include "pycore_pystate.h" // _PyThreadState_GET() @@ -341,7 +341,7 @@ _PyGen_yf(PyGenObject *gen) if (gen->gi_frame_state == FRAME_SUSPENDED_YIELD_FROM) { _PyInterpreterFrame *frame = (_PyInterpreterFrame *)gen->gi_iframe; assert(is_resume(frame->instr_ptr)); - assert((frame->instr_ptr->op.arg & RESUME_OPARG_LOCATION_MASK) >= RESUME_AFTER_YIELD_FROM); + assert(RESUME_WITH_SUBITERATOR(frame->instr_ptr->op.arg)); return Py_NewRef(_PyFrame_StackPeek(frame)); } return NULL; diff --git a/Python/bytecodes.c b/Python/bytecodes.c index f487e95854b7fa..2c1f1fce18b558 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -149,7 +149,7 @@ dummy_func( next_instr = this_instr; } else { - if ((oparg & RESUME_OPARG_LOCATION_MASK) < RESUME_AFTER_YIELD_FROM) { + if (!RESUME_WITH_SUBITERATOR(oparg)) { CHECK_EVAL_BREAKER(); } this_instr->op.code = RESUME_CHECK; @@ -177,7 +177,7 @@ dummy_func( next_instr = this_instr; } else { - if ((oparg & RESUME_OPARG_LOCATION_MASK) < RESUME_AFTER_YIELD_FROM) { + if (!RESUME_WITH_SUBITERATOR(oparg)) { CHECK_EVAL_BREAKER(); } _PyFrame_SetStackPointer(frame, stack_pointer); diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 12e4f5204cc3dc..cd66bb2fd28bc9 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -35,7 +35,7 @@ next_instr = this_instr; } else { - if ((oparg & RESUME_OPARG_LOCATION_MASK) < RESUME_AFTER_YIELD_FROM) { + if (!RESUME_WITH_SUBITERATOR(oparg)) { CHECK_EVAL_BREAKER(); } this_instr->op.code = RESUME_CHECK; @@ -71,7 +71,7 @@ next_instr = this_instr; } else { - if ((oparg & RESUME_OPARG_LOCATION_MASK) < RESUME_AFTER_YIELD_FROM) { + if (!RESUME_WITH_SUBITERATOR(oparg)) { CHECK_EVAL_BREAKER(); } _PyFrame_SetStackPointer(frame, stack_pointer); From 6bd958c5cb0ae39fe6b67b479916fd746e1f8afd Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Mon, 6 Nov 2023 19:55:21 +0000 Subject: [PATCH 6/7] Revert "add RESUME_WITH_SUBITERATOR macro" This reverts commit 0936160c2b3fbb9c6902e20ec8af8ff7c51f3bd2. --- Include/internal/pycore_opcode_utils.h | 4 ---- Objects/genobject.c | 4 ++-- Python/bytecodes.c | 4 ++-- Python/generated_cases.c.h | 4 ++-- 4 files changed, 6 insertions(+), 10 deletions(-) diff --git a/Include/internal/pycore_opcode_utils.h b/Include/internal/pycore_opcode_utils.h index 9b50c993df6d01..208bfb2f75308b 100644 --- a/Include/internal/pycore_opcode_utils.h +++ b/Include/internal/pycore_opcode_utils.h @@ -67,10 +67,6 @@ extern "C" { #define RESUME_OPARG_LOCATION_MASK 0x3 #define RESUME_OPARG_DEPTH1_MASK 0x4 - -#define RESUME_WITH_SUBITERATOR(V) \ - (((V) & RESUME_OPARG_LOCATION_MASK) >= RESUME_AFTER_YIELD_FROM) - #ifdef __cplusplus } #endif diff --git a/Objects/genobject.c b/Objects/genobject.c index bc5b5a292ca1e6..ef63485a394a9c 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -9,7 +9,7 @@ #include "pycore_genobject.h" // struct _Py_async_gen_state #include "pycore_modsupport.h" // _PyArg_CheckPositional() #include "pycore_object.h" // _PyObject_GC_UNTRACK() -#include "pycore_opcode_utils.h" // RESUME_WITH_SUBITERATOR +#include "pycore_opcode_utils.h" // RESUME_AFTER_YIELD_FROM #include "pycore_pyerrors.h" // _PyErr_ClearExcState() #include "pycore_pystate.h" // _PyThreadState_GET() @@ -341,7 +341,7 @@ _PyGen_yf(PyGenObject *gen) if (gen->gi_frame_state == FRAME_SUSPENDED_YIELD_FROM) { _PyInterpreterFrame *frame = (_PyInterpreterFrame *)gen->gi_iframe; assert(is_resume(frame->instr_ptr)); - assert(RESUME_WITH_SUBITERATOR(frame->instr_ptr->op.arg)); + assert((frame->instr_ptr->op.arg & RESUME_OPARG_LOCATION_MASK) >= RESUME_AFTER_YIELD_FROM); return Py_NewRef(_PyFrame_StackPeek(frame)); } return NULL; diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 2c1f1fce18b558..f487e95854b7fa 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -149,7 +149,7 @@ dummy_func( next_instr = this_instr; } else { - if (!RESUME_WITH_SUBITERATOR(oparg)) { + if ((oparg & RESUME_OPARG_LOCATION_MASK) < RESUME_AFTER_YIELD_FROM) { CHECK_EVAL_BREAKER(); } this_instr->op.code = RESUME_CHECK; @@ -177,7 +177,7 @@ dummy_func( next_instr = this_instr; } else { - if (!RESUME_WITH_SUBITERATOR(oparg)) { + if ((oparg & RESUME_OPARG_LOCATION_MASK) < RESUME_AFTER_YIELD_FROM) { CHECK_EVAL_BREAKER(); } _PyFrame_SetStackPointer(frame, stack_pointer); diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index cd66bb2fd28bc9..12e4f5204cc3dc 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -35,7 +35,7 @@ next_instr = this_instr; } else { - if (!RESUME_WITH_SUBITERATOR(oparg)) { + if ((oparg & RESUME_OPARG_LOCATION_MASK) < RESUME_AFTER_YIELD_FROM) { CHECK_EVAL_BREAKER(); } this_instr->op.code = RESUME_CHECK; @@ -71,7 +71,7 @@ next_instr = this_instr; } else { - if (!RESUME_WITH_SUBITERATOR(oparg)) { + if ((oparg & RESUME_OPARG_LOCATION_MASK) < RESUME_AFTER_YIELD_FROM) { CHECK_EVAL_BREAKER(); } _PyFrame_SetStackPointer(frame, stack_pointer); From 1f305195d8a0fcf02ce6c4e08cf9421ef563b208 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Mon, 6 Nov 2023 21:55:16 +0000 Subject: [PATCH 7/7] FRAME_STATE_CLOSED --> FRAME_STATE_FINISHED --- Include/internal/pycore_frame.h | 2 +- Objects/genobject.c | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Include/internal/pycore_frame.h b/Include/internal/pycore_frame.h index 21d874f16aa00e..0f9e7333cf1e1c 100644 --- a/Include/internal/pycore_frame.h +++ b/Include/internal/pycore_frame.h @@ -45,7 +45,7 @@ typedef enum _framestate { } PyFrameState; #define FRAME_STATE_SUSPENDED(S) ((S) == FRAME_SUSPENDED || (S) == FRAME_SUSPENDED_YIELD_FROM) -#define FRAME_STATE_CLOSED(S) ((S) >= FRAME_COMPLETED) +#define FRAME_STATE_FINISHED(S) ((S) >= FRAME_COMPLETED) enum _frameowner { FRAME_OWNED_BY_THREAD = 0, diff --git a/Objects/genobject.c b/Objects/genobject.c index ef63485a394a9c..f98aa357cd2ce1 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -65,7 +65,7 @@ _PyGen_Finalize(PyObject *self) { PyGenObject *gen = (PyGenObject *)self; - if (FRAME_STATE_CLOSED(gen->gi_frame_state)) { + if (FRAME_STATE_FINISHED(gen->gi_frame_state)) { /* Generator isn't paused, so no need to close */ return; } @@ -188,7 +188,7 @@ gen_send_ex2(PyGenObject *gen, PyObject *arg, PyObject **presult, PyErr_SetString(PyExc_ValueError, msg); return PYGEN_ERROR; } - if (FRAME_STATE_CLOSED(gen->gi_frame_state)) { + if (FRAME_STATE_FINISHED(gen->gi_frame_state)) { if (PyCoro_CheckExact(gen) && !closing) { /* `gen` is an exhausted coroutine: raise an error, except when called from gen_close(), which should @@ -358,7 +358,7 @@ gen_close(PyGenObject *gen, PyObject *args) gen->gi_frame_state = FRAME_COMPLETED; Py_RETURN_NONE; } - if (FRAME_STATE_CLOSED(gen->gi_frame_state)) { + if (FRAME_STATE_FINISHED(gen->gi_frame_state)) { Py_RETURN_NONE; } PyObject *yf = _PyGen_yf(gen); @@ -2087,7 +2087,7 @@ async_gen_athrow_send(PyAsyncGenAThrow *o, PyObject *arg) return NULL; } - if (FRAME_STATE_CLOSED(gen->gi_frame_state)) { + if (FRAME_STATE_FINISHED(gen->gi_frame_state)) { o->agt_state = AWAITABLE_STATE_CLOSED; PyErr_SetNone(PyExc_StopIteration); return NULL;