Skip to content

Commit 6788303

Browse files
authored
gh-91248: Optimize PyFrame_GetVar() (#99252)
PyFrame_GetVar() no longer creates a temporary dictionary to get a variable.
1 parent 57be545 commit 6788303

File tree

3 files changed

+144
-95
lines changed

3 files changed

+144
-95
lines changed

Doc/c-api/frame.rst

+2
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,8 @@ See also :ref:`Reflection <reflection>`.
8787
* Raise :exc:`NameError` and return ``NULL`` if the variable does not exist.
8888
* Raise an exception and return ``NULL`` on error.
8989
90+
*name* type must be a :class:`str`.
91+
9092
.. versionadded:: 3.12
9193
9294

Lib/test/test_frame.py

+6
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,12 @@ def test_getvar(self):
352352
with self.assertRaises(NameError):
353353
_testcapi.frame_getvarstring(current_frame, b"y")
354354

355+
# wrong name type
356+
with self.assertRaises(TypeError):
357+
_testcapi.frame_getvar(current_frame, b'x')
358+
with self.assertRaises(TypeError):
359+
_testcapi.frame_getvar(current_frame, 123)
360+
355361
def getgenframe(self):
356362
yield sys._getframe()
357363

Objects/frameobject.c

+136-95
Original file line numberDiff line numberDiff line change
@@ -1109,81 +1109,106 @@ _PyFrame_OpAlreadyRan(_PyInterpreterFrame *frame, int opcode, int oparg)
11091109
return 0;
11101110
}
11111111

