bpo-44990: Change layout of evaluation frames. "Layout B" (GH-27933)

Places the locals between the specials and stack. This is the more "natural" layout for a C struct, makes the code simpler and gives a slight speedup (~1%)

3 files changed, 62 insertions, 66 deletions

diff --git a/Python/ceval.c b/Python/ceval.c
index 333c54f..5fec90b 100644
--- a/Python/ceval.c
+++ b/Python/ceval.c
@@ -1367,7 +1367,7 @@ eval_frame_handle_pending(PyThreadState *tstate)
 
 /* The stack can grow at most MAXINT deep, as co_nlocals and
    co_stacksize are ints. */
-#define STACK_LEVEL()     ((int)(stack_pointer - frame->stack))
+#define STACK_LEVEL()     ((int)(stack_pointer - _PyFrame_Stackbase(frame)))
 #define EMPTY()           (STACK_LEVEL() == 0)
 #define TOP()             (stack_pointer[-1])
 #define SECOND()          (stack_pointer[-2])
@@ -1413,7 +1413,7 @@ eval_frame_handle_pending(PyThreadState *tstate)
 
 /* Local variable macros */
 
-#define GETLOCAL(i)     (localsplus[i])
+#define GETLOCAL(i)     (frame->localsplus[i])
 
 /* The SETLOCAL() macro must not DECREF the local variable in-place and
    then store the new value; it must copy the old value to a temporary
@@ -1559,7 +1559,6 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr
 
     PyObject *names = co->co_names;
     PyObject *consts = co->co_consts;
-    PyObject **localsplus = _PyFrame_GetLocalsArray(frame);
     _Py_CODEUNIT *first_instr = co->co_firstinstr;
     /*
        frame->f_lasti refers to the index of the last instruction,
@@ -1578,14 +1577,14 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr
     */
     assert(frame->f_lasti >= -1);
     _Py_CODEUNIT *next_instr = first_instr + frame->f_lasti + 1;
-    PyObject **stack_pointer = frame->stack + frame->stackdepth;
+    PyObject **stack_pointer = _PyFrame_GetStackPointer(frame);
     /* Set stackdepth to -1.
      * Update when returning or calling trace function.
-       Having f_stackdepth <= 0 ensures that invalid
+       Having stackdepth <= 0 ensures that invalid
        values are not visible to the cycle GC.
        We choose -1 rather than 0 to assist debugging.
      */
-    frame->stackdepth = -1;
+    frame->stacktop = -1;
     frame->f_state = FRAME_EXECUTING;
 
 #ifdef LLTRACE
@@ -1668,7 +1667,7 @@ check_eval_breaker:
             int err;
             /* see maybe_call_line_trace()
                for expository comments */
-            frame->stackdepth = (int)(stack_pointer - frame->stack);
+            _PyFrame_SetStackPointer(frame, stack_pointer);
 
             err = maybe_call_line_trace(tstate->c_tracefunc,
                                         tstate->c_traceobj,
@@ -1679,8 +1678,9 @@ check_eval_breaker:
             }
             /* Reload possibly changed frame fields */
             JUMPTO(frame->f_lasti);
-            stack_pointer = frame->stack+frame->stackdepth;
-            frame->stackdepth = -1;
+
+            stack_pointer = _PyFrame_GetStackPointer(frame);
+            frame->stacktop = -1;
             TRACING_NEXTOPARG();
         }
         PRE_DISPATCH_GOTO();
@@ -2439,7 +2439,7 @@ check_eval_breaker:
             retval = POP();
             assert(EMPTY());
             frame->f_state = FRAME_RETURNED;
-            frame->stackdepth = 0;
+            _PyFrame_SetStackPointer(frame, stack_pointer);
             goto exiting;
         }
 
@@ -2627,7 +2627,7 @@ check_eval_breaker:
             assert(frame->f_lasti > 0);
             frame->f_lasti -= 1;
             frame->f_state = FRAME_SUSPENDED;
-            frame->stackdepth = (int)(stack_pointer - frame->stack);
+            _PyFrame_SetStackPointer(frame, stack_pointer);
             goto exiting;
         }
 
@@ -2644,7 +2644,7 @@ check_eval_breaker:
                 retval = w;
             }
             frame->f_state = FRAME_SUSPENDED;
-            frame->stackdepth = (int)(stack_pointer - frame->stack);
+            _PyFrame_SetStackPointer(frame, stack_pointer);
             goto exiting;
         }
 
