Py3.12: Always expose greenlet frames on a switch.

Add benchmarks for testing this.

Author: jamadden

benchmarks/chain.py

@@ -5,10 +5,24 @@
 along.
 """
 
+import os
 import pyperf
 import greenlet
 
+# This is obsolete now, we always expose frames for Python 3.12.
+# See https://github.com/python-greenlet/greenlet/pull/393/
+# for a complete discussion of performance.
+EXPOSE_FRAMES = 'EXPOSE_FRAMES' in os.environ
 
+# Exposing
+# 100 frames Mean +- std dev: 5.62 us +- 0.10 us
+# 200 frames Mean +- std dev: 14.0 us +- 0.6 us
+# 300 frames Mean +- std dev: 22.7 us +- 0.4 us
+#
+# Non-exposing
+# 100 frames Mean +- std dev: 3.64 us +- 0.06 us -> 1.54/1.98us
+# 200 frames Mean +- std dev: 9.49 us +- 0.13 us -> 1.47/4.51us
+# 300 frames Mean +- std dev: 15.7 us +- 0.3 us  -> 1.45/7us
 
 def link(next_greenlet):
     value = greenlet.getcurrent().parent.switch()
@@ -23,6 +37,7 @@ def bm_chain(loops):
         start_node = greenlet.getcurrent()
         for _ in range(CHAIN_GREENLET_COUNT):
             g = greenlet.greenlet(link)
+            g.gr_frames_always_exposed = EXPOSE_FRAMES
             g.switch(start_node)
             start_node = g
         x = start_node.switch(0)
@@ -51,7 +66,8 @@ def bm_getcurrent(loops):
     return end - begin
 
 SWITCH_INNER_LOOPS = 10000
-def bm_switch(loops):
+def bm_switch_shallow(loops):
+    # pylint:disable=attribute-defined-outside-init
     class G(greenlet.greenlet):
         other = None
         def run(self):
@@ -60,15 +76,63 @@ def run(self):
                 o.switch()
 
     begin = pyperf.perf_counter()
+
+    for _ in range(loops):
+        gl1 = G()
+        gl2 = G()
+        gl1.gr_frames_always_exposed = EXPOSE_FRAMES
+        gl2.gr_frames_always_exposed = EXPOSE_FRAMES
+        gl1.other = gl2
+        gl2.other = gl1
+        gl1.switch()
+
+        gl1.switch()
+        gl2.switch()
+        gl1.other = gl2.other = None
+        assert gl1.dead
+        assert gl2.dead
+
+    end = pyperf.perf_counter()
+    return end - begin
+
+def bm_switch_deep(loops, _MAX_DEPTH=200):
+    # pylint:disable=attribute-defined-outside-init
+    class G(greenlet.greenlet):
+        other = None
+        def run(self):
+            for _ in range(SWITCH_INNER_LOOPS):
+                self.recur_then_switch()
+
+        def recur_then_switch(self, depth=_MAX_DEPTH):
+            if not depth:
+                self.other.switch()
+            else:
+                self.recur_then_switch(depth - 1)
+
+    begin = pyperf.perf_counter()
+
     for _ in range(loops):
         gl1 = G()
         gl2 = G()
+        gl1.gr_frames_always_exposed = EXPOSE_FRAMES
+        gl2.gr_frames_always_exposed = EXPOSE_FRAMES
         gl1.other = gl2
         gl2.other = gl1
         gl1.switch()
+
+        gl1.switch()
+        gl2.switch()
+        gl1.other = gl2.other = None
+        assert gl1.dead
+        assert gl2.dead
+
     end = pyperf.perf_counter()
     return end - begin
 
+def bm_switch_deeper(loops):
+    return bm_switch_deep(loops, 400)
+
+
 CREATE_INNER_LOOPS = 10
 def bm_create(loops):
     gl = greenlet.greenlet
@@ -87,20 +151,81 @@ def bm_create(loops):
     end = pyperf.perf_counter()
     return end - begin
 
+
+
+
+def _bm_recur_frame(loops, RECUR_DEPTH):
+
+    def recur(depth):
+        if not depth:
+            return greenlet.getcurrent().parent.switch(greenlet.getcurrent())
+        return recur(depth - 1)
+
+
+    begin = pyperf.perf_counter()
+    for _ in range(loops):
+
+        for _ in range(CHAIN_GREENLET_COUNT):
+            g = greenlet.greenlet(recur)
+            g.gr_frames_always_exposed = EXPOSE_FRAMES
+            g2 = g.switch(RECUR_DEPTH)
+            assert g2 is g, (g2, g)
+            f = g2.gr_frame
+            assert f is not None, "frame is none"
+            count = 0
+            while f:
+                count += 1
+                f = f.f_back
+            # This assertion fails with the released versions of greenlet
+            # on Python 3.12
+            #assert count == RECUR_DEPTH + 1, (count, RECUR_DEPTH)
+            # Switch back so it can be collected; otherwise they build
+            # up forever.
+            g.switch()
+            # fall off the end of it and back to us.
+            del g
+            del g2
+            del f
+
+
+    end = pyperf.perf_counter()
+    return end - begin
+
+def bm_recur_frame_2(loops):
+    return _bm_recur_frame(loops, 2)
+
+def bm_recur_frame_20(loops):
+    return _bm_recur_frame(loops, 20)
+
+def bm_recur_frame_200(loops):
+    return _bm_recur_frame(loops, 200)
+
 if __name__ == '__main__':
     runner = pyperf.Runner()
+
     runner.bench_time_func(
         'create a greenlet',
         bm_create,
         inner_loops=CREATE_INNER_LOOPS
     )
 
     runner.bench_time_func(
-        'switch between two greenlets',
-        bm_switch,
+        'switch between two greenlets (shallow)',
+        bm_switch_shallow,
         inner_loops=SWITCH_INNER_LOOPS
     )
 
+    runner.bench_time_func(
+        'switch between two greenlets (deep)',
+        bm_switch_deep,
+        inner_loops=SWITCH_INNER_LOOPS
+    )
+
+    runner.bench_time_func(
+        'switch between two greenlets (deeper)',
+        bm_switch_deeper,
+        inner_loops=SWITCH_INNER_LOOPS
+    )
     runner.bench_time_func(
         'getcurrent single thread',
         bm_getcurrent,
@@ -110,3 +235,17 @@ def bm_create(loops):
         'chain(%s)' % CHAIN_GREENLET_COUNT,
         bm_chain,
     )
+
+    runner.bench_time_func(
+        'read 2 nested frames',
+        bm_recur_frame_2,
+    )
+
+    runner.bench_time_func(
+        'read 20 nested frames',
+        bm_recur_frame_20,
+    )
+    runner.bench_time_func(
+        'read 200 nested frames',
+        bm_recur_frame_200,
+    )

src/greenlet/TGreenlet.cpp

@@ -168,11 +168,7 @@ Greenlet::g_switchstack(void)
         current->exception_state << tstate;
         this->python_state.will_switch_from(tstate);
         switching_thread_state = this;
-#if GREENLET_PY312
-        if (current->python_state.expose_frames_on_every_suspension) {
-            current->expose_frames();
-        }
-#endif
+        current->expose_frames();
     }
     assert(this->args() || PyErr_Occurred());
     // If this is the first switch into a greenlet, this will
@@ -614,8 +610,8 @@ bool Greenlet::is_currently_running_in_some_thread() const
 #if GREENLET_PY312
 void GREENLET_NOINLINE(Greenlet::expose_frames)()
 {
-    if (!this->python_state.frame_exposure_needs_stack_rewrite()) {
-        return; // nothing to do
+    if (!this->python_state.top_frame()) {
+        return;
     }
 
     _PyInterpreterFrame* last_complete_iframe = nullptr;
@@ -680,8 +676,8 @@ void GREENLET_NOINLINE(Greenlet::expose_frames)()
             // which can't have happened yet because the frame is currently
             // executing as far as the interpreter is concerned. So, we can
             // reuse it for our own purposes.
-            assert(iframe->owner == FRAME_OWNED_BY_THREAD ||
-                   iframe->owner == FRAME_OWNED_BY_GENERATOR);
+            assert(iframe->owner == FRAME_OWNED_BY_THREAD
+                   || iframe->owner == FRAME_OWNED_BY_GENERATOR);
             if (last_complete_iframe) {
                 assert(last_complete_iframe->frame_obj);
                 memcpy(&last_complete_iframe->frame_obj->_f_frame_data[0],
@@ -707,6 +703,11 @@ void GREENLET_NOINLINE(Greenlet::expose_frames)()
                &last_complete_iframe->previous, sizeof(void *));
         last_complete_iframe->previous = nullptr;
     }
+}
+#else
+void Greenlet::expose_frames()
+{
+
 }
 #endif
 

src/greenlet/TPythonState.cpp

@@ -25,10 +25,6 @@ PythonState::PythonState()
     ,datastack_top(nullptr)
     ,datastack_limit(nullptr)
 #endif
-#if GREENLET_PY312
-    ,frames_were_exposed(false)
-    ,expose_frames_on_every_suspension(false)
-#endif
 {
 #if GREENLET_USE_CFRAME
     /*
@@ -162,21 +158,24 @@ void PythonState::operator<<(const PyThreadState *const tstate) noexcept
 #if GREENLET_PY312
 void GREENLET_NOINLINE(PythonState::unexpose_frames)()
 {
-    if (!this->frames_were_exposed) {
+    if (!this->top_frame()) {
         return;
     }
+
     // See GreenletState::expose_frames() and the comment on frames_were_exposed
     // for more information about this logic.
-    for (_PyInterpreterFrame *iframe = this->_top_frame->f_frame;
-         iframe != nullptr; ) {
+    _PyInterpreterFrame *iframe = this->_top_frame->f_frame;
+    while (iframe != nullptr) {
         _PyInterpreterFrame *prev_exposed = iframe->previous;
         assert(iframe->frame_obj);
         memcpy(&iframe->previous, &iframe->frame_obj->_f_frame_data[0],
                sizeof(void *));
         iframe = prev_exposed;
     }
-    this->frames_were_exposed = false;
 }
+#else
+void PythonState::unexpose_frames()
+{}
 #endif
 
 void PythonState::operator>>(PyThreadState *const tstate) noexcept
@@ -201,9 +200,7 @@ void PythonState::operator>>(PyThreadState *const tstate) noexcept
   #if GREENLET_PY312
     tstate->py_recursion_remaining = tstate->py_recursion_limit - this->py_recursion_depth;
     tstate->c_recursion_remaining = C_RECURSION_LIMIT - this->c_recursion_depth;
-    if (this->frames_were_exposed) {
-        this->unexpose_frames();
-    }
+    this->unexpose_frames();
   #else // \/ 3.11
     tstate->recursion_remaining = tstate->recursion_limit - this->recursion_depth;
   #endif // GREENLET_PY312

src/greenlet/TStackState.cpp

@@ -230,25 +230,27 @@ void StackState::copy_from_stack(void* vdest, const void* vsrc, size_t n) const
 {
     char* dest = static_cast<char*>(vdest);
     const char* src = static_cast<const char*>(vsrc);
-    if (src + n <= _stack_start || src >= _stack_start + _stack_saved ||
-        _stack_saved == 0) {
+    if (src + n <= this->_stack_start
+        || src >= this->_stack_start + this->_stack_saved
+        || this->_stack_saved == 0) {
         // Nothing we're copying was spilled from the stack
         memcpy(dest, src, n);
         return;
     }
-    if (src < _stack_start) {
+
+    if (src < this->_stack_start) {
         // Copy the part before the saved stack.
         // We know src + n > _stack_start due to the test above.
-        size_t nbefore = _stack_start - src;
+        const size_t nbefore = this->_stack_start - src;
         memcpy(dest, src, nbefore);
         dest += nbefore;
         src += nbefore;
         n -= nbefore;
     }
     // We know src >= _stack_start after the before-copy, and
     // src < _stack_start + _stack_saved due to the first if condition
-    size_t nspilled = std::min<size_t>(n, _stack_start + _stack_saved - src);
-    memcpy(dest, stack_copy + (src - _stack_start), nspilled);
+    size_t nspilled = std::min<size_t>(n, this->_stack_start + this->_stack_saved - src);
+    memcpy(dest, this->stack_copy + (src - this->_stack_start), nspilled);
     dest += nspilled;
     src += nspilled;
     n -= nspilled;

src/greenlet/greenlet.cpp

@@ -758,38 +758,10 @@ green_setcontext(BorrowedGreenlet self, PyObject* nctx, void* UNUSED(context))
 static PyObject*
 green_getframe(BorrowedGreenlet self, void* UNUSED(context))
 {
-#if GREENLET_PY312
-    self->expose_frames();
-#endif
     const PythonState::OwnedFrame& top_frame = self->top_frame();
     return top_frame.acquire_or_None();
 }
 
-static PyObject*
-green_getframeexposed(BorrowedGreenlet self, void* UNUSED(context))
-{
-#if GREENLET_PY312
-    if (!self->expose_frames_on_every_suspension()) {
-        Py_RETURN_FALSE;
-    }
-#endif
-    Py_RETURN_TRUE;
-}
-
-static int
-green_setframeexposed(BorrowedGreenlet self, PyObject* val, void* UNUSED(context))
-{
-    if (val != Py_True && val != Py_False) {
-        PyErr_Format(PyExc_TypeError,
-                     "expected a bool, not '%s'",
-                     Py_TYPE(val)->tp_name);
-        return -1;
-    }
-#if GREENLET_PY312
-    self->set_expose_frames_on_every_suspension(val == Py_True);
-#endif
-    return 0;
-}
 
 static PyObject*
 green_getstate(PyGreenlet* self)
@@ -1008,10 +980,6 @@ static PyGetSetDef green_getsets[] = {
     {"run", (getter)green_getrun, (setter)green_setrun, /*XXX*/ NULL},
     {"parent", (getter)green_getparent, (setter)green_setparent, /*XXX*/ NULL},
     {"gr_frame", (getter)green_getframe, NULL, /*XXX*/ NULL},
-    {"gr_frames_always_exposed",
-     (getter)green_getframeexposed,
-     (setter)green_setframeexposed,
-     /*XXX*/ NULL},
     {"gr_context",
      (getter)green_getcontext,
      (setter)green_setcontext,