Skip to content

Commit

Permalink
GH-100987: Don't cache references to the names and consts array in `_…
Browse files Browse the repository at this point in the history 
…PyEval_EvalFrameDefault`. (#102640)

* Rename local variables, names and consts, from the interpeter loop. Will allow non-code objects in frames for better introspection of C builtins and extensions.

* Remove unused dummy variables.
  • Loading branch information
@markshannon
markshannon authored Mar 13, 2023
1 parent 634cb61 commit 2d370da
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 53 deletions.
44 changes: 21 additions & 23 deletions Python/bytecodes.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,6 @@ dummy_func(
unsigned int oparg,
_Py_atomic_int * const eval_breaker,
_PyCFrame cframe,
PyObject *names,
PyObject *consts,
_Py_CODEUNIT *next_instr,
PyObject **stack_pointer,
PyObject *kwnames,
Expand Down Expand Up @@ -115,7 +113,7 @@ dummy_func(
}

inst(LOAD_CONST, (-- value)) {
value = GETITEM(consts, oparg);
value = GETITEM(frame->f_code->co_consts, oparg);
Py_INCREF(value);
}

Expand Down Expand Up @@ -554,7 +552,7 @@ dummy_func(
}

inst(RETURN_CONST, (--)) {
PyObject *retval = GETITEM(consts, oparg);
PyObject *retval = GETITEM(frame->f_code->co_consts, oparg);
Py_INCREF(retval);
assert(EMPTY());
_PyFrame_SetStackPointer(frame, stack_pointer);
Expand Down Expand Up @@ -844,7 +842,7 @@ dummy_func(
}

inst(STORE_NAME, (v -- )) {
PyObject *name = GETITEM(names, oparg);
PyObject *name = GETITEM(frame->f_code->co_names, oparg);
PyObject *ns = LOCALS();
int err;
if (ns == NULL) {
Expand All @@ -862,7 +860,7 @@ dummy_func(
}

inst(DELETE_NAME, (--)) {
PyObject *name = GETITEM(names, oparg);
PyObject *name = GETITEM(frame->f_code->co_names, oparg);
PyObject *ns = LOCALS();
int err;
if (ns == NULL) {
Expand Down Expand Up @@ -956,7 +954,7 @@ dummy_func(
#if ENABLE_SPECIALIZATION
if (ADAPTIVE_COUNTER_IS_ZERO(counter)) {
assert(cframe.use_tracing == 0);
PyObject *name = GETITEM(names, oparg);
PyObject *name = GETITEM(frame->f_code->co_names, oparg);
next_instr--;
_Py_Specialize_StoreAttr(owner, next_instr, name);
DISPATCH_SAME_OPARG();
Expand All @@ -967,29 +965,29 @@ dummy_func(
#else
(void)counter; // Unused.
#endif /* ENABLE_SPECIALIZATION */
PyObject *name = GETITEM(names, oparg);
PyObject *name = GETITEM(frame->f_code->co_names, oparg);
int err = PyObject_SetAttr(owner, name, v);
Py_DECREF(v);
Py_DECREF(owner);
ERROR_IF(err, error);
}

inst(DELETE_ATTR, (owner --)) {
PyObject *name = GETITEM(names, oparg);
PyObject *name = GETITEM(frame->f_code->co_names, oparg);
int err = PyObject_SetAttr(owner, name, (PyObject *)NULL);
Py_DECREF(owner);
ERROR_IF(err, error);
}

inst(STORE_GLOBAL, (v --)) {
PyObject *name = GETITEM(names, oparg);
PyObject *name = GETITEM(frame->f_code->co_names, oparg);
int err = PyDict_SetItem(GLOBALS(), name, v);
Py_DECREF(v);
ERROR_IF(err, error);
}

inst(DELETE_GLOBAL, (--)) {
PyObject *name = GETITEM(names, oparg);
PyObject *name = GETITEM(frame->f_code->co_names, oparg);
int err;
err = PyDict_DelItem(GLOBALS(), name);
// Can't use ERROR_IF here.
Expand All @@ -1003,7 +1001,7 @@ dummy_func(
}

inst(LOAD_NAME, ( -- v)) {
PyObject *name = GETITEM(names, oparg);
PyObject *name = GETITEM(frame->f_code->co_names, oparg);
PyObject *locals = LOCALS();
if (locals == NULL) {
_PyErr_Format(tstate, PyExc_SystemError,
Expand Down Expand Up @@ -1074,15 +1072,15 @@ dummy_func(
_PyLoadGlobalCache *cache = (_PyLoadGlobalCache *)next_instr;
if (ADAPTIVE_COUNTER_IS_ZERO(cache->counter)) {
assert(cframe.use_tracing == 0);
PyObject *name = GETITEM(names, oparg>>1);
PyObject *name = GETITEM(frame->f_code->co_names, oparg>>1);
next_instr--;
_Py_Specialize_LoadGlobal(GLOBALS(), BUILTINS(), next_instr, name);
DISPATCH_SAME_OPARG();
}
STAT_INC(LOAD_GLOBAL, deferred);
DECREMENT_ADAPTIVE_COUNTER(cache->counter);
#endif /* ENABLE_SPECIALIZATION */
PyObject *name = GETITEM(names, oparg>>1);
PyObject *name = GETITEM(frame->f_code->co_names, oparg>>1);
if (PyDict_CheckExact(GLOBALS())
&& PyDict_CheckExact(BUILTINS()))
{
Expand Down Expand Up @@ -1436,15 +1434,15 @@ dummy_func(
_PyAttrCache *cache = (_PyAttrCache *)next_instr;
if (ADAPTIVE_COUNTER_IS_ZERO(cache->counter)) {
assert(cframe.use_tracing == 0);
PyObject *name = GETITEM(names, oparg>>1);
PyObject *name = GETITEM(frame->f_code->co_names, oparg>>1);
next_instr--;
_Py_Specialize_LoadAttr(owner, next_instr, name);
DISPATCH_SAME_OPARG();
}
STAT_INC(LOAD_ATTR, deferred);
DECREMENT_ADAPTIVE_COUNTER(cache->counter);
#endif /* ENABLE_SPECIALIZATION */
PyObject *name = GETITEM(names, oparg >> 1);
PyObject *name = GETITEM(frame->f_code->co_names, oparg >> 1);
if (oparg & 1) {
/* Designed to work in tandem with CALL, pushes two values. */
PyObject* meth = NULL;
Expand Down Expand Up @@ -1525,7 +1523,7 @@ dummy_func(
PyDictObject *dict = (PyDictObject *)_PyDictOrValues_GetDict(dorv);
DEOPT_IF(dict == NULL, LOAD_ATTR);
assert(PyDict_CheckExact((PyObject *)dict));
PyObject *name = GETITEM(names, oparg>>1);
PyObject *name = GETITEM(frame->f_code->co_names, oparg>>1);
uint16_t hint = index;
DEOPT_IF(hint >= (size_t)dict->ma_keys->dk_nentries, LOAD_ATTR);
if (DK_IS_UNICODE(dict->ma_keys)) {
Expand Down Expand Up @@ -1616,7 +1614,7 @@ dummy_func(
DEOPT_IF(!_PyThreadState_HasStackSpace(tstate, code->co_framesize), LOAD_ATTR);
STAT_INC(LOAD_ATTR, hit);

PyObject *name = GETITEM(names, oparg >> 1);
PyObject *name = GETITEM(frame->f_code->co_names, oparg >> 1);
Py_INCREF(f);
_PyInterpreterFrame *new_frame = _PyFrame_PushUnchecked(tstate, f, 2);
// Manipulate stack directly because we exit with DISPATCH_INLINED().
Expand Down Expand Up @@ -1661,7 +1659,7 @@ dummy_func(
PyDictObject *dict = (PyDictObject *)_PyDictOrValues_GetDict(dorv);
DEOPT_IF(dict == NULL, STORE_ATTR);
assert(PyDict_CheckExact((PyObject *)dict));
PyObject *name = GETITEM(names, oparg);
PyObject *name = GETITEM(frame->f_code->co_names, oparg);
DEOPT_IF(hint >= (size_t)dict->ma_keys->dk_nentries, STORE_ATTR);
PyObject *old_value;
uint64_t new_version;
Expand Down Expand Up @@ -1855,14 +1853,14 @@ dummy_func(
}

inst(IMPORT_NAME, (level, fromlist -- res)) {
PyObject *name = GETITEM(names, oparg);
PyObject *name = GETITEM(frame->f_code->co_names, oparg);
res = import_name(tstate, frame, name, fromlist, level);
DECREF_INPUTS();
ERROR_IF(res == NULL, error);
}

inst(IMPORT_FROM, (from -- from, res)) {
PyObject *name = GETITEM(names, oparg);
PyObject *name = GETITEM(frame->f_code->co_names, oparg);
res = import_from(tstate, from, name);
ERROR_IF(res == NULL, error);
}
Expand Down Expand Up @@ -2360,8 +2358,8 @@ dummy_func(

inst(KW_NAMES, (--)) {
assert(kwnames == NULL);
assert(oparg < PyTuple_GET_SIZE(consts));
kwnames = GETITEM(consts, oparg);
assert(oparg < PyTuple_GET_SIZE(frame->f_code->co_consts));
kwnames = GETITEM(frame->f_code->co_consts, oparg);
}

// Cache layout: counter/1, func_version/2, min_args/1
Expand Down
7 changes: 0 additions & 7 deletions Python/ceval.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -773,18 +773,11 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
/* Local "register" variables.
* These are cached values from the frame and code object. */

PyObject *names;
PyObject *consts;
_Py_CODEUNIT *next_instr;
PyObject **stack_pointer;

/* Sets the above local variables from the frame */
#define SET_LOCALS_FROM_FRAME() \
{ \
PyCodeObject *co = frame->f_code; \
names = co->co_names; \
consts = co->co_consts; \
} \
assert(_PyInterpreterFrame_LASTI(frame) >= -1); \
/* Jump back to the last instruction executed... */ \
next_instr = frame->prev_instr + 1; \
Expand Down
Loading

0 comments on commit 2d370da

Please sign in to comment.