Avoid extra copies in take_ownership

Author: mpage

Include/internal/pycore_frame.h

@@ -153,20 +153,27 @@ _PyFrame_NumSlotsForCodeObject(PyCodeObject *code)
 }
 
 static inline void
-_PyFrame_Copy(_PyInterpreterFrame *src, _PyInterpreterFrame *dest)
+_PyFrame_CopyToHeap(_PyInterpreterFrame *src, _PyInterpreterFrame *dest)
 {
     *dest = *src;
     assert(src->stackpointer != NULL);
     int stacktop = (int)(src->stackpointer - src->localsplus);
     assert(stacktop >= _PyFrame_GetCode(src)->co_nlocalsplus);
     dest->stackpointer = dest->localsplus + stacktop;
-    // The generator may outlive any references that were providing "support"
-    // for borrowed references in the frame. Convert them to strong references.
+    // The destination frame may outlive any references that were providing
+    // "support" for borrowed references in the source frame. Convert any
+    // borrowed references that were copied into dest into strong references.
     for (int i = 0; i < stacktop; i++) {
         dest->localsplus[i] = _PyStackRef_NewIfBorrowedOrSteal(src->localsplus[i]);
     }
     dest->f_executable = _PyStackRef_NewIfBorrowedOrSteal(dest->f_executable);
     dest->f_funcobj = _PyStackRef_NewIfBorrowedOrSteal(dest->f_funcobj);
+}
+
+static inline void
+_PyFrame_CopyToNewGen(_PyInterpreterFrame *src, _PyInterpreterFrame *dest)
+{
+    _PyFrame_CopyToHeap(src, dest);
     // Don't leave a dangling pointer to the old frame when creating generators
     // and coroutines:
     dest->previous = NULL;

Objects/genobject.c

@@ -989,7 +989,7 @@ gen_new_with_qualname(PyTypeObject *type, PyFrameObject *f,
     assert(f->f_frame->frame_obj == NULL);
     assert(f->f_frame->owner == FRAME_OWNED_BY_FRAME_OBJECT);
     _PyInterpreterFrame *frame = &gen->gi_iframe;
-    _PyFrame_Copy((_PyInterpreterFrame *)f->_f_frame_data, frame);
+    _PyFrame_CopyToNewGen((_PyInterpreterFrame *)f->_f_frame_data, frame);
     gen->gi_frame_state = FRAME_CREATED;
     assert(frame->frame_obj == f);
     f->f_frame = frame;

Python/bytecodes.c

@@ -4694,7 +4694,7 @@ dummy_func(
             SAVE_STACK();
             _PyInterpreterFrame *gen_frame = &gen->gi_iframe;
             frame->instr_ptr++;
-            _PyFrame_Copy(frame, gen_frame);
+            _PyFrame_CopyToNewGen(frame, gen_frame);
             assert(frame->frame_obj == NULL);
             gen->gi_frame_state = FRAME_CREATED;
             gen_frame->owner = FRAME_OWNED_BY_GENERATOR;

Python/executor_cases.c.h

@@ -50,20 +50,19 @@ take_ownership(PyFrameObject *f, _PyInterpreterFrame *frame)
 {
     assert(frame->owner < FRAME_OWNED_BY_INTERPRETER);
     assert(frame->owner != FRAME_OWNED_BY_FRAME_OBJECT);
-    Py_ssize_t size = ((char*)frame->stackpointer) - (char *)frame;
-    memcpy((_PyInterpreterFrame *)f->_f_frame_data, frame, size);
-    frame = (_PyInterpreterFrame *)f->_f_frame_data;
-    frame->stackpointer = (_PyStackRef *)(((char *)frame) + size);
+    // While ownership of f_executable is transferred to the frame object, the
+    // reference in the frame will also be closed when the frame is popped from
+    // the stack. Create another reference to ensure that works correctly.
+    //
+    // This must happen in the source frame (not in the frame
+    // object). _PyFrame_CopyToHeap converts borrowed references in the source
+    // frame into strong references in the destination frame. duping the
+    // reference in the dest frame would result in a leak if the source was
+    // borrowed because the close on the source would not destroy the newly
+    // created reference.
     frame->f_executable = PyStackRef_DUP(frame->f_executable);
-    int stacktop = (int)(frame->stackpointer - frame->localsplus);
-    assert(stacktop >= _PyFrame_GetCode(frame)->co_nlocalsplus);
-    // The frame object may outlive any references that were providing "support"
-    // for borrowed references in the frame. Convert them to strong references.
-    frame->f_executable = _PyStackRef_NewIfBorrowedOrSteal(frame->f_executable);
-    frame->f_funcobj = _PyStackRef_NewIfBorrowedOrSteal(frame->f_funcobj);
-    for (int i = 0; i < stacktop; i++) {
-        frame->localsplus[i] = _PyStackRef_NewIfBorrowedOrSteal(frame->localsplus[i]);
-    }
+    _PyFrame_CopyToHeap(frame, (_PyInterpreterFrame *) f->_f_frame_data);
+    frame = (_PyInterpreterFrame *)f->_f_frame_data;
     f->f_frame = frame;
     frame->owner = FRAME_OWNED_BY_FRAME_OBJECT;
     if (_PyFrame_IsIncomplete(frame)) {