Address review comments: use _Py_LOCK_DONT_DETACH, atomic OR/AND, _PyOnceFlag

Author: yoney

Include/internal/pycore_dict_state.h

@@ -8,15 +8,14 @@ extern "C" {
 #  error "this header requires Py_BUILD_CORE define"
 #endif
 
-#include "pycore_lock.h"          // PyMutex
-
 #define DICT_MAX_WATCHERS 8
 #define DICT_WATCHED_MUTATION_BITS 4
 
 struct _Py_dict_state {
     uint32_t next_keys_version;
-    PyDict_WatchCallback watchers[DICT_MAX_WATCHERS];
     PyMutex watcher_mutex;        // Protects the watchers array (free-threaded builds)
+    _PyOnceFlag watcher_setup_once; // One-time optimizer watcher setup
+    PyDict_WatchCallback watchers[DICT_MAX_WATCHERS];
 };
 
 #define _dict_state_INIT \

Include/internal/pycore_pyatomic_ft_wrappers.h

@@ -129,7 +129,12 @@ extern "C" {
     (void)_Py_atomic_add_ssize(&value, new_value)
 #define FT_ATOMIC_ADD_UINT64(value, new_value) \
     (void)_Py_atomic_add_uint64(&value, new_value)
+#define FT_ATOMIC_OR_UINT64(value, new_value) \
+    (void)_Py_atomic_or_uint64(&value, new_value)
+#define FT_ATOMIC_AND_UINT64(value, new_value) \
+    (void)_Py_atomic_and_uint64(&value, new_value)
 #define FT_MUTEX_LOCK(lock) PyMutex_Lock(lock)
+#define FT_MUTEX_LOCK_FLAGS(lock, flags) PyMutex_LockFlags(lock, flags)
 #define FT_MUTEX_UNLOCK(lock) PyMutex_Unlock(lock)
 
 #else
@@ -188,7 +193,10 @@ extern "C" {
 #define FT_ATOMIC_STORE_ULLONG_RELAXED(value, new_value) value = new_value
 #define FT_ATOMIC_ADD_SSIZE(value, new_value) (void)(value += new_value)
 #define FT_ATOMIC_ADD_UINT64(value, new_value) (void)(value += new_value)
+#define FT_ATOMIC_OR_UINT64(value, new_value) (void)(value |= new_value)
+#define FT_ATOMIC_AND_UINT64(value, new_value) (void)(value &= new_value)
 #define FT_MUTEX_LOCK(lock) do {} while (0)
+#define FT_MUTEX_LOCK_FLAGS(lock, flags) do {} while (0)
 #define FT_MUTEX_UNLOCK(lock) do {} while (0)
 
 #endif

Objects/dictobject.c

@@ -7866,9 +7866,8 @@ PyDict_Watch(int watcher_id, PyObject* dict)
     if (validate_watcher_id(interp, watcher_id)) {
         return -1;
     }
-    Py_BEGIN_CRITICAL_SECTION(dict);
-    ((PyDictObject*)dict)->_ma_watcher_tag |= (1ULL << watcher_id);
-    Py_END_CRITICAL_SECTION();
+    FT_ATOMIC_OR_UINT64(((PyDictObject*)dict)->_ma_watcher_tag,
+                        1ULL << watcher_id);
     return 0;
 }
 
@@ -7883,9 +7882,8 @@ PyDict_Unwatch(int watcher_id, PyObject* dict)
     if (validate_watcher_id(interp, watcher_id)) {
         return -1;
     }
-    Py_BEGIN_CRITICAL_SECTION(dict);
-    ((PyDictObject*)dict)->_ma_watcher_tag &= ~(1ULL << watcher_id);
-    Py_END_CRITICAL_SECTION();
+    FT_ATOMIC_AND_UINT64(((PyDictObject*)dict)->_ma_watcher_tag,
+                         ~(1ULL << watcher_id));
     return 0;
 }
 
@@ -7895,7 +7893,8 @@ PyDict_AddWatcher(PyDict_WatchCallback callback)
     int watcher_id = -1;
     PyInterpreterState *interp = _PyInterpreterState_GET();
 
-    FT_MUTEX_LOCK(&interp->dict_state.watcher_mutex);
+    FT_MUTEX_LOCK_FLAGS(&interp->dict_state.watcher_mutex,
+                        _Py_LOCK_DONT_DETACH);
     /* Some watchers are reserved for CPython, start at the first available one */
     for (int i = FIRST_AVAILABLE_WATCHER; i < DICT_MAX_WATCHERS; i++) {
         if (!interp->dict_state.watchers[i]) {
@@ -7915,7 +7914,8 @@ PyDict_ClearWatcher(int watcher_id)
 {
     int res = 0;
     PyInterpreterState *interp = _PyInterpreterState_GET();
-    FT_MUTEX_LOCK(&interp->dict_state.watcher_mutex);
+    FT_MUTEX_LOCK_FLAGS(&interp->dict_state.watcher_mutex,
+                        _Py_LOCK_DONT_DETACH);
     if (validate_watcher_id(interp, watcher_id)) {
         res = -1;
         goto done;

Python/optimizer_analysis.c

@@ -155,6 +155,17 @@ type_watcher_callback(PyTypeObject* type)
     return 0;
 }
 
+static int
+_setup_optimizer_watchers(void *Py_UNUSED(arg))
+{
+    PyInterpreterState *interp = _PyInterpreterState_GET();
+    FT_ATOMIC_STORE_PTR_RELEASE(
+        interp->dict_state.watchers[GLOBALS_WATCHER_ID],
+        globals_watcher_callback);
+    interp->type_watchers[TYPE_WATCHER_ID] = type_watcher_callback;
+    return 0;
+}
+
 static PyObject *
 convert_global_to_const(_PyUOpInstruction *inst, PyObject *obj, bool pop, bool insert)
 {
@@ -469,12 +480,8 @@ optimize_uops(
 
     // Make sure that watchers are set up
     PyInterpreterState *interp = _PyInterpreterState_GET();
-    if (FT_ATOMIC_LOAD_PTR_RELAXED(interp->dict_state.watchers[GLOBALS_WATCHER_ID]) == NULL) {
-        FT_ATOMIC_STORE_PTR_RELEASE(
-            interp->dict_state.watchers[GLOBALS_WATCHER_ID],
-            globals_watcher_callback);
-        interp->type_watchers[TYPE_WATCHER_ID] = type_watcher_callback;
-    }
+    _PyOnceFlag_CallOnce(&interp->dict_state.watcher_setup_once,
+                         _setup_optimizer_watchers, NULL);
 
     _Py_uop_abstractcontext_init(ctx, dependencies);
     _Py_UOpsAbstractFrame *frame = _Py_uop_frame_new(ctx, (PyCodeObject *)func->func_code, NULL, 0);