From 9cc3f1f72321cfbc085d2d0c610b24bf2d34ac7a Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Tue, 26 Oct 2021 16:41:58 +0100 Subject: [PATCH 1/2] Tighten up Python-to-Python calls. --- Include/internal/pycore_frame.h | 15 ++++ Objects/frameobject.c | 11 ++- Python/ceval.c | 141 +++++++++++++++++++------------- Python/pystate.c | 22 +++-- 4 files changed, 120 insertions(+), 69 deletions(-) diff --git a/Include/internal/pycore_frame.h b/Include/internal/pycore_frame.h index 7e63f584eb3b01..b025ca0b8d766c 100644 --- a/Include/internal/pycore_frame.h +++ b/Include/internal/pycore_frame.h @@ -152,6 +152,21 @@ _PyFrame_LocalsToFast(InterpreterFrame *frame, int clear); InterpreterFrame *_PyThreadState_PushFrame( PyThreadState *tstate, PyFrameConstructor *con, PyObject *locals); +extern InterpreterFrame * +_PyThreadState_BumpFramePointerSlow(PyThreadState *tstate, size_t size); + +static inline InterpreterFrame * +_PyThreadState_BumpFramePointer(PyThreadState *tstate, size_t size) +{ + PyObject **base = tstate->datastack_top; + PyObject **top = base + size; + if (top < tstate->datastack_limit) { + tstate->datastack_top = top; + return (InterpreterFrame *)base; + } + return _PyThreadState_BumpFramePointerSlow(tstate, size); +} + void _PyThreadState_PopFrame(PyThreadState *tstate, InterpreterFrame *frame); #ifdef __cplusplus diff --git a/Objects/frameobject.c b/Objects/frameobject.c index ffe19b3d994007..09857c7fa007d8 100644 --- a/Objects/frameobject.c +++ b/Objects/frameobject.c @@ -786,16 +786,15 @@ allocate_heap_frame(PyFrameConstructor *con, PyObject *locals) { PyCodeObject *code = (PyCodeObject *)con->fc_code; int size = code->co_nlocalsplus+code->co_stacksize + FRAME_SPECIALS_SIZE; - PyObject **localsarray = PyMem_Malloc(sizeof(PyObject *)*size); - if (localsarray == NULL) { + InterpreterFrame *frame = (InterpreterFrame *)PyMem_Malloc(sizeof(PyObject *)*size); + if (frame == NULL) { PyErr_NoMemory(); return NULL; } - for (Py_ssize_t i=0; i < code->co_nlocalsplus; i++) { - localsarray[i] = NULL; - } - InterpreterFrame *frame = (InterpreterFrame *)(localsarray + code->co_nlocalsplus); _PyFrame_InitializeSpecials(frame, con, locals, code->co_nlocalsplus); + for (Py_ssize_t i = 0; i < code->co_nlocalsplus; i++) { + frame->localsplus[i] = NULL; + } return frame; } diff --git a/Python/ceval.c b/Python/ceval.c index adc7b536247b2e..7159f0ad89bad6 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -101,7 +101,7 @@ static int get_exception_handler(PyCodeObject *, int, int*, int*, int*); static InterpreterFrame * _PyEvalFramePushAndInit(PyThreadState *tstate, PyFrameConstructor *con, PyObject *locals, PyObject* const* args, - size_t argcount, PyObject *kwnames, int steal_args); + size_t argcount, PyObject *kwnames); static int _PyEvalFrameClearAndPop(PyThreadState *tstate, InterpreterFrame * frame); @@ -4646,7 +4646,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr InterpreterFrame *new_frame = _PyEvalFramePushAndInit( tstate, PyFunction_AS_FRAME_CONSTRUCTOR(function), locals, stack_pointer, - nargs, kwnames, 1); + nargs, kwnames); STACK_SHRINK(postcall_shrink); // The frame has stolen all the arguments from the stack, // so there is no need to clean them up. @@ -4720,11 +4720,14 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr /* PEP 523 */ DEOPT_IF(tstate->interp->eval_frame != NULL, CALL_FUNCTION); STAT_INC(CALL_FUNCTION, hit); - InterpreterFrame *new_frame = _PyThreadState_PushFrame( - tstate, PyFunction_AS_FRAME_CONSTRUCTOR(func), NULL); + PyCodeObject *code = (PyCodeObject *)func->func_code; + size_t size = code->co_nlocalsplus + code->co_stacksize + FRAME_SPECIALS_SIZE; + InterpreterFrame *new_frame = _PyThreadState_BumpFramePointer(tstate, size); if (new_frame == NULL) { goto error; } + _PyFrame_InitializeSpecials(new_frame, PyFunction_AS_FRAME_CONSTRUCTOR(func), + NULL, code->co_nlocalsplus); STACK_SHRINK(argcount); for (int i = 0; i < argcount; i++) { new_frame->localsplus[i] = stack_pointer[i]; @@ -4735,6 +4738,9 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr Py_INCREF(def); new_frame->localsplus[argcount+i] = def; } + for (int i = argcount+deflen; i < code->co_nlocalsplus; i++) { + new_frame->localsplus[i] = NULL; + } STACK_SHRINK(1); Py_DECREF(func); _PyFrame_SetStackPointer(frame, stack_pointer); @@ -5592,7 +5598,7 @@ get_exception_handler(PyCodeObject *code, int index, int *level, int *handler, i static int initialize_locals(PyThreadState *tstate, PyFrameConstructor *con, PyObject **localsplus, PyObject *const *args, - Py_ssize_t argcount, PyObject *kwnames, int steal_args) + Py_ssize_t argcount, PyObject *kwnames) { PyCodeObject *co = (PyCodeObject*)con->fc_code; const Py_ssize_t total_args = co->co_argcount + co->co_kwonlyargcount; @@ -5626,9 +5632,6 @@ initialize_locals(PyThreadState *tstate, PyFrameConstructor *con, } for (j = 0; j < n; j++) { PyObject *x = args[j]; - if (!steal_args) { - Py_INCREF(x); - } assert(localsplus[j] == NULL); localsplus[j] = x; } @@ -5636,11 +5639,7 @@ initialize_locals(PyThreadState *tstate, PyFrameConstructor *con, /* Pack other positional arguments into the *args argument */ if (co->co_flags & CO_VARARGS) { PyObject *u = NULL; - if (steal_args) { - u = _PyTuple_FromArraySteal(args + n, argcount - n); - } else { - u = _PyTuple_FromArray(args + n, argcount - n); - } + u = _PyTuple_FromArraySteal(args + n, argcount - n); if (u == NULL) { goto fail_post_positional; } @@ -5649,10 +5648,8 @@ initialize_locals(PyThreadState *tstate, PyFrameConstructor *con, } else if (argcount > n) { /* Too many postional args. Error is reported later */ - if (steal_args) { - for (j = n; j < argcount; j++) { - Py_DECREF(args[j]); - } + for (j = n; j < argcount; j++) { + Py_DECREF(args[j]); } } @@ -5714,19 +5711,15 @@ initialize_locals(PyThreadState *tstate, PyFrameConstructor *con, if (PyDict_SetItem(kwdict, keyword, value) == -1) { goto kw_fail; } - if (steal_args) { - Py_DECREF(value); - } + Py_DECREF(value); continue; kw_fail: - if (steal_args) { - for (;i < kwcount; i++) { - PyObject *value = args[i+argcount]; - Py_DECREF(value); - } + for (;i < kwcount; i++) { + PyObject *value = args[i+argcount]; + Py_DECREF(value); } - goto fail_noclean; + goto fail_post_args; kw_found: if (localsplus[j] != NULL) { @@ -5735,9 +5728,6 @@ initialize_locals(PyThreadState *tstate, PyFrameConstructor *con, con->fc_qualname, keyword); goto kw_fail; } - if (!steal_args) { - Py_INCREF(value); - } localsplus[j] = value; } } @@ -5746,7 +5736,7 @@ initialize_locals(PyThreadState *tstate, PyFrameConstructor *con, if ((argcount > co->co_argcount) && !(co->co_flags & CO_VARARGS)) { too_many_positional(tstate, co, argcount, con->fc_defaults, localsplus, con->fc_qualname); - goto fail_noclean; + goto fail_post_args; } /* Add missing positional arguments (copy default values from defs) */ @@ -5762,7 +5752,7 @@ initialize_locals(PyThreadState *tstate, PyFrameConstructor *con, if (missing) { missing_arguments(tstate, co, missing, defcount, localsplus, con->fc_qualname); - goto fail_noclean; + goto fail_post_args; } if (n > m) i = n - m; @@ -5795,7 +5785,7 @@ initialize_locals(PyThreadState *tstate, PyFrameConstructor *con, continue; } else if (_PyErr_Occurred(tstate)) { - goto fail_noclean; + goto fail_post_args; } } missing++; @@ -5803,35 +5793,31 @@ initialize_locals(PyThreadState *tstate, PyFrameConstructor *con, if (missing) { missing_arguments(tstate, co, missing, -1, localsplus, con->fc_qualname); - goto fail_noclean; + goto fail_post_args; } } - /* Copy closure variables to free variables */ for (i = 0; i < co->co_nfreevars; ++i) { PyObject *o = PyTuple_GET_ITEM(con->fc_closure, i); Py_INCREF(o); localsplus[co->co_nlocals + co->co_nplaincellvars + i] = o; } - return 0; fail_pre_positional: - if (steal_args) { - for (j = 0; j < argcount; j++) { - Py_DECREF(args[j]); - } + for (j = 0; j < argcount; j++) { + Py_DECREF(args[j]); } /* fall through */ fail_post_positional: - if (steal_args) { - Py_ssize_t kwcount = kwnames != NULL ? PyTuple_GET_SIZE(kwnames) : 0; + if (kwnames) { + Py_ssize_t kwcount = PyTuple_GET_SIZE(kwnames); for (j = argcount; j < argcount+kwcount; j++) { Py_DECREF(args[j]); } } /* fall through */ -fail_noclean: +fail_post_args: return -1; } @@ -5847,21 +5833,34 @@ make_coro_frame(PyThreadState *tstate, int size = code->co_nlocalsplus+code->co_stacksize + FRAME_SPECIALS_SIZE; InterpreterFrame *frame = (InterpreterFrame *)PyMem_Malloc(sizeof(PyObject *)*size); if (frame == NULL) { - PyErr_NoMemory(); - return NULL; + goto fail; } - for (Py_ssize_t i=0; i < code->co_nlocalsplus; i++) { + _PyFrame_InitializeSpecials(frame, con, locals, code->co_nlocalsplus); + for (int i = 0; i < code->co_nlocalsplus; i++) { frame->localsplus[i] = NULL; } - _PyFrame_InitializeSpecials(frame, con, locals, code->co_nlocalsplus); assert(frame->frame_obj == NULL); - if (initialize_locals(tstate, con, frame->localsplus, args, argcount, kwnames, 0)) { + if (initialize_locals(tstate, con, frame->localsplus, args, argcount, kwnames)) { _PyFrame_Clear(frame, 1); return NULL; } return frame; +fail: + /* Consume the references */ + for (Py_ssize_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; } +/* Consumes all the references to the args */ static PyObject * make_coro(PyThreadState *tstate, PyFrameConstructor *con, PyObject *locals, @@ -5877,30 +5876,46 @@ make_coro(PyThreadState *tstate, PyFrameConstructor *con, if (gen == NULL) { return NULL; } - return gen; } -// If *steal_args* is set, the function will steal the references to all the arguments. -// In case of error, the function returns null and if *steal_args* is set, the caller -// will still own all the arguments. +/* Consumes all the references to the args */ static InterpreterFrame * _PyEvalFramePushAndInit(PyThreadState *tstate, PyFrameConstructor *con, PyObject *locals, PyObject* const* args, - size_t argcount, PyObject *kwnames, int steal_args) + size_t argcount, PyObject *kwnames) { - InterpreterFrame * frame = _PyThreadState_PushFrame(tstate, con, locals); + PyCodeObject * code = (PyCodeObject *)con->fc_code; + size_t size = code->co_nlocalsplus + code->co_stacksize + FRAME_SPECIALS_SIZE; + InterpreterFrame *frame = _PyThreadState_BumpFramePointer(tstate, size); if (frame == NULL) { - return NULL; + goto fail; } - PyObject **localsarray = _PyFrame_GetLocalsArray(frame); - if (initialize_locals(tstate, con, localsarray, args, argcount, kwnames, steal_args)) { + _PyFrame_InitializeSpecials(frame, con, locals, code->co_nlocalsplus); + PyObject **localsarray = &frame->localsplus[0]; + for (int i = 0; i < code->co_nlocalsplus; i++) { + localsarray[i] = NULL; + } + if (initialize_locals(tstate, con, localsarray, args, argcount, kwnames)) { _PyFrame_Clear(frame, 0); return NULL; } frame->previous = tstate->frame; tstate->frame = frame; 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 int @@ -5925,13 +5940,25 @@ _PyEval_Vector(PyThreadState *tstate, PyFrameConstructor *con, PyObject *kwnames) { PyCodeObject *code = (PyCodeObject *)con->fc_code; + /* _PyEvalFramePushAndInit and make_coro consume + * all the references to their arguments + */ + for (size_t i = 0; i < argcount; i++) { + Py_INCREF(args[i]); + } + if (kwnames) { + Py_ssize_t kwcount = PyTuple_GET_SIZE(kwnames); + for (Py_ssize_t i = 0; i < kwcount; i++) { + Py_INCREF(args[i+argcount]); + } + } int is_coro = code->co_flags & (CO_GENERATOR | CO_COROUTINE | CO_ASYNC_GENERATOR); if (is_coro) { return make_coro(tstate, con, locals, args, argcount, kwnames); } InterpreterFrame *frame = _PyEvalFramePushAndInit( - tstate, con, locals, args, argcount, kwnames, 0); + tstate, con, locals, args, argcount, kwnames); if (frame == NULL) { return NULL; } diff --git a/Python/pystate.c b/Python/pystate.c index 7804e17a064e15..a4a78d46c30dab 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -2065,12 +2065,8 @@ push_chunk(PyThreadState *tstate, int size) } InterpreterFrame * -_PyThreadState_PushFrame(PyThreadState *tstate, PyFrameConstructor *con, PyObject *locals) +_PyThreadState_BumpFramePointerSlow(PyThreadState *tstate, size_t size) { - PyCodeObject *code = (PyCodeObject *)con->fc_code; - int nlocalsplus = code->co_nlocalsplus; - size_t size = nlocalsplus + code->co_stacksize + - FRAME_SPECIALS_SIZE; assert(size < INT_MAX/sizeof(PyObject *)); PyObject **base = tstate->datastack_top; PyObject **top = base + size; @@ -2083,7 +2079,21 @@ _PyThreadState_PushFrame(PyThreadState *tstate, PyFrameConstructor *con, PyObjec else { tstate->datastack_top = top; } - InterpreterFrame *frame = (InterpreterFrame *)base; + return (InterpreterFrame *)base; +} + + +InterpreterFrame * +_PyThreadState_PushFrame(PyThreadState *tstate, PyFrameConstructor *con, PyObject *locals) +{ + PyCodeObject *code = (PyCodeObject *)con->fc_code; + int nlocalsplus = code->co_nlocalsplus; + size_t size = nlocalsplus + code->co_stacksize + + FRAME_SPECIALS_SIZE; + InterpreterFrame *frame = _PyThreadState_BumpFramePointer(tstate, size); + if (frame == NULL) { + return NULL; + } _PyFrame_InitializeSpecials(frame, con, locals, nlocalsplus); for (int i=0; i < nlocalsplus; i++) { frame->localsplus[i] = NULL; From 0a93c9fae8de8fc308eeb6c3b2194eccb969a70b Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Thu, 28 Oct 2021 10:41:30 +0100 Subject: [PATCH 2/2] Rename a label for clarity. --- Python/ceval.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index 7159f0ad89bad6..89a9453499436c 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -5833,7 +5833,7 @@ make_coro_frame(PyThreadState *tstate, int size = code->co_nlocalsplus+code->co_stacksize + FRAME_SPECIALS_SIZE; InterpreterFrame *frame = (InterpreterFrame *)PyMem_Malloc(sizeof(PyObject *)*size); if (frame == NULL) { - goto fail; + goto fail_no_memory; } _PyFrame_InitializeSpecials(frame, con, locals, code->co_nlocalsplus); for (int i = 0; i < code->co_nlocalsplus; i++) { @@ -5845,7 +5845,7 @@ make_coro_frame(PyThreadState *tstate, return NULL; } return frame; -fail: +fail_no_memory: /* Consume the references */ for (Py_ssize_t i = 0; i < argcount; i++) { Py_DECREF(args[i]);