GH-100000: Cleanup and polish various watchers code (GH-99998)

* Initialize `type_watchers` array to `NULL`s * Optimize code watchers notification * Optimize func watchers notification
python · Dec 14, 2022 · ae83c78 · ae83c78
1 parent aa8591e
commit ae83c78
Show file tree

Hide file tree

Showing 6 changed files with 37 additions and 14 deletions.
diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h
@@ -142,7 +142,6 @@ struct _is {
     // Initialized to _PyEval_EvalFrameDefault().
     _PyFrameEvalFunction eval_frame;
 
-    PyDict_WatchCallback dict_watchers[DICT_MAX_WATCHERS];
     PyFunction_WatchCallback func_watchers[FUNC_MAX_WATCHERS];
     // One bit is set for each non-NULL entry in func_watchers
     uint8_t active_func_watchers;

diff --git a/Modules/_testcapi/watchers.c b/Modules/_testcapi/watchers.c
@@ -630,14 +630,16 @@ static PyMethodDef test_methods[] = {
     {"clear_dict_watcher",       clear_dict_watcher,      METH_O,       NULL},
     {"watch_dict",               watch_dict,              METH_VARARGS, NULL},
     {"unwatch_dict",             unwatch_dict,            METH_VARARGS, NULL},
-    {"get_dict_watcher_events",  get_dict_watcher_events, METH_NOARGS,  NULL},
+    {"get_dict_watcher_events",
+     (PyCFunction) get_dict_watcher_events,               METH_NOARGS,  NULL},
 
     // Type watchers.
     {"add_type_watcher",         add_type_watcher,        METH_O,       NULL},
     {"clear_type_watcher",       clear_type_watcher,      METH_O,       NULL},
     {"watch_type",               watch_type,              METH_VARARGS, NULL},
     {"unwatch_type",             unwatch_type,            METH_VARARGS, NULL},
-    {"get_type_modified_events", get_type_modified_events, METH_NOARGS, NULL},
+    {"get_type_modified_events",
+     (PyCFunction) get_type_modified_events,              METH_NOARGS, NULL},
 
     // Code object watchers.
     {"add_code_watcher",         add_code_watcher,        METH_O,       NULL},

diff --git a/Objects/codeobject.c b/Objects/codeobject.c
@@ -15,14 +15,21 @@ static void
 notify_code_watchers(PyCodeEvent event, PyCodeObject *co)
 {
     PyInterpreterState *interp = _PyInterpreterState_GET();
-    if (interp->active_code_watchers) {
-        assert(interp->_initialized);
-        for (int i = 0; i < CODE_MAX_WATCHERS; i++) {
+    assert(interp->_initialized);
+    uint8_t bits = interp->active_code_watchers;
+    int i = 0;
+    while (bits) {
+        assert(i < CODE_MAX_WATCHERS);
+        if (bits & 1) {
             PyCode_WatchCallback cb = interp->code_watchers[i];
-            if ((cb != NULL) && (cb(event, co) < 0)) {
+            // callback must be non-null if the watcher bit is set
+            assert(cb != NULL);
+            if (cb(event, co) < 0) {
                 PyErr_WriteUnraisable((PyObject *) co);
             }
         }
+        i++;
+        bits >>= 1;
     }
 }
 

diff --git a/Objects/funcobject.c b/Objects/funcobject.c
@@ -12,11 +12,20 @@ static void
 notify_func_watchers(PyInterpreterState *interp, PyFunction_WatchEvent event,
                      PyFunctionObject *func, PyObject *new_value)
 {
-    for (int i = 0; i < FUNC_MAX_WATCHERS; i++) {
-        PyFunction_WatchCallback cb = interp->func_watchers[i];
-        if ((cb != NULL) && (cb(event, func, new_value) < 0)) {
-            PyErr_WriteUnraisable((PyObject *) func);
+    uint8_t bits = interp->active_func_watchers;
+    int i = 0;
+    while (bits) {
+        assert(i < FUNC_MAX_WATCHERS);
+        if (bits & 1) {
+            PyFunction_WatchCallback cb = interp->func_watchers[i];
+            // callback must be non-null if the watcher bit is set
+            assert(cb != NULL);
+            if (cb(event, func, new_value) < 0) {
+                PyErr_WriteUnraisable((PyObject *) func);
+            }
         }
+        i++;
+        bits >>= 1;
     }
 }
 
@@ -25,6 +34,7 @@ handle_func_event(PyFunction_WatchEvent event, PyFunctionObject *func,
                   PyObject *new_value)
 {
     PyInterpreterState *interp = _PyInterpreterState_GET();
+    assert(interp->_initialized);
     if (interp->active_func_watchers) {
         notify_func_watchers(interp, event, func, new_value);
     }

diff --git a/Objects/typeobject.c b/Objects/typeobject.c
@@ -485,23 +485,24 @@ PyType_Modified(PyTypeObject *type)
         }
     }
 
+    // Notify registered type watchers, if any
     if (type->tp_watched) {
         PyInterpreterState *interp = _PyInterpreterState_GET();
         int bits = type->tp_watched;
         int i = 0;
-        while(bits && i < TYPE_MAX_WATCHERS) {
+        while (bits) {
+            assert(i < TYPE_MAX_WATCHERS);
             if (bits & 1) {
                 PyType_WatchCallback cb = interp->type_watchers[i];
                 if (cb && (cb(type) < 0)) {
                     PyErr_WriteUnraisable((PyObject *)type);
                 }
             }
-            i += 1;
+            i++;
             bits >>= 1;
         }
     }
 
-
     type->tp_flags &= ~Py_TPFLAGS_VALID_VERSION_TAG;
     type->tp_version_tag = 0; /* 0 is not a valid version tag */
 }

diff --git a/Python/pystate.c b/Python/pystate.c
@@ -461,6 +461,10 @@ interpreter_clear(PyInterpreterState *interp, PyThreadState *tstate)
         interp->dict_state.watchers[i] = NULL;
     }
 
+    for (int i=0; i < TYPE_MAX_WATCHERS; i++) {
+        interp->type_watchers[i] = NULL;
+    }
+
     for (int i=0; i < FUNC_MAX_WATCHERS; i++) {
         interp->func_watchers[i] = NULL;
     }