@@ -4346,7 +4346,7 @@ check_eval_breaker:
                 oparg = cache->adaptive.original_oparg;
                 STAT_DEC(LOAD_METHOD, unquickened);
                 JUMP_TO_INSTRUCTION(LOAD_METHOD);
-            }            
+            }
         }
 
         TARGET(LOAD_METHOD_CACHED): {
@@ -4364,7 +4364,7 @@ check_eval_breaker:
             assert(cache1->tp_version != 0);
             assert(self_cls->tp_dictoffset >= 0);
             assert(Py_TYPE(self_cls)->tp_dictoffset > 0);
-            
+
             // inline version of _PyObject_GetDictPtr for offset >= 0
             PyObject *dict = self_cls->tp_dictoffset != 0 ?
                 *(PyObject **) ((char *)self + self_cls->tp_dictoffset) : NULL;
@@ -4817,17 +4817,20 @@ exception_unwind:
             assert(_PyErr_Occurred(tstate));
 
             /* Pop remaining stack entries. */
-            while (!EMPTY()) {
+            PyObject **stackbase = _PyFrame_Stackbase(frame);
+            while (stack_pointer > stackbase) {
                 PyObject *o = POP();
                 Py_XDECREF(o);
             }
-            frame->stackdepth = 0;
+            assert(STACK_LEVEL() == 0);
+            _PyFrame_SetStackPointer(frame, stack_pointer);
             frame->f_state = FRAME_RAISED;
             goto exiting;
         }
 
         assert(STACK_LEVEL() >= level);
-        while (STACK_LEVEL() > level) {
+        PyObject **new_top = _PyFrame_Stackbase(frame) + level;
+        while (stack_pointer > new_top) {
             PyObject *v = POP();
             Py_XDECREF(v);
         }
@@ -4981,7 +4984,7 @@ missing_arguments(PyThreadState *tstate, PyCodeObject *co,
         end = start + co->co_kwonlyargcount;
     }
     for (i = start; i < end; i++) {
-        if (GETLOCAL(i) == NULL) {
+        if (localsplus[i] == NULL) {
             PyObject *raw = PyTuple_GET_ITEM(co->co_localsplusnames, i);
             PyObject *name = PyObject_Repr(raw);
             if (name == NULL) {
@@ -5010,7 +5013,7 @@ too_many_positional(PyThreadState *tstate, PyCodeObject *co,
     assert((co->co_flags & CO_VARARGS) == 0);
     /* Count missing keyword-only args. */
     for (i = co_argcount; i < co_argcount + co->co_kwonlyargcount; i++) {
-        if (GETLOCAL(i) != NULL) {
+        if (localsplus[i] != NULL) {
             kwonly_given++;
         }
     }
@@ -5217,7 +5220,8 @@ initialize_locals(PyThreadState *tstate, PyFrameConstructor *con,
         if (co->co_flags & CO_VARARGS) {
             i++;
         }
-        SETLOCAL(i, kwdict);
+        assert(localsplus[i] == NULL);
+        localsplus[i] = kwdict;
     }
     else {
         kwdict = NULL;
@@ -5234,7 +5238,8 @@ initialize_locals(PyThreadState *tstate, PyFrameConstructor *con,
     for (j = 0; j < n; j++) {
         PyObject *x = args[j];
         Py_INCREF(x);
-        SETLOCAL(j, x);
+        assert(localsplus[j] == NULL);
+        localsplus[j] = x;
     }
 
     /* Pack other positional arguments into the *args argument */
@@ -5243,7 +5248,8 @@ initialize_locals(PyThreadState *tstate, PyFrameConstructor *con,
         if (u == NULL) {
             goto fail;
         }
-        SETLOCAL(total_args, u);
+        assert(localsplus[total_args] == NULL);
+        localsplus[total_args] = u;
     }
 
     /* Handle keyword arguments */
@@ -5307,14 +5313,14 @@ initialize_locals(PyThreadState *tstate, PyFrameConstructor *con,
             continue;
 
         kw_found:
-            if (GETLOCAL(j) != NULL) {
+            if (localsplus[j] != NULL) {
                 _PyErr_Format(tstate, PyExc_TypeError,
                             "%U() got multiple values for argument '%S'",
                           con->fc_qualname, keyword);
                 goto fail;
             }
             Py_INCREF(value);
-            SETLOCAL(j, value);
+            localsplus[j] = value;
         }
     }
 
@@ -5331,7 +5337,7 @@ initialize_locals(PyThreadState *tstate, PyFrameConstructor *con,
         Py_ssize_t m = co->co_argcount - defcount;
         Py_ssize_t missing = 0;
         for (i = argcount; i < m; i++) {
-            if (GETLOCAL(i) == NULL) {
+            if (localsplus[i] == NULL) {
                 missing++;
             }
         }
@@ -5347,10 +5353,10 @@ initialize_locals(PyThreadState *tstate, PyFrameConstructor *con,
         if (defcount) {
             PyObject **defs = &PyTuple_GET_ITEM(con->fc_defaults, 0);
             for (; i < defcount; i++) {
-                if (GETLOCAL(m+i) == NULL) {
+                if (localsplus[m+i] == NULL) {
                     PyObject *def = defs[i];
                     Py_INCREF(def);
-                    SETLOCAL(m+i, def);
+                    localsplus[m+i] = def;
                 }
             }
         }
@@ -5360,14 +5366,14 @@ initialize_locals(PyThreadState *tstate, PyFrameConstructor *con,
     if (co->co_kwonlyargcount > 0) {
         Py_ssize_t missing = 0;
         for (i = co->co_argcount; i < total_args; i++) {
-            if (GETLOCAL(i) != NULL)
+            if (localsplus[i] != NULL)
                 continue;
             PyObject *varname = PyTuple_GET_ITEM(co->co_localsplusnames, i);
             if (con->fc_kwdefaults != NULL) {
                 PyObject *def = PyDict_GetItemWithError(con->fc_kwdefaults, varname);
                 if (def) {
                     Py_INCREF(def);
-                    SETLOCAL(i, def);
+                    localsplus[i] = def;
                     continue;
                 }
                 else if (_PyErr_Occurred(tstate)) {
@@ -5407,18 +5413,17 @@ make_coro_frame(PyThreadState *tstate,
     assert(con->fc_defaults == NULL || PyTuple_CheckExact(con->fc_defaults));
     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;
+        frame->localsplus[i] = NULL;
     }
-    InterpreterFrame *frame = (InterpreterFrame *)(localsarray + code->co_nlocalsplus);
     _PyFrame_InitializeSpecials(frame, con, locals, code->co_nlocalsplus);
     assert(frame->frame_obj == NULL);
-    if (initialize_locals(tstate, con, localsarray, args, argcount, kwnames)) {
+    if (initialize_locals(tstate, con, frame->localsplus, args, argcount, kwnames)) {
         _PyFrame_Clear(frame, 1);
         return NULL;
     }
@@ -5497,7 +5502,7 @@ _PyEval_Vector(PyThreadState *tstate, PyFrameConstructor *con,
     }
     assert (tstate->interp->eval_frame != NULL);
     PyObject *retval = _PyEval_EvalFrame(tstate, frame, 0);
-    assert(frame->stackdepth == 0);
+    assert(_PyFrame_GetStackPointer(frame) == _PyFrame_Stackbase(frame));
     if (_PyEvalFrameClearAndPop(tstate, frame)) {
         retval = NULL;
     }
@@ -6764,7 +6769,6 @@ unicode_concatenate(PyThreadState *tstate, PyObject *v, PyObject *w,
         switch (opcode) {
         case STORE_FAST:
         {
-            PyObject **localsplus = _PyFrame_GetLocalsArray(frame);
             if (GETLOCAL(oparg) == v)
                 SETLOCAL(oparg, NULL);
             break;
diff --git a/Python/frame.c b/Python/frame.c
index ae42843..3d2415f 100644
--- a/Python/frame.c
+++ b/Python/frame.c
@@ -14,13 +14,11 @@ _PyFrame_Traverse(InterpreterFrame *frame, visitproc visit, void *arg)
     Py_VISIT(frame->f_code);
    /* locals */
     PyObject **locals = _PyFrame_GetLocalsArray(frame);
-    for (int i = 0; i < frame->nlocalsplus; i++) {
+    int i = 0;
+    /* locals and stack */
+    for (; i <frame->stacktop; i++) {
         Py_VISIT(locals[i]);
     }
-    /* stack */
-    for (int i = 0; i <frame->stackdepth; i++) {
-        Py_VISIT(frame->stack[i]);
-    }
     return 0;
 }
 
@@ -47,17 +45,15 @@ _PyFrame_MakeAndSetFrameObject(InterpreterFrame *frame)
 static InterpreterFrame *
 copy_frame_to_heap(InterpreterFrame *frame)
 {
-
-    Py_ssize_t size = ((char*)&frame->stack[frame->stackdepth]) - (char *)_PyFrame_GetLocalsArray(frame);
-    PyObject **copy = PyMem_Malloc(size);
+    assert(frame->stacktop >= frame->f_code->co_nlocalsplus);
+    Py_ssize_t size = ((char*)&frame->localsplus[frame->stacktop]) - (char *)frame;
+    InterpreterFrame *copy = PyMem_Malloc(size);
     if (copy == NULL) {
         PyErr_NoMemory();
         return NULL;
     }
-    PyObject **locals = _PyFrame_GetLocalsArray(frame);
-    memcpy(copy, locals, size);
-    InterpreterFrame *res = (InterpreterFrame *)(copy + frame->nlocalsplus);
-    return res;
+    memcpy(copy, frame, size);
+    return copy;
 }
 
 static inline void
@@ -103,7 +99,6 @@ take_ownership(PyFrameObject *f, InterpreterFrame *frame)
 int
 _PyFrame_Clear(InterpreterFrame * frame, int take)
 {
-    PyObject **localsarray = ((PyObject **)frame)-frame->nlocalsplus;
     if (frame->frame_obj) {
         PyFrameObject *f = frame->frame_obj;
         frame->frame_obj = NULL;
@@ -120,16 +115,13 @@ _PyFrame_Clear(InterpreterFrame * frame, int take)
         }
         Py_DECREF(f);
     }
-    for (int i = 0; i < frame->nlocalsplus; i++) {
-        Py_XDECREF(localsarray[i]);
-    }
-    assert(frame->stackdepth >= 0);
-    for (int i = 0; i < frame->stackdepth; i++) {
-        Py_DECREF(frame->stack[i]);
+    assert(_PyFrame_GetStackPointer(frame) >= _PyFrame_Stackbase(frame));
+    for (int i = 0; i < frame->stacktop; i++) {
+        Py_XDECREF(frame->localsplus[i]);
     }
     clear_specials(frame);
     if (take) {
-        PyMem_Free(_PyFrame_GetLocalsArray(frame));
+        PyMem_Free(frame);
     }
     return 0;
 }
diff --git a/Python/pystate.c b/Python/pystate.c
index 6a15b54..4527214 100644
--- a/Python/pystate.c
+++ b/Python/pystate.c
@@ -2045,21 +2045,21 @@ _PyThreadState_PushFrame(PyThreadState *tstate, PyFrameConstructor *con, PyObjec
     size_t size = nlocalsplus + code->co_stacksize +
         FRAME_SPECIALS_SIZE;
     assert(size < INT_MAX/sizeof(PyObject *));
-    PyObject **localsarray = tstate->datastack_top;
-    PyObject **top = localsarray + size;
+    PyObject **base = tstate->datastack_top;
+    PyObject **top = base + size;
     if (top >= tstate->datastack_limit) {
-        localsarray = push_chunk(tstate, (int)size);
-        if (localsarray == NULL) {
+        base = push_chunk(tstate, (int)size);
+        if (base == NULL) {
             return NULL;
         }
     }
     else {
         tstate->datastack_top = top;
     }
-    InterpreterFrame * frame = (InterpreterFrame *)(localsarray + nlocalsplus);
+    InterpreterFrame *frame  = (InterpreterFrame *)base;
     _PyFrame_InitializeSpecials(frame, con, locals, nlocalsplus);
     for (int i=0; i < nlocalsplus; i++) {
-        localsarray[i] = NULL;
+        frame->localsplus[i] = NULL;
     }
     return frame;
 }
@@ -2067,8 +2067,8 @@ _PyThreadState_PushFrame(PyThreadState *tstate, PyFrameConstructor *con, PyObjec
 void
 _PyThreadState_PopFrame(PyThreadState *tstate, InterpreterFrame * frame)
 {
-    PyObject **locals = _PyFrame_GetLocalsArray(frame);
-    if (locals == &tstate->datastack_chunk->data[0]) {
+    PyObject **base = (PyObject **)frame;
+    if (base == &tstate->datastack_chunk->data[0]) {
         _PyStackChunk *chunk = tstate->datastack_chunk;
         _PyStackChunk *previous = chunk->previous;
         tstate->datastack_top = &previous->data[previous->top];
@@ -2077,8 +2077,8 @@ _PyThreadState_PopFrame(PyThreadState *tstate, InterpreterFrame * frame)
         tstate->datastack_limit = (PyObject **)(((char *)previous) + previous->size);
     }
     else {
-        assert(tstate->datastack_top >= locals);
-        tstate->datastack_top = locals;
+        assert(tstate->datastack_top >= base);
+        tstate->datastack_top = base;
     }
 }