From 6788303f5c57273c054d2b9e94e478051d7c8f8d Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Sun, 13 Nov 2022 15:37:03 +0100 Subject: [PATCH] gh-91248: Optimize PyFrame_GetVar() (#99252) PyFrame_GetVar() no longer creates a temporary dictionary to get a variable. --- Doc/c-api/frame.rst | 2 + Lib/test/test_frame.py | 6 ++ Objects/frameobject.c | 231 ++++++++++++++++++++++++----------------- 3 files changed, 144 insertions(+), 95 deletions(-) diff --git a/Doc/c-api/frame.rst b/Doc/c-api/frame.rst index 4a062dd8623c47..b81faab1d97009 100644 --- a/Doc/c-api/frame.rst +++ b/Doc/c-api/frame.rst @@ -87,6 +87,8 @@ See also :ref:`Reflection `. * Raise :exc:`NameError` and return ``NULL`` if the variable does not exist. * Raise an exception and return ``NULL`` on error. + *name* type must be a :class:`str`. + .. versionadded:: 3.12 diff --git a/Lib/test/test_frame.py b/Lib/test/test_frame.py index ada96668392426..a7db22007dedce 100644 --- a/Lib/test/test_frame.py +++ b/Lib/test/test_frame.py @@ -352,6 +352,12 @@ def test_getvar(self): with self.assertRaises(NameError): _testcapi.frame_getvarstring(current_frame, b"y") + # wrong name type + with self.assertRaises(TypeError): + _testcapi.frame_getvar(current_frame, b'x') + with self.assertRaises(TypeError): + _testcapi.frame_getvar(current_frame, 123) + def getgenframe(self): yield sys._getframe() diff --git a/Objects/frameobject.c b/Objects/frameobject.c index 287d5bd9ad1afd..15e1928a547f8a 100644 --- a/Objects/frameobject.c +++ b/Objects/frameobject.c @@ -1109,81 +1109,106 @@ _PyFrame_OpAlreadyRan(_PyInterpreterFrame *frame, int opcode, int oparg) return 0; } + +// Initialize frame free variables if needed +static void +frame_init_get_vars(_PyInterpreterFrame *frame) +{ + // COPY_FREE_VARS has no quickened forms, so no need to use _PyOpcode_Deopt + // here: + PyCodeObject *co = frame->f_code; + int lasti = _PyInterpreterFrame_LASTI(frame); + if (!(lasti < 0 && _Py_OPCODE(_PyCode_CODE(co)[0]) == COPY_FREE_VARS + && PyFunction_Check(frame->f_funcobj))) + { + /* Free vars are initialized */ + return; + } + + /* Free vars have not been initialized -- Do that */ + PyObject *closure = ((PyFunctionObject *)frame->f_funcobj)->func_closure; + int offset = co->co_nlocals + co->co_nplaincellvars; + for (int i = 0; i < co->co_nfreevars; ++i) { + PyObject *o = PyTuple_GET_ITEM(closure, i); + frame->localsplus[offset + i] = Py_NewRef(o); + } + // COPY_FREE_VARS doesn't have inline CACHEs, either: + frame->prev_instr = _PyCode_CODE(frame->f_code); +} + + +static int +frame_get_var(_PyInterpreterFrame *frame, PyCodeObject *co, int i, + PyObject **pvalue) +{ + _PyLocals_Kind kind = _PyLocals_GetKind(co->co_localspluskinds, i); + + /* If the namespace is unoptimized, then one of the + following cases applies: + 1. It does not contain free variables, because it + uses import * or is a top-level namespace. + 2. It is a class namespace. + We don't want to accidentally copy free variables + into the locals dict used by the class. + */ + if (kind & CO_FAST_FREE && !(co->co_flags & CO_OPTIMIZED)) { + return 0; + } + + PyObject *value = frame->localsplus[i]; + if (frame->stacktop) { + if (kind & CO_FAST_FREE) { + // The cell was set by COPY_FREE_VARS. + assert(value != NULL && PyCell_Check(value)); + value = PyCell_GET(value); + } + else if (kind & CO_FAST_CELL) { + // Note that no *_DEREF ops can happen before MAKE_CELL + // executes. So there's no need to duplicate the work + // that MAKE_CELL would otherwise do later, if it hasn't + // run yet. + if (value != NULL) { + if (PyCell_Check(value) && + _PyFrame_OpAlreadyRan(frame, MAKE_CELL, i)) { + // (likely) MAKE_CELL must have executed already. + value = PyCell_GET(value); + } + // (likely) Otherwise it it is an arg (kind & CO_FAST_LOCAL), + // with the initial value set when the frame was created... + // (unlikely) ...or it was set to some initial value by + // an earlier call to PyFrame_LocalsToFast(). + } + } + } + else { + assert(value == NULL); + } + *pvalue = value; + return 1; +} + int -_PyFrame_FastToLocalsWithError(_PyInterpreterFrame *frame) { +_PyFrame_FastToLocalsWithError(_PyInterpreterFrame *frame) +{ /* Merge fast locals into f->f_locals */ - PyObject *locals; - PyObject **fast; - PyCodeObject *co; - locals = frame->f_locals; + PyObject *locals = frame->f_locals; if (locals == NULL) { locals = frame->f_locals = PyDict_New(); - if (locals == NULL) + if (locals == NULL) { return -1; - } - co = frame->f_code; - fast = _PyFrame_GetLocalsArray(frame); - // COPY_FREE_VARS has no quickened forms, so no need to use _PyOpcode_Deopt - // here: - int lasti = _PyInterpreterFrame_LASTI(frame); - if (lasti < 0 && _Py_OPCODE(_PyCode_CODE(co)[0]) == COPY_FREE_VARS - && PyFunction_Check(frame->f_funcobj)) - { - /* Free vars have not been initialized -- Do that */ - PyCodeObject *co = frame->f_code; - PyObject *closure = ((PyFunctionObject *)frame->f_funcobj)->func_closure; - int offset = co->co_nlocals + co->co_nplaincellvars; - for (int i = 0; i < co->co_nfreevars; ++i) { - PyObject *o = PyTuple_GET_ITEM(closure, i); - frame->localsplus[offset + i] = Py_NewRef(o); } - // COPY_FREE_VARS doesn't have inline CACHEs, either: - frame->prev_instr = _PyCode_CODE(frame->f_code); } - for (int i = 0; i < co->co_nlocalsplus; i++) { - _PyLocals_Kind kind = _PyLocals_GetKind(co->co_localspluskinds, i); - /* If the namespace is unoptimized, then one of the - following cases applies: - 1. It does not contain free variables, because it - uses import * or is a top-level namespace. - 2. It is a class namespace. - We don't want to accidentally copy free variables - into the locals dict used by the class. - */ - if (kind & CO_FAST_FREE && !(co->co_flags & CO_OPTIMIZED)) { + frame_init_get_vars(frame); + + PyCodeObject *co = frame->f_code; + for (int i = 0; i < co->co_nlocalsplus; i++) { + PyObject *value; // borrowed reference + if (!frame_get_var(frame, co, i, &value)) { continue; } PyObject *name = PyTuple_GET_ITEM(co->co_localsplusnames, i); - PyObject *value = fast[i]; - if (frame->stacktop) { - if (kind & CO_FAST_FREE) { - // The cell was set by COPY_FREE_VARS. - assert(value != NULL && PyCell_Check(value)); - value = PyCell_GET(value); - } - else if (kind & CO_FAST_CELL) { - // Note that no *_DEREF ops can happen before MAKE_CELL - // executes. So there's no need to duplicate the work - // that MAKE_CELL would otherwise do later, if it hasn't - // run yet. - if (value != NULL) { - if (PyCell_Check(value) && - _PyFrame_OpAlreadyRan(frame, MAKE_CELL, i)) { - // (likely) MAKE_CELL must have executed already. - value = PyCell_GET(value); - } - // (likely) Otherwise it it is an arg (kind & CO_FAST_LOCAL), - // with the initial value set when the frame was created... - // (unlikely) ...or it was set to some initial value by - // an earlier call to PyFrame_LocalsToFast(). - } - } - } - else { - assert(value == NULL); - } if (value == NULL) { if (PyObject_DelItem(locals, name) != 0) { if (PyErr_ExceptionMatches(PyExc_KeyError)) { @@ -1203,6 +1228,54 @@ _PyFrame_FastToLocalsWithError(_PyInterpreterFrame *frame) { return 0; } + +PyObject * +PyFrame_GetVar(PyFrameObject *frame_obj, PyObject *name) +{ + if (!PyUnicode_Check(name)) { + PyErr_Format(PyExc_TypeError, "name must be str, not %s", + Py_TYPE(name)->tp_name); + return NULL; + } + + _PyInterpreterFrame *frame = frame_obj->f_frame; + frame_init_get_vars(frame); + + PyCodeObject *co = frame->f_code; + for (int i = 0; i < co->co_nlocalsplus; i++) { + PyObject *var_name = PyTuple_GET_ITEM(co->co_localsplusnames, i); + if (!_PyUnicode_Equal(var_name, name)) { + continue; + } + + PyObject *value; // borrowed reference + if (!frame_get_var(frame, co, i, &value)) { + break; + } + if (value == NULL) { + break; + } + return Py_NewRef(value); + } + + PyErr_Format(PyExc_NameError, "variable %R does not exist", name); + return NULL; +} + + +PyObject * +PyFrame_GetVarString(PyFrameObject *frame, const char *name) +{ + PyObject *name_obj = PyUnicode_FromString(name); + if (name_obj == NULL) { + return NULL; + } + PyObject *value = PyFrame_GetVar(frame, name_obj); + Py_DECREF(name_obj); + return value; +} + + int PyFrame_FastToLocalsWithError(PyFrameObject *f) { @@ -1413,35 +1486,3 @@ _PyEval_BuiltinsFromGlobals(PyThreadState *tstate, PyObject *globals) return _PyEval_GetBuiltins(tstate); } - -PyObject * -PyFrame_GetVar(PyFrameObject *frame, PyObject *name) -{ - PyObject *locals = PyFrame_GetLocals(frame); - if (locals == NULL) { - return NULL; - } - PyObject *value = PyDict_GetItemWithError(locals, name); - Py_DECREF(locals); - - if (value == NULL) { - if (PyErr_Occurred()) { - return NULL; - } - PyErr_Format(PyExc_NameError, "variable %R does not exist", name); - return NULL; - } - return Py_NewRef(value); -} - -PyObject * -PyFrame_GetVarString(PyFrameObject *frame, const char *name) -{ - PyObject *name_obj = PyUnicode_FromString(name); - if (name_obj == NULL) { - return NULL; - } - PyObject *value = PyFrame_GetVar(frame, name_obj); - Py_DECREF(name_obj); - return value; -}