From 468ef723a49f3b375af273aee7a362885ac70d99 Mon Sep 17 00:00:00 2001 From: Itamar Ostricher Date: Tue, 22 Nov 2022 17:22:42 -0800 Subject: [PATCH 1/5] Cleanup nits in watchers-related code - Remove `dict_watchers` array on the interpreter state (was never used, likely added in [the function watchers PR](https://github.com/python/cpython/pull/98175/files#diff-cb06b7ba43789a1d6ae5bef46baa8a255b72dbcd2b44bfec7f146ea4e94dbfa0) when resolving a merge conflict) - Cast `METH_NOARGS` function to `(PyCFunction)` in watchers test methods - Initialize `type_watchers` array to `NULL`s --- Include/internal/pycore_interp.h | 1 - Modules/_testcapi/watchers.c | 6 ++++-- Python/pystate.c | 4 ++++ 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h index b5c46773c90fd3..ed90f6392019fa 100644 --- a/Include/internal/pycore_interp.h +++ b/Include/internal/pycore_interp.h @@ -172,7 +172,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 index 1d91c206f63092..2e8fe1dbf78651 100644 --- 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/Python/pystate.c b/Python/pystate.c index 0fdcdf1e3569b1..e006bf2517cfff 100644 --- 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; } From d380f0cfdacf5051e2ee3844a0a09126cdc2f856 Mon Sep 17 00:00:00 2001 From: Itamar Ostricher Date: Sun, 4 Dec 2022 15:44:49 -0800 Subject: [PATCH 2/5] Optimize code watchers notification --- Objects/codeobject.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/Objects/codeobject.c b/Objects/codeobject.c index 0c197d767b0a23..f54acc3bbc7c40 100644 --- a/Objects/codeobject.c +++ b/Objects/codeobject.c @@ -16,14 +16,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; } } From 8e2e17a4fe748dddf4b7f900bef650814f6b894e Mon Sep 17 00:00:00 2001 From: Itamar Ostricher Date: Sun, 4 Dec 2022 15:47:58 -0800 Subject: [PATCH 3/5] Replace invariant test with assert in type watchers notification --- Objects/typeobject.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index ae80f5a8fd88e0..2c3b39521a6d9f 100644 --- 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 */ } From 0eaed3ec2d0da98ef990cf4e4e4a67386d5dce61 Mon Sep 17 00:00:00 2001 From: Itamar Ostricher Date: Sun, 4 Dec 2022 16:32:15 -0800 Subject: [PATCH 4/5] Optimize func watchers notification --- Objects/funcobject.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/Objects/funcobject.c b/Objects/funcobject.c index bf97edc53ad7d9..458a72447a3dfc 100644 --- a/Objects/funcobject.c +++ b/Objects/funcobject.c @@ -12,11 +12,18 @@ 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]; + if ((cb != NULL) && (cb(event, func, new_value) < 0)) { + PyErr_WriteUnraisable((PyObject *) func); + } } + i++; + bits >>= 1; } } @@ -25,6 +32,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); } From 40044c09b4cd796df47a1eddff10b59d319dc7f0 Mon Sep 17 00:00:00 2001 From: Itamar Ostricher Date: Mon, 5 Dec 2022 11:35:21 -0800 Subject: [PATCH 5/5] Replace invariant test with assert in function watchers dispatch --- Objects/funcobject.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Objects/funcobject.c b/Objects/funcobject.c index 458a72447a3dfc..12f9843e319926 100644 --- a/Objects/funcobject.c +++ b/Objects/funcobject.c @@ -18,7 +18,9 @@ notify_func_watchers(PyInterpreterState *interp, PyFunction_WatchEvent event, assert(i < FUNC_MAX_WATCHERS); if (bits & 1) { PyFunction_WatchCallback cb = interp->func_watchers[i]; - if ((cb != NULL) && (cb(event, func, new_value) < 0)) { + // 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); } }