bpo-45786: Allocate space for frame in frame object. (GH-29729)

markshannon · web-flow · commit 60929576e400 · 2021-11-29T12:34:59.000Z
diff --git a/Include/cpython/frameobject.h b/Include/cpython/frameobject.h
@@ -12,7 +12,9 @@ struct _frame {
     int f_lineno;               /* Current line number. Only valid if non-zero */
     char f_trace_lines;         /* Emit per-line trace events? */
     char f_trace_opcodes;       /* Emit per-opcode trace events? */
-    char f_own_locals_memory;   /* This frame owns the memory for the locals */
+    char f_owns_frame;          /* This frame owns the frame */
+    /* The frame data, if this frame object owns the frame */
+    PyObject *_f_frame_data[1];
 };
 
 /* Standard object interface */
@@ -26,7 +28,7 @@ PyAPI_FUNC(PyFrameObject *) PyFrame_New(PyThreadState *, PyCodeObject *,
 
 /* only internal use */
 PyFrameObject*
-_PyFrame_New_NoTrack(struct _interpreter_frame *, int);
+_PyFrame_New_NoTrack(PyCodeObject *code);
 
 
 /* The rest of the interface is specific for frame objects */
diff --git a/Include/internal/pycore_frame.h b/Include/internal/pycore_frame.h
@@ -69,8 +69,7 @@ static inline void _PyFrame_StackPush(InterpreterFrame *f, PyObject *value) {
 
 #define FRAME_SPECIALS_SIZE ((sizeof(InterpreterFrame)-1)/sizeof(PyObject *))
 
-InterpreterFrame *
-_PyInterpreterFrame_HeapAlloc(PyFunctionObject *func, PyObject *locals);
+InterpreterFrame *_PyFrame_Copy(InterpreterFrame *frame);
 
 static inline void
 _PyFrame_InitializeSpecials(
@@ -139,8 +138,8 @@ _PyFrame_GetFrameObject(InterpreterFrame *frame)
  * take should  be set to 1 for heap allocated
  * frames like the ones in generators and coroutines.
  */
-int
-_PyFrame_Clear(InterpreterFrame * frame, int take);
+void
+_PyFrame_Clear(InterpreterFrame * frame);
 
 int
 _PyFrame_Traverse(InterpreterFrame *frame, visitproc visit, void *arg);
diff --git a/Include/internal/pycore_gc.h b/Include/internal/pycore_gc.h
@@ -167,7 +167,6 @@ extern Py_ssize_t _PyGC_CollectNoFail(PyThreadState *tstate);
 
 
 // Functions to clear types free lists
-extern void _PyFrame_ClearFreeList(PyInterpreterState *interp);
 extern void _PyTuple_ClearFreeList(PyInterpreterState *interp);
 extern void _PyFloat_ClearFreeList(PyInterpreterState *interp);
 extern void _PyList_ClearFreeList(PyInterpreterState *interp);
diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h
@@ -93,7 +93,6 @@ struct _Py_unicode_state {
 #  define PyTuple_MAXFREELIST 1
 #  define PyList_MAXFREELIST 0
 #  define PyDict_MAXFREELIST 0
-#  define PyFrame_MAXFREELIST 0
 #  define _PyAsyncGen_MAXFREELIST 0
 #  define PyContext_MAXFREELIST 0
 #endif
@@ -158,18 +157,6 @@ struct _Py_dict_state {
 #endif
 };
 
-#ifndef PyFrame_MAXFREELIST
-#  define PyFrame_MAXFREELIST 200
-#endif
-
-struct _Py_frame_state {
-#if PyFrame_MAXFREELIST > 0
-    PyFrameObject *free_list;
-    /* number of frames currently in free_list */
-    int numfree;
-#endif
-};
-
 #ifndef _PyAsyncGen_MAXFREELIST
 #  define _PyAsyncGen_MAXFREELIST 80
 #endif
@@ -332,7 +319,6 @@ struct _is {
     struct _Py_tuple_state tuple;
     struct _Py_list_state list;
     struct _Py_dict_state dict_state;
-    struct _Py_frame_state frame;
     struct _Py_async_gen_state async_gen;
     struct _Py_context_state context;
     struct _Py_exc_state exc_state;
diff --git a/Lib/test/test_exceptions.py b/Lib/test/test_exceptions.py
@@ -209,7 +209,7 @@ def check(self, src, lineno, offset, encoding='utf-8'):
                     src = src.decode(encoding, 'replace')
                 line = src.split('\n')[lineno-1]
                 self.assertIn(line, cm.exception.text)
-    
+
     def test_error_offset_continuation_characters(self):
         check = self.check
         check('"\\\n"(1 for c in I,\\\n\\', 2, 2)
@@ -1342,9 +1342,7 @@ def recurse(cnt):
         """
         with SuppressCrashReport():
             rc, out, err = script_helper.assert_python_failure("-c", code)
-            self.assertIn(b'Fatal Python error: _PyErr_NormalizeException: '
-                          b'Cannot recover from MemoryErrors while '
-                          b'normalizing exceptions.', err)
+            self.assertIn(b'MemoryError', err)
 
     @cpython_only
     def test_MemoryError(self):
diff --git a/Lib/test/test_sys.py b/Lib/test/test_sys.py
@@ -1320,9 +1320,10 @@ class C(object): pass
         # sys.floatinfo
         check(sys.float_info, vsize('') + self.P * len(sys.float_info))
         # frame
-        import inspect
-        x = inspect.currentframe()
-        check(x, size('3Pi3c'))
+        def func():
+            return sys._getframe()
+        x = func()
+        check(x, size('3Pi3c8P2iciP'))
         # function
         def func(): pass
         check(func, size('14Pi'))
diff --git a/Misc/NEWS.d/next/Core and Builtins/2021-11-23-15-25-00.bpo-45786.UdEciD.rst b/Misc/NEWS.d/next/Core and Builtins/2021-11-23-15-25-00.bpo-45786.UdEciD.rst
@@ -0,0 +1,2 @@
+Allocate space for the interpreter frame in the frame object, to avoid an
+additional allocation when the frame object outlives the frame activation.
diff --git a/Modules/gcmodule.c b/Modules/gcmodule.c
@@ -1038,7 +1038,6 @@ delete_garbage(PyThreadState *tstate, GCState *gcstate,
 static void
 clear_freelists(PyInterpreterState *interp)
 {
-    _PyFrame_ClearFreeList(interp);
     _PyTuple_ClearFreeList(interp);
     _PyFloat_ClearFreeList(interp);
     _PyList_ClearFreeList(interp);
diff --git a/Objects/frameobject.c b/Objects/frameobject.c
@@ -610,7 +610,7 @@ static PyGetSetDef frame_getsetlist[] = {
     f_back              next item on free list, or NULL
 */
 
-static void _Py_HOT_FUNCTION
+static void
 frame_dealloc(PyFrameObject *f)
 {
     if (_PyObject_GC_IS_TRACKED(f)) {
@@ -621,9 +621,10 @@ frame_dealloc(PyFrameObject *f)
     PyCodeObject *co = NULL;
 
     /* Kill all local variables including specials, if we own them */
-    if (f->f_own_locals_memory) {
-        f->f_own_locals_memory = 0;
-        InterpreterFrame *frame = f->f_frame;
+    if (f->f_owns_frame) {
+        f->f_owns_frame = 0;
+        assert(f->f_frame == (InterpreterFrame *)f->_f_frame_data);
+        InterpreterFrame *frame = (InterpreterFrame *)f->_f_frame_data;
         /* Don't clear code object until the end */
         co = frame->f_code;
         frame->f_code = NULL;
@@ -633,27 +634,10 @@ frame_dealloc(PyFrameObject *f)
         for (int i = 0; i < frame->stacktop; i++) {
             Py_CLEAR(locals[i]);
         }
-        PyMem_Free(frame);
     }
     Py_CLEAR(f->f_back);
     Py_CLEAR(f->f_trace);
-#if PyFrame_MAXFREELIST > 0
-    struct _Py_frame_state *state = get_frame_state();
-#ifdef Py_DEBUG
-    // frame_dealloc() must not be called after _PyFrame_Fini()
-    assert(state->numfree != -1);
-#endif
-    if (state->numfree < PyFrame_MAXFREELIST) {
-        ++state->numfree;
-        f->f_back = state->free_list;
-        state->free_list = f;
-    }
-    else
-#endif
-    {
-        PyObject_GC_Del(f);
-    }
-
+    PyObject_GC_Del(f);
     Py_XDECREF(co);
     Py_TRASHCAN_END;
 }
@@ -663,7 +647,7 @@ frame_traverse(PyFrameObject *f, visitproc visit, void *arg)
 {
     Py_VISIT(f->f_back);
     Py_VISIT(f->f_trace);
-    if (f->f_own_locals_memory == 0) {
+    if (f->f_owns_frame == 0) {
         return 0;
     }
     assert(f->f_frame->frame_obj == NULL);
@@ -715,11 +699,9 @@ static PyObject *
 frame_sizeof(PyFrameObject *f, PyObject *Py_UNUSED(ignored))
 {
     Py_ssize_t res;
-    res = sizeof(PyFrameObject);
-    if (f->f_own_locals_memory) {
-        PyCodeObject *code = f->f_frame->f_code;
-        res += (code->co_nlocalsplus+code->co_stacksize) * sizeof(PyObject *);
-    }
+    res = offsetof(PyFrameObject, _f_frame_data) + offsetof(InterpreterFrame, localsplus);
+    PyCodeObject *code = f->f_frame->f_code;
+    res += (code->co_nlocalsplus+code->co_stacksize) * sizeof(PyObject *);
     return PyLong_FromSsize_t(res);
 }
 
@@ -747,7 +729,8 @@ static PyMethodDef frame_methods[] = {
 PyTypeObject PyFrame_Type = {
     PyVarObject_HEAD_INIT(&PyType_Type, 0)
     "frame",
-    sizeof(PyFrameObject),
+    offsetof(PyFrameObject, _f_frame_data) +
+    offsetof(InterpreterFrame, localsplus),
     sizeof(PyObject *),
     (destructor)frame_dealloc,                  /* tp_dealloc */
     0,                                          /* tp_vectorcall_offset */
@@ -781,67 +764,21 @@ PyTypeObject PyFrame_Type = {
 
 _Py_IDENTIFIER(__builtins__);
 
-static InterpreterFrame *
-allocate_heap_frame(PyFunctionObject *func, PyObject *locals)
+static void
+init_frame(InterpreterFrame *frame, PyFunctionObject *func, PyObject *locals)
 {
     PyCodeObject *code = (PyCodeObject *)func->func_code;
-    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;
-    }
     _PyFrame_InitializeSpecials(frame, func, locals, code->co_nlocalsplus);
     for (Py_ssize_t i = 0; i < code->co_nlocalsplus; i++) {
         frame->localsplus[i] = NULL;
     }
-    return frame;
 }
 
-static inline PyFrameObject*
-frame_alloc(InterpreterFrame *frame, int owns)
-{
-    PyFrameObject *f;
-#if PyFrame_MAXFREELIST > 0
-    struct _Py_frame_state *state = get_frame_state();
-    if (state->free_list == NULL)
-#endif
-    {
-        f = PyObject_GC_New(PyFrameObject, &PyFrame_Type);
-        if (f == NULL) {
-            if (owns) {
-                Py_XDECREF(frame->f_code);
-                Py_XDECREF(frame->f_builtins);
-                Py_XDECREF(frame->f_globals);
-                Py_XDECREF(frame->f_locals);
-                PyMem_Free(frame);
-            }
-            return NULL;
-        }
-    }
-#if PyFrame_MAXFREELIST > 0
-    else
-    {
-#ifdef Py_DEBUG
-        // frame_alloc() must not be called after _PyFrame_Fini()
-        assert(state->numfree != -1);
-#endif
-        assert(state->numfree > 0);
-        --state->numfree;
-        f = state->free_list;
-        state->free_list = state->free_list->f_back;
-        _Py_NewReference((PyObject *)f);
-    }
-#endif
-    f->f_frame = frame;
-    f->f_own_locals_memory = owns;
-    return f;
-}
-
-PyFrameObject* _Py_HOT_FUNCTION
-_PyFrame_New_NoTrack(InterpreterFrame *frame, int owns)
+PyFrameObject*
+_PyFrame_New_NoTrack(PyCodeObject *code)
 {
-    PyFrameObject *f = frame_alloc(frame, owns);
+    int slots = code->co_nlocalsplus + code->co_stacksize;
+    PyFrameObject *f = PyObject_GC_NewVar(PyFrameObject, &PyFrame_Type, slots);
     if (f == NULL) {
         return NULL;
     }
@@ -876,15 +813,16 @@ PyFrame_New(PyThreadState *tstate, PyCodeObject *code,
     if (func == NULL) {
         return NULL;
     }
-    InterpreterFrame *frame = allocate_heap_frame(func, locals);
-    Py_DECREF(func);
-    if (frame == NULL) {
+    PyFrameObject *f = _PyFrame_New_NoTrack(code);
+    if (f == NULL) {
+        Py_DECREF(func);
         return NULL;
     }
-    PyFrameObject *f = _PyFrame_New_NoTrack(frame, 1);
-    if (f) {
-        _PyObject_GC_TRACK(f);
-    }
+    init_frame((InterpreterFrame *)f->_f_frame_data, func, locals);
+    f->f_frame = (InterpreterFrame *)f->_f_frame_data;
+    f->f_owns_frame = 1;
+    Py_DECREF(func);
+    _PyObject_GC_TRACK(f);
     return f;
 }
 
@@ -1087,42 +1025,15 @@ PyFrame_LocalsToFast(PyFrameObject *f, int clear)
     _PyFrame_LocalsToFast(f->f_frame, clear);
 }
 
-/* Clear out the free list */
-void
-_PyFrame_ClearFreeList(PyInterpreterState *interp)
-{
-#if PyFrame_MAXFREELIST > 0
-    struct _Py_frame_state *state = &interp->frame;
-    while (state->free_list != NULL) {
-        PyFrameObject *f = state->free_list;
-        state->free_list = state->free_list->f_back;
-        PyObject_GC_Del(f);
-        --state->numfree;
-    }
-    assert(state->numfree == 0);
-#endif
-}
-
 void
 _PyFrame_Fini(PyInterpreterState *interp)
 {
-    _PyFrame_ClearFreeList(interp);
-#if defined(Py_DEBUG) && PyFrame_MAXFREELIST > 0
-    struct _Py_frame_state *state = &interp->frame;
-    state->numfree = -1;
-#endif
 }
 
 /* Print summary info about the state of the optimized allocator */
 void
 _PyFrame_DebugMallocStats(FILE *out)
 {
-#if PyFrame_MAXFREELIST > 0
-    struct _Py_frame_state *state = get_frame_state();
-    _PyDebugAllocatorStats(out,
-                           "free PyFrameObject",
-                           state->numfree, sizeof(PyFrameObject));
-#endif
 }
 
 
diff --git a/Objects/genobject.c b/Objects/genobject.c
diff --git a/Python/ceval.c b/Python/ceval.c
diff --git a/Python/frame.c b/Python/frame.c

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+Allocate space for the interpreter frame in the frame object, to avoid an`
	`2`	`+additional allocation when the frame object outlives the frame activation.`
Original file line number	Diff line number	Diff line change
`@@ -1038,7 +1038,6 @@ delete_garbage(PyThreadState tstate, GCState gcstate,`
`1038`	`1038`	`static void`
`1039`	`1039`	`clear_freelists(PyInterpreterState *interp)`
`1040`	`1040`	`{`
`1041`		`- _PyFrame_ClearFreeList(interp);`
`1042`	`1041`	`_PyTuple_ClearFreeList(interp);`
`1043`	`1042`	`_PyFloat_ClearFreeList(interp);`
`1044`	`1043`	`_PyList_ClearFreeList(interp);`