Changes from review

Author: colesbury

Include/internal/pycore_genobject.h

@@ -34,30 +34,6 @@ PyGenObject *_PyGen_GetGeneratorFromFrame(_PyInterpreterFrame *frame)
     return (PyGenObject *)(((char *)frame) - offset_in_gen);
 }
 
-// Mark the generator as executing. Returns true if the state was changed,
-// false if it was already executing or finished.
-static inline bool
-_PyGen_SetExecuting(PyGenObject *gen)
-{
-#ifdef Py_GIL_DISABLED
-    if (!_PyObject_IsUniquelyReferenced((PyObject *)gen)) {
-        int8_t frame_state = _Py_atomic_load_int8_relaxed(&gen->gi_frame_state);
-        while (frame_state < FRAME_EXECUTING) {
-            if (_Py_atomic_compare_exchange_int8(&gen->gi_frame_state,
-                                                 &frame_state,
-                                                 FRAME_EXECUTING)) {
-                return true;
-            }
-        }
-    }
-#endif
-    if (gen->gi_frame_state < FRAME_EXECUTING) {
-        gen->gi_frame_state = FRAME_EXECUTING;
-        return true;
-    }
-    return false;
-}
-
 extern void _PyGen_Finalize(PyObject *self);
 
 // Export for '_asyncio' shared extension

Objects/genobject.c

@@ -234,7 +234,7 @@ gen_send_ex2(PyGenObject *gen, PyObject *arg, PyObject **presult, int exc)
         _PyErr_ChainStackItem();
     }
 
-#if defined(Py_GIL_DISABLED) && defined(Py_DEBUG)
+#if defined(Py_GIL_DISABLED)
     ((_PyThreadStateImpl *)tstate)->gen_last_frame_state = FRAME_EXECUTING;
 #endif
 

Python/bytecodes.c

