From 2c7444399ffcd4b2b8765f394c6a1c9c909ff94c Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Mon, 3 Oct 2022 14:58:58 -0700 Subject: [PATCH] Update to multiple-watchers API. --- Doc/c-api/dict.rst | 30 +++----- Include/cpython/dictobject.h | 15 ++-- Include/internal/pycore_dict.h | 28 +++++++- Include/internal/pycore_interp.h | 2 +- Modules/_testcapimodule.c | 46 ++----------- Objects/dictobject.c | 114 ++++++++++++++----------------- Python/ceval.c | 34 +-------- 7 files changed, 99 insertions(+), 170 deletions(-) diff --git a/Doc/c-api/dict.rst b/Doc/c-api/dict.rst index 782fded5b789c31..1b31729d9e7ebd7 100644 --- a/Doc/c-api/dict.rst +++ b/Doc/c-api/dict.rst @@ -239,15 +239,18 @@ Dictionary Objects if override or key not in a: a[key] = value -.. c:function:: void PyDict_Watch(PyObject *dict) +.. c:function:: int PyDict_AddWatcher(PyDict_WatchCallback callback) - Mark dictionary *dict* as watched. The callback set via - :c:func:`PyDict_SetWatchCallback` will be called when *dict* is modified or - deallocated. + Register *callback* as a dictionary watcher. Return a non-negative integer + id which must be passed to future calls to e.g. :c:func:`PyDict_Watch`. In + case of error (e.g. no more watcher IDs available), return ``-1`` and set an + exception. -.. c:function:: int PyDict_IsWatched(PyObject *dict) +.. c:function:: int PyDict_Watch(int watcher_id, PyObject *dict) - Return ``1`` if *dict* is marked as watched, ``0`` otherwise. + Mark dictionary *dict* as watched. The callback granted *watcher_id* by + :c:func:`PyDict_AddWatcher` will be called when *dict* is modified or + deallocated. .. c:type:: PyDict_WatchEvent @@ -271,23 +274,8 @@ Dictionary Objects single ``PyDict_EVENT_CLONED`` is issued, and *key* will be the source dictionary. -.. c:function:: void PyDict_SetWatchCallback(PyDict_WatchCallback callback) - - Set a callback for modification events on dictionaries watched via - :c:func:`PyDict_Watch`. - - There is only one callback per interpreter. Before setting the callback, you - must check if there is one already set (use - :c:func:`PyDict_GetWatchCallback`) and if so, call it from your own new - callback. Failure to do this is a critical bug in your callback and may break - other dict-watching clients. - The callback may inspect but should not modify *dict*; doing so could have unpredictable effects, including infinite recursion. Callbacks occur before the notified modification to *dict* takes place, so the prior state of *dict* can be inspected. - -.. c:function:: PyDict_WatchCallback PyDict_GetWatchCallback(void) - - Return the existing dictionary watcher callback, or ``NULL`` if none has been set. diff --git a/Include/cpython/dictobject.h b/Include/cpython/dictobject.h index 0216a77b514e882..2f3cedd660b8bd9 100644 --- a/Include/cpython/dictobject.h +++ b/Include/cpython/dictobject.h @@ -2,6 +2,7 @@ # error "this header file must not be included directly" #endif + typedef struct _dictkeysobject PyDictKeysObject; typedef struct _dictvalues PyDictValues; @@ -79,12 +80,6 @@ PyAPI_FUNC(PyObject *) _PyDictView_Intersect(PyObject* self, PyObject *other); /* Dictionary watchers */ -// Mark given dictionary as "watched" (callback will be called if it is modified) -PyAPI_FUNC(int) PyDict_Watch(PyObject* dict); - -// Check if given dictionary is watched -PyAPI_FUNC(int) PyDict_IsWatched(PyObject* dict); - typedef enum { PyDict_EVENT_ADDED, PyDict_EVENT_MODIFIED, @@ -99,8 +94,8 @@ typedef enum { // new value for key, NULL if key is being deleted. typedef void(*PyDict_WatchCallback)(PyDict_WatchEvent event, PyObject* dict, PyObject* key, PyObject* new_value); -// Set new global watch callback; supply NULL to clear callback -PyAPI_FUNC(void) PyDict_SetWatchCallback(PyDict_WatchCallback callback); +// Register a dict-watcher callback +PyAPI_FUNC(int) PyDict_AddWatcher(PyDict_WatchCallback callback); -// Get existing global watch callback -PyAPI_FUNC(PyDict_WatchCallback) PyDict_GetWatchCallback(void); +// Mark given dictionary as "watched" (callback will be called if it is modified) +PyAPI_FUNC(int) PyDict_Watch(int watcher_id, PyObject* dict); diff --git a/Include/internal/pycore_dict.h b/Include/internal/pycore_dict.h index 856db0eee302f3b..1bb9d650665510f 100644 --- a/Include/internal/pycore_dict.h +++ b/Include/internal/pycore_dict.h @@ -160,9 +160,33 @@ struct _dictvalues { #define DK_IS_UNICODE(dk) ((dk)->dk_kind != DICT_KEYS_GENERAL) extern uint64_t _pydict_global_version; -#define DICT_VERSION_WATCHED_TAG 1 -#define DICT_NEXT_VERSION() (_pydict_global_version += 2) +#define DICT_MAX_WATCHERS 8 +#define DICT_VERSION_MASK 255 +#define DICT_VERSION_INCREMENT 256 + +#define DICT_NEXT_VERSION() (_pydict_global_version += DICT_VERSION_INCREMENT) + +void +_PyDict_SendEvent(int watcher_bits, + PyDict_WatchEvent event, + PyDictObject *mp, + PyObject *key, + PyObject *value); + +static inline uint64_t +_PyDict_NotifyEvent(PyDict_WatchEvent event, + PyDictObject *mp, + PyObject *key, + PyObject *value) +{ + int watcher_bits = mp->ma_version_tag & DICT_VERSION_MASK; + if (watcher_bits) { + _PyDict_SendEvent(watcher_bits, event, mp, key, value); + return DICT_NEXT_VERSION() | watcher_bits; + } + return DICT_NEXT_VERSION(); +} extern PyObject *_PyObject_MakeDictFromInstanceAttributes(PyObject *obj, PyDictValues *values); extern PyObject *_PyDict_FromItems( diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h index af31407c7dce0fa..cc922c341fc5043 100644 --- a/Include/internal/pycore_interp.h +++ b/Include/internal/pycore_interp.h @@ -148,7 +148,7 @@ struct _is { // Initialized to _PyEval_EvalFrameDefault(). _PyFrameEvalFunction eval_frame; - void *dict_watch_callback; + void *dict_watchers[8]; Py_ssize_t co_extra_user_count; freefunc co_extra_freefuncs[MAX_CO_EXTRA_USERS]; diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index a84177f8b40b11f..0fa9779b018fb72 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -5777,7 +5777,6 @@ test_tstate_capi(PyObject *self, PyObject *Py_UNUSED(args)) // Test dict watching static PyObject *g_dict_watch_events; -static PyDict_WatchCallback g_prev_callback; static void dict_watch_callback(PyDict_WatchEvent event, @@ -5810,9 +5809,6 @@ dict_watch_callback(PyDict_WatchEvent event, } assert(PyList_Check(g_dict_watch_events)); PyList_Append(g_dict_watch_events, msg); - if (g_prev_callback != NULL) { - g_prev_callback(event, dict, key, new_value); - } } static int @@ -5845,8 +5841,8 @@ dict_watch_assert(Py_ssize_t expected_num_events, } static int -try_watch(PyObject *obj) { - if (PyDict_Watch(obj)) { +try_watch(int watcher_id, PyObject *obj) { + if (PyDict_Watch(watcher_id, obj)) { raiseTestError("test_watch_dict", "PyDict_Watch() failed on dict"); return -1; } @@ -5864,23 +5860,12 @@ test_watch_dict(PyObject *self, PyObject *Py_UNUSED(args)) PyObject *key2 = PyUnicode_FromString("key2"); g_dict_watch_events = PyList_New(0); - g_prev_callback = PyDict_GetWatchCallback(); - PyDict_SetWatchCallback(dict_watch_callback); - if (PyDict_GetWatchCallback() != dict_watch_callback) { - return raiseTestError("test_watch_dict", "GetWatchCallback did not return set callback"); - } - if (try_watch(watched)) { + int wid = PyDict_AddWatcher(dict_watch_callback); + if (try_watch(wid, watched)) { return NULL; } - if (!PyDict_IsWatched(watched)) { - return raiseTestError("test_watch_dict", "IsWatched returned false for watched dict"); - } - if (PyDict_IsWatched(unwatched)) { - return raiseTestError("test_watch_dict", "IsWatched returned true for unwatched dict"); - } - PyDict_SetItem(unwatched, key1, two); PyDict_Merge(watched, unwatched, 1); @@ -5938,9 +5923,7 @@ test_watch_dict(PyObject *self, PyObject *Py_UNUSED(args)) } PyObject *copy = PyDict_Copy(watched); - if (PyDict_IsWatched(copy)) { - return raiseTestError("test_watch_dict", "copying a watched dict should not watch the copy"); - } + // copied dict is not watched, so this does not add an event Py_CLEAR(copy); Py_CLEAR(watched); @@ -5950,24 +5933,8 @@ test_watch_dict(PyObject *self, PyObject *Py_UNUSED(args)) return NULL; } - PyDict_SetWatchCallback(g_prev_callback); - g_prev_callback = NULL; - - // no events after callback unset - watched = PyDict_New(); - if (try_watch(watched)) { - return NULL; - } - - PyDict_SetItem(watched, key1, one); - Py_CLEAR(watched); - - if (dict_watch_assert(9, "dealloc")) { - return NULL; - } - // it is an error to try to watch a non-dict - if (!PyDict_Watch(one)) { + if (!PyDict_Watch(wid, one)) { raiseTestError("test_watch_dict", "PyDict_Watch() succeeded on non-dict"); return NULL; } else if (!PyErr_Occurred()) { @@ -5977,7 +5944,6 @@ test_watch_dict(PyObject *self, PyObject *Py_UNUSED(args)) PyErr_Clear(); } - Py_CLEAR(g_dict_watch_events); Py_DECREF(one); Py_DECREF(two); diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 8465b8c6ad415ca..20dffd5a7dfa6d4 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -289,47 +289,6 @@ unicode_get_hash(PyObject *o) return ((PyASCIIObject*)o)->hash; } -PyDict_WatchCallback -PyDict_GetWatchCallback(void) -{ - PyThreadState *tstate = PyThreadState_GET(); - return (PyDict_WatchCallback)tstate->interp->dict_watch_callback; -} - -static inline int -dict_is_watched(PyDictObject *mp) -{ - return mp->ma_version_tag & DICT_VERSION_WATCHED_TAG; -} - -/* If we're using GCC, use __builtin_expect() to reduce overhead of - the watched-dict checks */ -#if defined(__GNUC__) && (__GNUC__ > 2) && defined(__OPTIMIZE__) -# define UNLIKELY(value) __builtin_expect((value), 0) -# define LIKELY(value) __builtin_expect((value), 1) -#else -# define UNLIKELY(value) (value) -# define LIKELY(value) (value) -#endif - -static inline uint64_t -notify_dict_event(PyDict_WatchEvent event, - PyDictObject *mp, - PyObject *key, - PyObject *value) -{ - // Most dicts will not be watched. - if (UNLIKELY(dict_is_watched(mp))) { - PyDict_WatchCallback callback = PyDict_GetWatchCallback(); - // If a dict is watched, it is very likely someone installed a callback. - if (LIKELY(callback != NULL)) { - callback(event, (PyObject*)mp, key, value); - } - return DICT_NEXT_VERSION() | DICT_VERSION_WATCHED_TAG; - } - return DICT_NEXT_VERSION(); -} - /* Print summary info about the state of the optimized allocator */ void _PyDict_DebugMallocStats(FILE *out) @@ -1278,7 +1237,7 @@ insertdict(PyDictObject *mp, PyObject *key, Py_hash_t hash, PyObject *value) MAINTAIN_TRACKING(mp, key, value); if (ix == DKIX_EMPTY) { - uint64_t new_version = notify_dict_event(PyDict_EVENT_ADDED, mp, key, value); + uint64_t new_version = _PyDict_NotifyEvent(PyDict_EVENT_ADDED, mp, key, value); /* Insert into new slot. */ mp->ma_keys->dk_version = 0; assert(old_value == NULL); @@ -1322,7 +1281,7 @@ insertdict(PyDictObject *mp, PyObject *key, Py_hash_t hash, PyObject *value) } if (old_value != value) { - uint64_t new_version = notify_dict_event(PyDict_EVENT_MODIFIED, mp, key, value); + uint64_t new_version = _PyDict_NotifyEvent(PyDict_EVENT_MODIFIED, mp, key, value); if (_PyDict_HasSplitTable(mp)) { mp->ma_values->values[ix] = value; if (old_value == NULL) { @@ -1360,7 +1319,7 @@ insert_to_emptydict(PyDictObject *mp, PyObject *key, Py_hash_t hash, { assert(mp->ma_keys == Py_EMPTY_KEYS); - uint64_t new_version = notify_dict_event(PyDict_EVENT_ADDED, mp, key, value); + uint64_t new_version = _PyDict_NotifyEvent(PyDict_EVENT_ADDED, mp, key, value); int unicode = PyUnicode_CheckExact(key); PyDictKeysObject *newkeys = new_keys_object(PyDict_LOG_MINSIZE, unicode); @@ -2066,7 +2025,7 @@ _PyDict_DelItem_KnownHash(PyObject *op, PyObject *key, Py_hash_t hash) return -1; } - uint64_t new_version = notify_dict_event(PyDict_EVENT_DELETED, mp, key, NULL); + uint64_t new_version = _PyDict_NotifyEvent(PyDict_EVENT_DELETED, mp, key, NULL); return delitem_common(mp, hash, ix, old_value, new_version); } @@ -2109,7 +2068,7 @@ _PyDict_DelItemIf(PyObject *op, PyObject *key, assert(hashpos >= 0); if (res > 0) { - uint64_t new_version = notify_dict_event(PyDict_EVENT_DELETED, mp, key, NULL); + uint64_t new_version = _PyDict_NotifyEvent(PyDict_EVENT_DELETED, mp, key, NULL); return delitem_common(mp, hashpos, ix, old_value, new_version); } else { return 0; @@ -2134,7 +2093,7 @@ PyDict_Clear(PyObject *op) return; } /* Empty the dict... */ - uint64_t new_version = notify_dict_event(PyDict_EVENT_CLEARED, mp, NULL, NULL); + uint64_t new_version = _PyDict_NotifyEvent(PyDict_EVENT_CLEARED, mp, NULL, NULL); dictkeys_incref(Py_EMPTY_KEYS); mp->ma_keys = Py_EMPTY_KEYS; mp->ma_values = NULL; @@ -2279,7 +2238,7 @@ _PyDict_Pop_KnownHash(PyObject *dict, PyObject *key, Py_hash_t hash, PyObject *d } assert(old_value != NULL); Py_INCREF(old_value); - uint64_t new_version = notify_dict_event(PyDict_EVENT_DELETED, mp, key, NULL); + uint64_t new_version = _PyDict_NotifyEvent(PyDict_EVENT_DELETED, mp, key, NULL); delitem_common(mp, hash, ix, old_value, new_version); ASSERT_CONSISTENT(mp); @@ -2405,7 +2364,7 @@ _PyDict_FromKeys(PyObject *cls, PyObject *iterable, PyObject *value) static void dict_dealloc(PyDictObject *mp) { - notify_dict_event(PyDict_EVENT_DEALLOCED, mp, NULL, NULL); + _PyDict_NotifyEvent(PyDict_EVENT_DEALLOCED, mp, NULL, NULL); PyDictValues *values = mp->ma_values; PyDictKeysObject *keys = mp->ma_keys; Py_ssize_t i, n; @@ -2893,7 +2852,7 @@ dict_merge(PyObject *a, PyObject *b, int override) other->ma_used == okeys->dk_nentries && (DK_LOG_SIZE(okeys) == PyDict_LOG_MINSIZE || USABLE_FRACTION(DK_SIZE(okeys)/2) < other->ma_used)) { - uint64_t new_version = notify_dict_event(PyDict_EVENT_CLONED, mp, b, NULL); + uint64_t new_version = _PyDict_NotifyEvent(PyDict_EVENT_CLONED, mp, b, NULL); PyDictKeysObject *keys = clone_combined_dict_keys(other); if (keys == NULL) { return -1; @@ -3379,7 +3338,7 @@ PyDict_SetDefault(PyObject *d, PyObject *key, PyObject *defaultobj) return NULL; if (ix == DKIX_EMPTY) { - uint64_t new_version = notify_dict_event(PyDict_EVENT_ADDED, mp, key, defaultobj); + uint64_t new_version = _PyDict_NotifyEvent(PyDict_EVENT_ADDED, mp, key, defaultobj); mp->ma_keys->dk_version = 0; value = defaultobj; if (mp->ma_keys->dk_usable <= 0) { @@ -3420,7 +3379,7 @@ PyDict_SetDefault(PyObject *d, PyObject *key, PyObject *defaultobj) assert(mp->ma_keys->dk_usable >= 0); } else if (value == NULL) { - uint64_t new_version = notify_dict_event(PyDict_EVENT_ADDED, mp, key, defaultobj); + uint64_t new_version = _PyDict_NotifyEvent(PyDict_EVENT_ADDED, mp, key, defaultobj); value = defaultobj; assert(_PyDict_HasSplitTable(mp)); assert(mp->ma_values->values[ix] == NULL); @@ -3542,7 +3501,7 @@ dict_popitem_impl(PyDictObject *self) assert(i >= 0); key = ep0[i].me_key; - new_version = notify_dict_event(PyDict_EVENT_DELETED, self, key, NULL); + new_version = _PyDict_NotifyEvent(PyDict_EVENT_DELETED, self, key, NULL); hash = unicode_get_hash(key); value = ep0[i].me_value; ep0[i].me_key = NULL; @@ -3557,7 +3516,7 @@ dict_popitem_impl(PyDictObject *self) assert(i >= 0); key = ep0[i].me_key; - new_version = notify_dict_event(PyDict_EVENT_DELETED, self, key, NULL); + new_version = _PyDict_NotifyEvent(PyDict_EVENT_DELETED, self, key, NULL); hash = ep0[i].me_hash; value = ep0[i].me_value; ep0[i].me_key = NULL; @@ -5744,28 +5703,55 @@ uint32_t _PyDictKeys_GetVersionForCurrentState(PyDictKeysObject *dictkeys) } int -PyDict_Watch(PyObject* dict) +PyDict_Watch(int watcher_id, PyObject* dict) { if (!PyDict_Check(dict)) { - PyErr_BadInternalCall(); + PyErr_SetString(PyExc_ValueError, "Cannot watch non-dictionary"); return -1; } - ((PyDictObject*)dict)->ma_version_tag |= DICT_VERSION_WATCHED_TAG; + if (watcher_id < 0 || watcher_id >= DICT_MAX_WATCHERS) { + PyErr_Format(PyExc_ValueError, "Invalid dict watcher ID %d", watcher_id); + return -1; + } + PyInterpreterState *interp = _PyInterpreterState_GET(); + if (!interp->dict_watchers[watcher_id]) { + PyErr_Format(PyExc_ValueError, "No dict watcher set for ID %d", watcher_id); + return -1; + } + ((PyDictObject*)dict)->ma_version_tag |= (1 << watcher_id); return 0; } int -PyDict_IsWatched(PyObject* dict) +PyDict_AddWatcher(PyDict_WatchCallback callback) { - if (PyDict_Check(dict)) { - return dict_is_watched((PyDictObject*)dict); + PyInterpreterState *interp = _PyInterpreterState_GET(); + + for (int i = 0; i < DICT_MAX_WATCHERS; i++) { + if (!interp->dict_watchers[i]) { + interp->dict_watchers[i] = (void*)callback; + return i; + } } - return 0; + + PyErr_SetString(PyExc_RuntimeError, "no more dict watcher IDs available"); + return -1; } void -PyDict_SetWatchCallback(PyDict_WatchCallback callback) +_PyDict_SendEvent(int watcher_bits, + PyDict_WatchEvent event, + PyDictObject *mp, + PyObject *key, + PyObject *value) { - PyThreadState *tstate = PyThreadState_GET(); - tstate->interp->dict_watch_callback = (void*)callback; + PyInterpreterState *interp = _PyInterpreterState_GET(); + for (int i = 0; i < DICT_MAX_WATCHERS; i++) { + if (watcher_bits & (1 << i)) { + PyDict_WatchCallback cb = (PyDict_WatchCallback)interp->dict_watchers[i]; + if (cb) { + cb(event, (PyObject*)mp, key, value); + } + } + } } diff --git a/Python/ceval.c b/Python/ceval.c index 25ad87b09c2ddb3..0f6fe37586ae73d 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -722,36 +722,6 @@ Py_MakePendingCalls(void) return 0; } -/* Dict watchers -- TODO copied from dictobject.c, remove if ceval stops - directly fiddling with dict internals. */ - -/* If we're using GCC, use __builtin_expect() to reduce overhead of - the watched-dict checks */ -#if defined(__GNUC__) && (__GNUC__ > 2) && defined(__OPTIMIZE__) -# define UNLIKELY(value) __builtin_expect((value), 0) -# define LIKELY(value) __builtin_expect((value), 1) -#else -# define UNLIKELY(value) (value) -# define LIKELY(value) (value) -#endif - -static inline uint64_t -notify_dict_event(PyDict_WatchEvent event, - PyDictObject *mp, - PyObject *key, - PyObject *value) -{ - // Most dicts will not be watched. - if (UNLIKELY(mp->ma_version_tag & DICT_VERSION_WATCHED_TAG)) { - PyDict_WatchCallback callback = PyDict_GetWatchCallback(); - // If a dict is watched, it is very likely someone installed a callback. - if (LIKELY(callback != NULL)) { - callback(event, (PyObject*)mp, key, value); - } - return DICT_NEXT_VERSION() | DICT_VERSION_WATCHED_TAG; - } - return DICT_NEXT_VERSION(); -} /* The interpreter's recursion limit */ @@ -3639,7 +3609,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int DEOPT_IF(old_value == NULL, STORE_ATTR); STACK_SHRINK(1); value = POP(); - new_version = notify_dict_event(PyDict_EVENT_MODIFIED, dict, name, value); + new_version = _PyDict_NotifyEvent(PyDict_EVENT_MODIFIED, dict, name, value); ep->me_value = value; } else { @@ -3649,7 +3619,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int DEOPT_IF(old_value == NULL, STORE_ATTR); STACK_SHRINK(1); value = POP(); - new_version = notify_dict_event(PyDict_EVENT_MODIFIED, dict, name, value); + new_version = _PyDict_NotifyEvent(PyDict_EVENT_MODIFIED, dict, name, value); ep->me_value = value; } Py_DECREF(old_value);