1112+
1113+
// Initialize frame free variables if needed
1114+
static void
1115+
frame_init_get_vars(_PyInterpreterFrame *frame)
1116+
{
1117+
// COPY_FREE_VARS has no quickened forms, so no need to use _PyOpcode_Deopt
1118+
// here:
1119+
PyCodeObject *co = frame->f_code;
1120+
int lasti = _PyInterpreterFrame_LASTI(frame);
1121+
if (!(lasti < 0 && _Py_OPCODE(_PyCode_CODE(co)[0]) == COPY_FREE_VARS
1122+
&& PyFunction_Check(frame->f_funcobj)))
1123+
{
1124+
/* Free vars are initialized */
1125+
return;
1126+
}
1127+
1128+
/* Free vars have not been initialized -- Do that */
1129+
PyObject *closure = ((PyFunctionObject *)frame->f_funcobj)->func_closure;
1130+
int offset = co->co_nlocals + co->co_nplaincellvars;
1131+
for (int i = 0; i < co->co_nfreevars; ++i) {
1132+
PyObject *o = PyTuple_GET_ITEM(closure, i);
1133+
frame->localsplus[offset + i] = Py_NewRef(o);
1134+
}
1135+
// COPY_FREE_VARS doesn't have inline CACHEs, either:
1136+
frame->prev_instr = _PyCode_CODE(frame->f_code);
1137+
}
1138+
1139+
1140+
static int
1141+
frame_get_var(_PyInterpreterFrame *frame, PyCodeObject *co, int i,
1142+
PyObject **pvalue)
1143+
{
1144+
_PyLocals_Kind kind = _PyLocals_GetKind(co->co_localspluskinds, i);
1145+
1146+
/* If the namespace is unoptimized, then one of the
1147+
following cases applies:
1148+
1. It does not contain free variables, because it
1149+
uses import * or is a top-level namespace.
1150+
2. It is a class namespace.
1151+
We don't want to accidentally copy free variables
1152+
into the locals dict used by the class.
1153+
*/
1154+
if (kind & CO_FAST_FREE && !(co->co_flags & CO_OPTIMIZED)) {
1155+
return 0;
1156+
}
1157+
1158+
PyObject *value = frame->localsplus[i];
1159+
if (frame->stacktop) {
1160+
if (kind & CO_FAST_FREE) {
1161+
// The cell was set by COPY_FREE_VARS.
1162+
assert(value != NULL && PyCell_Check(value));
1163+
value = PyCell_GET(value);
1164+
}
1165+
else if (kind & CO_FAST_CELL) {
1166+
// Note that no *_DEREF ops can happen before MAKE_CELL
1167+
// executes. So there's no need to duplicate the work
1168+
// that MAKE_CELL would otherwise do later, if it hasn't
1169+
// run yet.
1170+
if (value != NULL) {
1171+
if (PyCell_Check(value) &&
1172+
_PyFrame_OpAlreadyRan(frame, MAKE_CELL, i)) {
1173+
// (likely) MAKE_CELL must have executed already.
1174+
value = PyCell_GET(value);
1175+
}
1176+
// (likely) Otherwise it it is an arg (kind & CO_FAST_LOCAL),
1177+
// with the initial value set when the frame was created...
1178+
// (unlikely) ...or it was set to some initial value by
1179+
// an earlier call to PyFrame_LocalsToFast().
1180+
}
1181+
}
1182+
}
1183+
else {
1184+
assert(value == NULL);
1185+
}
1186+
*pvalue = value;
1187+
return 1;
1188+
}
1189+
11121190
int
1113-
_PyFrame_FastToLocalsWithError(_PyInterpreterFrame *frame) {
1191+
_PyFrame_FastToLocalsWithError(_PyInterpreterFrame *frame)
1192+
{
11141193
/* Merge fast locals into f->f_locals */
1115-
PyObject *locals;
1116-
PyObject **fast;
1117-
PyCodeObject *co;
1118-
locals = frame->f_locals;
1194+
PyObject *locals = frame->f_locals;
11191195
if (locals == NULL) {
11201196
locals = frame->f_locals = PyDict_New();
1121-
if (locals == NULL)
1197+
if (locals == NULL) {
11221198
return -1;
1123-
}
1124-
co = frame->f_code;
1125-
fast = _PyFrame_GetLocalsArray(frame);
1126-
// COPY_FREE_VARS has no quickened forms, so no need to use _PyOpcode_Deopt
1127-
// here:
1128-
int lasti = _PyInterpreterFrame_LASTI(frame);
1129-
if (lasti < 0 && _Py_OPCODE(_PyCode_CODE(co)[0]) == COPY_FREE_VARS
1130-
&& PyFunction_Check(frame->f_funcobj))
1131-
{
1132-
/* Free vars have not been initialized -- Do that */
1133-
PyCodeObject *co = frame->f_code;
1134-
PyObject *closure = ((PyFunctionObject *)frame->f_funcobj)->func_closure;
1135-
int offset = co->co_nlocals + co->co_nplaincellvars;
1136-
for (int i = 0; i < co->co_nfreevars; ++i) {
1137-
PyObject *o = PyTuple_GET_ITEM(closure, i);
1138-
frame->localsplus[offset + i] = Py_NewRef(o);
11391199
}
1140-
// COPY_FREE_VARS doesn't have inline CACHEs, either:
1141-
frame->prev_instr = _PyCode_CODE(frame->f_code);
11421200
}
1143-
for (int i = 0; i < co->co_nlocalsplus; i++) {
1144-
_PyLocals_Kind kind = _PyLocals_GetKind(co->co_localspluskinds, i);
11451201

1146-
/* If the namespace is unoptimized, then one of the
1147-
following cases applies:
1148-
1. It does not contain free variables, because it
1149-
uses import * or is a top-level namespace.
1150-
2. It is a class namespace.
1151-
We don't want to accidentally copy free variables
1152-
into the locals dict used by the class.
1153-
*/
1154-
if (kind & CO_FAST_FREE && !(co->co_flags & CO_OPTIMIZED)) {
1202+
frame_init_get_vars(frame);
1203+
1204+
PyCodeObject *co = frame->f_code;
1205+
for (int i = 0; i < co->co_nlocalsplus; i++) {
1206+
PyObject *value; // borrowed reference
1207+
if (!frame_get_var(frame, co, i, &value)) {
11551208
continue;
11561209
}
11571210

11581211
PyObject *name = PyTuple_GET_ITEM(co->co_localsplusnames, i);
1159-
PyObject *value = fast[i];
1160-
if (frame->stacktop) {
1161-
if (kind & CO_FAST_FREE) {
1162-
// The cell was set by COPY_FREE_VARS.
1163-
assert(value != NULL && PyCell_Check(value));
1164-
value = PyCell_GET(value);
1165-
}
1166-
else if (kind & CO_FAST_CELL) {
1167-
// Note that no *_DEREF ops can happen before MAKE_CELL
1168-
// executes. So there's no need to duplicate the work
1169-
// that MAKE_CELL would otherwise do later, if it hasn't
1170-
// run yet.
1171-
if (value != NULL) {
1172-
if (PyCell_Check(value) &&
1173-
_PyFrame_OpAlreadyRan(frame, MAKE_CELL, i)) {
1174-
// (likely) MAKE_CELL must have executed already.
1175-
value = PyCell_GET(value);
1176-
}
1177-
// (likely) Otherwise it it is an arg (kind & CO_FAST_LOCAL),
1178-
// with the initial value set when the frame was created...
1179-
// (unlikely) ...or it was set to some initial value by
1180-
// an earlier call to PyFrame_LocalsToFast().
1181-
}
1182-
}
1183-
}
1184-
else {
1185-
assert(value == NULL);
1186-
}
11871212
if (value == NULL) {
11881213
if (PyObject_DelItem(locals, name) != 0) {
11891214
if (PyErr_ExceptionMatches(PyExc_KeyError)) {
@@ -1203,6 +1228,54 @@ _PyFrame_FastToLocalsWithError(_PyInterpreterFrame *frame) {
12031228
return 0;
12041229
}
12051230

1231+
1232+
PyObject *
1233+
PyFrame_GetVar(PyFrameObject *frame_obj, PyObject *name)
1234+
{
1235+
if (!PyUnicode_Check(name)) {
1236+
PyErr_Format(PyExc_TypeError, "name must be str, not %s",
1237+
Py_TYPE(name)->tp_name);
1238+
return NULL;
1239+
}
1240+
1241+
_PyInterpreterFrame *frame = frame_obj->f_frame;
1242+
frame_init_get_vars(frame);
1243+
1244+
PyCodeObject *co = frame->f_code;
1245+
for (int i = 0; i < co->co_nlocalsplus; i++) {
1246+
PyObject *var_name = PyTuple_GET_ITEM(co->co_localsplusnames, i);
1247+
if (!_PyUnicode_Equal(var_name, name)) {
1248+
continue;
1249+
}
1250+
1251+
PyObject *value; // borrowed reference
1252+
if (!frame_get_var(frame, co, i, &value)) {
1253+
break;
1254+
}
1255+
if (value == NULL) {
1256+
break;
1257+
}
1258+
return Py_NewRef(value);
1259+
}
1260+
1261+
PyErr_Format(PyExc_NameError, "variable %R does not exist", name);
1262+
return NULL;
1263+
}
1264+
1265+
1266+
PyObject *
1267+
PyFrame_GetVarString(PyFrameObject *frame, const char *name)
1268+
{
1269+
PyObject *name_obj = PyUnicode_FromString(name);
1270+
if (name_obj == NULL) {
1271+
return NULL;
1272+
}
1273+
PyObject *value = PyFrame_GetVar(frame, name_obj);
1274+
Py_DECREF(name_obj);
1275+
return value;
1276+
}
1277+
1278+
12061279
int
12071280
PyFrame_FastToLocalsWithError(PyFrameObject *f)
12081281
{
@@ -1413,35 +1486,3 @@ _PyEval_BuiltinsFromGlobals(PyThreadState *tstate, PyObject *globals)
14131486

14141487
return _PyEval_GetBuiltins(tstate);
14151488
}
1416-
1417-
PyObject *
1418-
PyFrame_GetVar(PyFrameObject *frame, PyObject *name)
1419-
{
1420-
PyObject *locals = PyFrame_GetLocals(frame);
1421-
if (locals == NULL) {
1422-
return NULL;
1423-
}
1424-
PyObject *value = PyDict_GetItemWithError(locals, name);
1425-
Py_DECREF(locals);
1426-
1427-
if (value == NULL) {
1428-
if (PyErr_Occurred()) {
1429-
return NULL;
1430-
}
1431-
PyErr_Format(PyExc_NameError, "variable %R does not exist", name);
1432-
return NULL;
1433-
}
1434-
return Py_NewRef(value);
1435-
}
1436-
1437-
PyObject *
1438-
PyFrame_GetVarString(PyFrameObject *frame, const char *name)
1439-
{
1440-
PyObject *name_obj = PyUnicode_FromString(name);
1441-
if (name_obj == NULL) {
1442-
return NULL;
1443-
}
1444-
PyObject *value = PyFrame_GetVar(frame, name_obj);
1445-
Py_DECREF(name_obj);
1446-
return value;
1447-
}

0 commit comments

Comments
 (0)