From e73d0cfb68386743b948c38d8e959016a6235b0d Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Wed, 4 Jan 2023 22:39:29 +0000 Subject: [PATCH 1/4] gh-100758: Refactor initialisation of frame headers into a single function (_PyFrame_InitializeHeader) --- Include/internal/pycore_frame.h | 9 +++++++-- Objects/frameobject.c | 7 ++----- Python/bytecodes.c | 15 --------------- Python/ceval.c | 5 +---- Python/generated_cases.c.h | 15 --------------- 5 files changed, 10 insertions(+), 41 deletions(-) diff --git a/Include/internal/pycore_frame.h b/Include/internal/pycore_frame.h index f18723b303224f..f13cb32afbc277 100644 --- a/Include/internal/pycore_frame.h +++ b/Include/internal/pycore_frame.h @@ -101,7 +101,7 @@ void _PyFrame_Copy(_PyInterpreterFrame *src, _PyInterpreterFrame *dest); when frame is linked into the frame stack. */ static inline void -_PyFrame_InitializeSpecials( +_PyFrame_InitializeHeader( _PyInterpreterFrame *frame, PyFunctionObject *func, PyObject *locals, PyCodeObject *code) { @@ -115,6 +115,11 @@ _PyFrame_InitializeSpecials( frame->prev_instr = _PyCode_CODE(code) - 1; frame->yield_offset = 0; frame->owner = FRAME_OWNED_BY_THREAD; + + PyObject **localsarray = &frame->localsplus[0]; + for (int i = 0; i < code->co_nlocalsplus; i++) { + localsarray[i] = NULL; + } } /* Gets the pointer to the locals array @@ -222,7 +227,7 @@ _PyFrame_PushUnchecked(PyThreadState *tstate, PyFunctionObject *func) _PyInterpreterFrame *new_frame = (_PyInterpreterFrame *)tstate->datastack_top; tstate->datastack_top += code->co_framesize; assert(tstate->datastack_top < tstate->datastack_limit); - _PyFrame_InitializeSpecials(new_frame, func, NULL, code); + _PyFrame_InitializeHeader(new_frame, func, NULL, code); return new_frame; } diff --git a/Objects/frameobject.c b/Objects/frameobject.c index eab85c08fc0165..d4790a351034cc 100644 --- a/Objects/frameobject.c +++ b/Objects/frameobject.c @@ -1011,12 +1011,9 @@ static void init_frame(_PyInterpreterFrame *frame, PyFunctionObject *func, PyObject *locals) { PyCodeObject *code = (PyCodeObject *)func->func_code; - _PyFrame_InitializeSpecials(frame, (PyFunctionObject*)Py_NewRef(func), - Py_XNewRef(locals), code); + _PyFrame_InitializeHeader(frame, (PyFunctionObject*)Py_NewRef(func), + Py_XNewRef(locals), code); frame->previous = NULL; - for (Py_ssize_t i = 0; i < code->co_nlocalsplus; i++) { - frame->localsplus[i] = NULL; - } } PyFrameObject* diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 839fac3fcd1176..a9f3168258310b 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -457,9 +457,6 @@ dummy_func( STACK_SHRINK(2); new_frame->localsplus[0] = container; new_frame->localsplus[1] = sub; - for (int i = 2; i < code->co_nlocalsplus; i++) { - new_frame->localsplus[i] = NULL; - } JUMPBY(INLINE_CACHE_ENTRIES_BINARY_SUBSCR); DISPATCH_INLINED(new_frame); } @@ -1787,9 +1784,6 @@ dummy_func( int shrink_stack = !(oparg & 1); STACK_SHRINK(shrink_stack); new_frame->localsplus[0] = owner; - for (int i = 1; i < code->co_nlocalsplus; i++) { - new_frame->localsplus[i] = NULL; - } JUMPBY(INLINE_CACHE_ENTRIES_LOAD_ATTR); DISPATCH_INLINED(new_frame); } @@ -1823,9 +1817,6 @@ dummy_func( STACK_SHRINK(shrink_stack); new_frame->localsplus[0] = owner; new_frame->localsplus[1] = Py_NewRef(name); - for (int i = 2; i < code->co_nlocalsplus; i++) { - new_frame->localsplus[i] = NULL; - } JUMPBY(INLINE_CACHE_ENTRIES_LOAD_ATTR); DISPATCH_INLINED(new_frame); } @@ -2793,9 +2784,6 @@ dummy_func( for (int i = 0; i < argcount; i++) { new_frame->localsplus[i] = stack_pointer[i]; } - for (int i = argcount; i < code->co_nlocalsplus; i++) { - new_frame->localsplus[i] = NULL; - } STACK_SHRINK(2-is_meth); JUMPBY(INLINE_CACHE_ENTRIES_CALL); DISPATCH_INLINED(new_frame); @@ -2828,9 +2816,6 @@ dummy_func( i - minargs); new_frame->localsplus[i] = Py_NewRef(def); } - for (int i = code->co_argcount; i < code->co_nlocalsplus; i++) { - new_frame->localsplus[i] = NULL; - } STACK_SHRINK(2-is_meth); JUMPBY(INLINE_CACHE_ENTRIES_CALL); DISPATCH_INLINED(new_frame); diff --git a/Python/ceval.c b/Python/ceval.c index 45f42800d7ce58..503891f481ccfc 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -1974,11 +1974,8 @@ _PyEvalFramePushAndInit(PyThreadState *tstate, PyFunctionObject *func, if (frame == NULL) { goto fail; } - _PyFrame_InitializeSpecials(frame, func, locals, code); + _PyFrame_InitializeHeader(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); diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index ed89e90b7c564d..93b3d0393b3c80 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -578,9 +578,6 @@ STACK_SHRINK(2); new_frame->localsplus[0] = container; new_frame->localsplus[1] = sub; - for (int i = 2; i < code->co_nlocalsplus; i++) { - new_frame->localsplus[i] = NULL; - } JUMPBY(INLINE_CACHE_ENTRIES_BINARY_SUBSCR); DISPATCH_INLINED(new_frame); } @@ -2027,9 +2024,6 @@ int shrink_stack = !(oparg & 1); STACK_SHRINK(shrink_stack); new_frame->localsplus[0] = owner; - for (int i = 1; i < code->co_nlocalsplus; i++) { - new_frame->localsplus[i] = NULL; - } JUMPBY(INLINE_CACHE_ENTRIES_LOAD_ATTR); DISPATCH_INLINED(new_frame); } @@ -2062,9 +2056,6 @@ STACK_SHRINK(shrink_stack); new_frame->localsplus[0] = owner; new_frame->localsplus[1] = Py_NewRef(name); - for (int i = 2; i < code->co_nlocalsplus; i++) { - new_frame->localsplus[i] = NULL; - } JUMPBY(INLINE_CACHE_ENTRIES_LOAD_ATTR); DISPATCH_INLINED(new_frame); } @@ -3148,9 +3139,6 @@ for (int i = 0; i < argcount; i++) { new_frame->localsplus[i] = stack_pointer[i]; } - for (int i = argcount; i < code->co_nlocalsplus; i++) { - new_frame->localsplus[i] = NULL; - } STACK_SHRINK(2-is_meth); JUMPBY(INLINE_CACHE_ENTRIES_CALL); DISPATCH_INLINED(new_frame); @@ -3182,9 +3170,6 @@ i - minargs); new_frame->localsplus[i] = Py_NewRef(def); } - for (int i = code->co_argcount; i < code->co_nlocalsplus; i++) { - new_frame->localsplus[i] = NULL; - } STACK_SHRINK(2-is_meth); JUMPBY(INLINE_CACHE_ENTRIES_CALL); DISPATCH_INLINED(new_frame); From 3d020c57ce10c8f19f56e3795afb2e2cde3dab60 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Thu, 5 Jan 2023 13:01:10 +0000 Subject: [PATCH 2/4] review comment: arg from where to NULL. Rename function. --- Include/internal/pycore_frame.h | 13 ++++++------- Objects/frameobject.c | 4 ++-- Python/bytecodes.c | 10 +++++----- Python/ceval.c | 2 +- Python/generated_cases.c.h | 10 +++++----- 5 files changed, 19 insertions(+), 20 deletions(-) diff --git a/Include/internal/pycore_frame.h b/Include/internal/pycore_frame.h index f13cb32afbc277..c843a3e75cd783 100644 --- a/Include/internal/pycore_frame.h +++ b/Include/internal/pycore_frame.h @@ -101,9 +101,9 @@ void _PyFrame_Copy(_PyInterpreterFrame *src, _PyInterpreterFrame *dest); when frame is linked into the frame stack. */ static inline void -_PyFrame_InitializeHeader( +_PyFrame_Initialize( _PyInterpreterFrame *frame, PyFunctionObject *func, - PyObject *locals, PyCodeObject *code) + PyObject *locals, PyCodeObject *code, int null_locals_from) { frame->f_funcobj = (PyObject *)func; frame->f_code = (PyCodeObject *)Py_NewRef(code); @@ -116,9 +116,8 @@ _PyFrame_InitializeHeader( frame->yield_offset = 0; frame->owner = FRAME_OWNED_BY_THREAD; - PyObject **localsarray = &frame->localsplus[0]; - for (int i = 0; i < code->co_nlocalsplus; i++) { - localsarray[i] = NULL; + for (int i = null_locals_from; i < code->co_nlocalsplus; i++) { + frame->localsplus[i] = NULL; } } @@ -220,14 +219,14 @@ void _PyThreadState_PopFrame(PyThreadState *tstate, _PyInterpreterFrame *frame); * Must be guarded by _PyThreadState_HasStackSpace() * Consumes reference to func. */ static inline _PyInterpreterFrame * -_PyFrame_PushUnchecked(PyThreadState *tstate, PyFunctionObject *func) +_PyFrame_PushUnchecked(PyThreadState *tstate, PyFunctionObject *func, int null_locals_from) { CALL_STAT_INC(frames_pushed); PyCodeObject *code = (PyCodeObject *)func->func_code; _PyInterpreterFrame *new_frame = (_PyInterpreterFrame *)tstate->datastack_top; tstate->datastack_top += code->co_framesize; assert(tstate->datastack_top < tstate->datastack_limit); - _PyFrame_InitializeHeader(new_frame, func, NULL, code); + _PyFrame_Initialize(new_frame, func, NULL, code, null_locals_from); return new_frame; } diff --git a/Objects/frameobject.c b/Objects/frameobject.c index d4790a351034cc..f01525f588cea0 100644 --- a/Objects/frameobject.c +++ b/Objects/frameobject.c @@ -1011,8 +1011,8 @@ static void init_frame(_PyInterpreterFrame *frame, PyFunctionObject *func, PyObject *locals) { PyCodeObject *code = (PyCodeObject *)func->func_code; - _PyFrame_InitializeHeader(frame, (PyFunctionObject*)Py_NewRef(func), - Py_XNewRef(locals), code); + _PyFrame_Initialize(frame, (PyFunctionObject*)Py_NewRef(func), + Py_XNewRef(locals), code, 0); frame->previous = NULL; } diff --git a/Python/bytecodes.c b/Python/bytecodes.c index a9f3168258310b..909bfed4c69088 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -453,7 +453,7 @@ dummy_func( DEOPT_IF(!_PyThreadState_HasStackSpace(tstate, code->co_framesize), BINARY_SUBSCR); STAT_INC(BINARY_SUBSCR, hit); Py_INCREF(getitem); - _PyInterpreterFrame *new_frame = _PyFrame_PushUnchecked(tstate, getitem); + _PyInterpreterFrame *new_frame = _PyFrame_PushUnchecked(tstate, getitem, 2); STACK_SHRINK(2); new_frame->localsplus[0] = container; new_frame->localsplus[1] = sub; @@ -1779,7 +1779,7 @@ dummy_func( DEOPT_IF(!_PyThreadState_HasStackSpace(tstate, code->co_framesize), LOAD_ATTR); STAT_INC(LOAD_ATTR, hit); Py_INCREF(fget); - _PyInterpreterFrame *new_frame = _PyFrame_PushUnchecked(tstate, f); + _PyInterpreterFrame *new_frame = _PyFrame_PushUnchecked(tstate, f, 1); SET_TOP(NULL); int shrink_stack = !(oparg & 1); STACK_SHRINK(shrink_stack); @@ -1811,7 +1811,7 @@ dummy_func( PyObject *name = GETITEM(names, oparg >> 1); Py_INCREF(f); - _PyInterpreterFrame *new_frame = _PyFrame_PushUnchecked(tstate, f); + _PyInterpreterFrame *new_frame = _PyFrame_PushUnchecked(tstate, f, 2); SET_TOP(NULL); int shrink_stack = !(oparg & 1); STACK_SHRINK(shrink_stack); @@ -2779,7 +2779,7 @@ dummy_func( DEOPT_IF(code->co_argcount != argcount, CALL); DEOPT_IF(!_PyThreadState_HasStackSpace(tstate, code->co_framesize), CALL); STAT_INC(CALL, hit); - _PyInterpreterFrame *new_frame = _PyFrame_PushUnchecked(tstate, func); + _PyInterpreterFrame *new_frame = _PyFrame_PushUnchecked(tstate, func, argcount); STACK_SHRINK(argcount); for (int i = 0; i < argcount; i++) { new_frame->localsplus[i] = stack_pointer[i]; @@ -2806,7 +2806,7 @@ dummy_func( DEOPT_IF(argcount < minargs, CALL); DEOPT_IF(!_PyThreadState_HasStackSpace(tstate, code->co_framesize), CALL); STAT_INC(CALL, hit); - _PyInterpreterFrame *new_frame = _PyFrame_PushUnchecked(tstate, func); + _PyInterpreterFrame *new_frame = _PyFrame_PushUnchecked(tstate, func, argcount); STACK_SHRINK(argcount); for (int i = 0; i < argcount; i++) { new_frame->localsplus[i] = stack_pointer[i]; diff --git a/Python/ceval.c b/Python/ceval.c index 503891f481ccfc..c644ff8c7eb9f5 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -1974,7 +1974,7 @@ _PyEvalFramePushAndInit(PyThreadState *tstate, PyFunctionObject *func, if (frame == NULL) { goto fail; } - _PyFrame_InitializeHeader(frame, func, locals, code); + _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); diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 93b3d0393b3c80..723404e3504f3b 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -574,7 +574,7 @@ DEOPT_IF(!_PyThreadState_HasStackSpace(tstate, code->co_framesize), BINARY_SUBSCR); STAT_INC(BINARY_SUBSCR, hit); Py_INCREF(getitem); - _PyInterpreterFrame *new_frame = _PyFrame_PushUnchecked(tstate, getitem); + _PyInterpreterFrame *new_frame = _PyFrame_PushUnchecked(tstate, getitem, 2); STACK_SHRINK(2); new_frame->localsplus[0] = container; new_frame->localsplus[1] = sub; @@ -2019,7 +2019,7 @@ DEOPT_IF(!_PyThreadState_HasStackSpace(tstate, code->co_framesize), LOAD_ATTR); STAT_INC(LOAD_ATTR, hit); Py_INCREF(fget); - _PyInterpreterFrame *new_frame = _PyFrame_PushUnchecked(tstate, f); + _PyInterpreterFrame *new_frame = _PyFrame_PushUnchecked(tstate, f, 1); SET_TOP(NULL); int shrink_stack = !(oparg & 1); STACK_SHRINK(shrink_stack); @@ -2050,7 +2050,7 @@ PyObject *name = GETITEM(names, oparg >> 1); Py_INCREF(f); - _PyInterpreterFrame *new_frame = _PyFrame_PushUnchecked(tstate, f); + _PyInterpreterFrame *new_frame = _PyFrame_PushUnchecked(tstate, f, 2); SET_TOP(NULL); int shrink_stack = !(oparg & 1); STACK_SHRINK(shrink_stack); @@ -3134,7 +3134,7 @@ DEOPT_IF(code->co_argcount != argcount, CALL); DEOPT_IF(!_PyThreadState_HasStackSpace(tstate, code->co_framesize), CALL); STAT_INC(CALL, hit); - _PyInterpreterFrame *new_frame = _PyFrame_PushUnchecked(tstate, func); + _PyInterpreterFrame *new_frame = _PyFrame_PushUnchecked(tstate, func, argcount); STACK_SHRINK(argcount); for (int i = 0; i < argcount; i++) { new_frame->localsplus[i] = stack_pointer[i]; @@ -3160,7 +3160,7 @@ DEOPT_IF(argcount < minargs, CALL); DEOPT_IF(!_PyThreadState_HasStackSpace(tstate, code->co_framesize), CALL); STAT_INC(CALL, hit); - _PyInterpreterFrame *new_frame = _PyFrame_PushUnchecked(tstate, func); + _PyInterpreterFrame *new_frame = _PyFrame_PushUnchecked(tstate, func, argcount); STACK_SHRINK(argcount); for (int i = 0; i < argcount; i++) { new_frame->localsplus[i] = stack_pointer[i]; From c1aa2ccd9312cb472fbaab60ac673d353fb471f8 Mon Sep 17 00:00:00 2001 From: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> Date: Thu, 5 Jan 2023 13:05:00 +0000 Subject: [PATCH 3/4] argcount --> code->co_argcount --- Python/bytecodes.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 909bfed4c69088..251004b586a570 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -2806,7 +2806,7 @@ dummy_func( DEOPT_IF(argcount < minargs, CALL); DEOPT_IF(!_PyThreadState_HasStackSpace(tstate, code->co_framesize), CALL); STAT_INC(CALL, hit); - _PyInterpreterFrame *new_frame = _PyFrame_PushUnchecked(tstate, func, argcount); + _PyInterpreterFrame *new_frame = _PyFrame_PushUnchecked(tstate, func, code->co_argcount); STACK_SHRINK(argcount); for (int i = 0; i < argcount; i++) { new_frame->localsplus[i] = stack_pointer[i]; From edc2c1a69cede6eff18a3c7f38b9e236fff0aa81 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Thu, 5 Jan 2023 13:07:05 +0000 Subject: [PATCH 4/4] regen cases --- Python/generated_cases.c.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 723404e3504f3b..954c010de91d24 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -3160,7 +3160,7 @@ DEOPT_IF(argcount < minargs, CALL); DEOPT_IF(!_PyThreadState_HasStackSpace(tstate, code->co_framesize), CALL); STAT_INC(CALL, hit); - _PyInterpreterFrame *new_frame = _PyFrame_PushUnchecked(tstate, func, argcount); + _PyInterpreterFrame *new_frame = _PyFrame_PushUnchecked(tstate, func, code->co_argcount); STACK_SHRINK(argcount); for (int i = 0; i < argcount; i++) { new_frame->localsplus[i] = stack_pointer[i];