@@ -1338,7 +1338,7 @@ dummy_func(
             assert(frame->owner != FRAME_OWNED_BY_INTERPRETER);
             if ((tstate->interp->eval_frame == NULL) &&
                 (Py_TYPE(receiver_o) == &PyGen_Type || Py_TYPE(receiver_o) == &PyCoro_Type) &&
-                _PyGen_SetExecuting((PyGenObject *)receiver_o))
+                gen_try_set_executing((PyGenObject *)receiver_o))
             {
                 PyGenObject *gen = (PyGenObject *)receiver_o;
                 _PyInterpreterFrame *gen_frame = &gen->gi_iframe;
@@ -1385,7 +1385,7 @@ dummy_func(
         op(_SEND_GEN_FRAME, (receiver, v -- receiver, gen_frame)) {
             PyGenObject *gen = (PyGenObject *)PyStackRef_AsPyObjectBorrow(receiver);
             DEOPT_IF(Py_TYPE(gen) != &PyGen_Type && Py_TYPE(gen) != &PyCoro_Type);
-            DEOPT_IF(!_PyGen_SetExecuting((PyGenObject *)gen));
+            DEOPT_IF(!gen_try_set_executing((PyGenObject *)gen));
             STAT_INC(SEND, hit);
             _PyInterpreterFrame *pushed_frame = &gen->gi_iframe;
             _PyFrame_StackPush(pushed_frame, PyStackRef_MakeHeapSafe(v));
@@ -1422,10 +1422,7 @@ dummy_func(
             _PyInterpreterFrame *gen_frame = frame;
             frame = tstate->current_frame = frame->previous;
             gen_frame->previous = NULL;
-            FT_ATOMIC_STORE_INT8_RELEASE(gen->gi_frame_state, FRAME_SUSPENDED + oparg);
-#ifdef Py_GIL_DISABLED
-            ((_PyThreadStateImpl *)tstate)->gen_last_frame_state = FRAME_SUSPENDED + oparg;
-#endif
+            gen_set_frame_state(gen, tstate, FRAME_SUSPENDED + oparg);
             /* We don't know which of these is relevant here, so keep them equal */
             assert(INLINE_CACHE_ENTRIES_SEND == INLINE_CACHE_ENTRIES_FOR_ITER);
             #if TIER_ONE
@@ -3424,7 +3421,7 @@ dummy_func(
         op(_FOR_ITER_GEN_FRAME, (iter, null -- iter, null, gen_frame)) {
             PyGenObject *gen = (PyGenObject *)PyStackRef_AsPyObjectBorrow(iter);
             DEOPT_IF(Py_TYPE(gen) != &PyGen_Type);
-            DEOPT_IF(!_PyGen_SetExecuting((PyGenObject *)gen));
+            DEOPT_IF(!gen_try_set_executing((PyGenObject *)gen));
             STAT_INC(FOR_ITER, hit);
             _PyInterpreterFrame *pushed_frame = &gen->gi_iframe;
             _PyFrame_StackPush(pushed_frame, PyStackRef_None);

Python/ceval.c

@@ -2304,10 +2304,7 @@ clear_gen_frame(PyThreadState *tstate, _PyInterpreterFrame * frame)
 {
     assert(frame->owner == FRAME_OWNED_BY_GENERATOR);
     PyGenObject *gen = _PyGen_GetGeneratorFromFrame(frame);
-    FT_ATOMIC_STORE_INT8_RELEASE(gen->gi_frame_state, FRAME_CLEARED);
-#ifdef Py_GIL_DISABLED
-    ((_PyThreadStateImpl *)tstate)->gen_last_frame_state = FRAME_CLEARED;
-#endif
+    gen_set_frame_state(gen, tstate, FRAME_CLEARED);
     assert(tstate->exc_info == &gen->gi_exc_state);
     tstate->exc_info = gen->gi_exc_state.previous_item;
     gen->gi_exc_state.previous_item = NULL;

Python/ceval_macros.h

@@ -495,3 +495,39 @@ check_periodics(PyThreadState *tstate) {
     return 0;
 }
 
+
+
+// Mark the generator as executing. Returns true if the state was changed,
+// false if it was already executing or finished.
+static inline bool
+gen_try_set_executing(PyGenObject *gen)
+{
+#ifdef Py_GIL_DISABLED
+    if (!_PyObject_IsUniquelyReferenced((PyObject *)gen)) {
+        int8_t frame_state = _Py_atomic_load_int8_relaxed(&gen->gi_frame_state);
+        while (frame_state < FRAME_EXECUTING) {
+            if (_Py_atomic_compare_exchange_int8(&gen->gi_frame_state,
+                                                 &frame_state,
+                                                 FRAME_EXECUTING)) {
+                return true;
+            }
+        }
+    }
+#endif
+    // Use faster non-atomic modifications in the GIL-enabled build and when
+    // the object is uniquely referenced in the free-threaded build.
+    if (gen->gi_frame_state < FRAME_EXECUTING) {
+        gen->gi_frame_state = FRAME_EXECUTING;
+        return true;
+    }
+    return false;
+}
+
+static inline void
+gen_set_frame_state(PyGenObject *gen, PyThreadState *tstate, int8_t frame_state)
+{
+    FT_ATOMIC_STORE_INT8_RELEASE(gen->gi_frame_state, frame_state);
+#ifdef Py_GIL_DISABLED
+    ((_PyThreadStateImpl *)tstate)->gen_last_frame_state = frame_state;
+#endif
+}

Python/executor_cases.c.h

@@ -643,7 +643,8 @@ def has_error_without_pop(op: parser.CodeDef) -> bool:
     "_PyFrame_StackPush",
     "_PyFunction_SetVersion",
     "_PyGen_GetGeneratorFromFrame",
-    "_PyGen_SetExecuting",
+    "gen_try_set_executing",
+    "gen_set_frame_state",
     "_PyInterpreterState_GET",
     "_PyList_AppendTakeRef",
     "_PyList_ITEMS",