From 3108bbe38ff126100024addfdb9b7dd5587e5e18 Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Mon, 7 Jul 2025 14:53:33 +0530 Subject: [PATCH 1/9] use stackrefs for dict loopkup --- Objects/object.c | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/Objects/object.c b/Objects/object.c index 3ed7d55593dffa..262b95848a2fca 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -1753,19 +1753,29 @@ _PyObject_GetMethodStackRef(PyThreadState *ts, PyObject *obj, } } if (dict != NULL) { - // TODO: use _Py_dict_lookup_threadsafe_stackref + assert(PyUnicode_CheckExact(name)); + Py_hash_t hash = _PyObject_HashFast(name); + // cannot fail for exact unicode + assert(hash != -1); Py_INCREF(dict); - PyObject *value; - if (PyDict_GetItemRef(dict, name, &value) != 0) { - // found or error - Py_DECREF(dict); + // ref is not visible to gc so there should be + // no escaping calls before assigning it to method + _PyStackRef ref; + Py_ssize_t ix = _Py_dict_lookup_threadsafe_stackref((PyDictObject *)dict, + name, hash, &ref); + if (ix == DKIX_ERROR) { + // error PyStackRef_CLEAR(*method); - if (value != NULL) { - *method = PyStackRef_FromPyObjectSteal(value); - } + Py_DECREF(dict); + return -1; + } else if (!PyStackRef_IsNull(ref)) { + // found + _PyStackRef tmp = *method; + *method = ref; + PyStackRef_XCLOSE(tmp); + Py_DECREF(dict); return 0; } - // not found Py_DECREF(dict); } From a5498aaf01431960d10c6534b93267f79a1086c7 Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Tue, 8 Jul 2025 17:12:35 +0530 Subject: [PATCH 2/9] add back comment --- Objects/object.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Objects/object.c b/Objects/object.c index 262b95848a2fca..e30fd8c0d766eb 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -1776,6 +1776,7 @@ _PyObject_GetMethodStackRef(PyThreadState *ts, PyObject *obj, Py_DECREF(dict); return 0; } + // not found Py_DECREF(dict); } From e4c36a8c23733bb9d461bfc3a8c5b64a3d5c3db2 Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Wed, 9 Jul 2025 18:14:40 +0530 Subject: [PATCH 3/9] avoid incref/decref for unicode keys dict --- Objects/object.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/Objects/object.c b/Objects/object.c index e30fd8c0d766eb..9b39350289544a 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -1757,27 +1757,30 @@ _PyObject_GetMethodStackRef(PyThreadState *ts, PyObject *obj, Py_hash_t hash = _PyObject_HashFast(name); // cannot fail for exact unicode assert(hash != -1); - Py_INCREF(dict); // ref is not visible to gc so there should be // no escaping calls before assigning it to method + PyDictObject *mp = (PyDictObject *)dict; + bool unicode_keys = DK_IS_UNICODE(FT_ATOMIC_LOAD_PTR_ACQUIRE(mp->ma_keys)); _PyStackRef ref; - Py_ssize_t ix = _Py_dict_lookup_threadsafe_stackref((PyDictObject *)dict, - name, hash, &ref); + if (!unicode_keys) { + Py_INCREF(mp); + } + Py_ssize_t ix = _Py_dict_lookup_threadsafe_stackref(mp, name, + hash, &ref); + if (!unicode_keys) { + Py_DECREF(mp); + } if (ix == DKIX_ERROR) { // error PyStackRef_CLEAR(*method); - Py_DECREF(dict); return -1; } else if (!PyStackRef_IsNull(ref)) { // found _PyStackRef tmp = *method; *method = ref; PyStackRef_XCLOSE(tmp); - Py_DECREF(dict); return 0; } - // not found - Py_DECREF(dict); } if (meth_found) { From c889427b6b9b3b107a079864a2aa3d3b0c541634 Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Wed, 9 Jul 2025 18:14:59 +0530 Subject: [PATCH 4/9] format --- Objects/object.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Objects/object.c b/Objects/object.c index 9b39350289544a..0bcb090bcc654a 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -1774,7 +1774,8 @@ _PyObject_GetMethodStackRef(PyThreadState *ts, PyObject *obj, // error PyStackRef_CLEAR(*method); return -1; - } else if (!PyStackRef_IsNull(ref)) { + } + else if (!PyStackRef_IsNull(ref)) { // found _PyStackRef tmp = *method; *method = ref; From 6468c7cf9611906d8c5ba75fad1db6f3b4d36738 Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Wed, 9 Jul 2025 18:31:43 +0530 Subject: [PATCH 5/9] fix compliation --- Objects/object.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Objects/object.c b/Objects/object.c index 0bcb090bcc654a..a5eb287c02662f 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -1760,7 +1760,8 @@ _PyObject_GetMethodStackRef(PyThreadState *ts, PyObject *obj, // ref is not visible to gc so there should be // no escaping calls before assigning it to method PyDictObject *mp = (PyDictObject *)dict; - bool unicode_keys = DK_IS_UNICODE(FT_ATOMIC_LOAD_PTR_ACQUIRE(mp->ma_keys)); + PyDictKeysObject *keys = FT_ATOMIC_LOAD_PTR_ACQUIRE(mp->ma_keys); + bool unicode_keys = DK_IS_UNICODE(keys); _PyStackRef ref; if (!unicode_keys) { Py_INCREF(mp); From aadb4826da39f56d248c9021705a28a6fe318ee0 Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Fri, 18 Jul 2025 16:02:37 +0530 Subject: [PATCH 6/9] add _Py_dict_lookup_unicode_threadsafe_stackref to lookup method --- Include/internal/pycore_dict.h | 1 + Objects/dictobject.c | 94 ++++++++++++++++++++++++++-------- Objects/object.c | 13 +---- 3 files changed, 76 insertions(+), 32 deletions(-) diff --git a/Include/internal/pycore_dict.h b/Include/internal/pycore_dict.h index 25bb224921aa8b..251ed5c51ab2fc 100644 --- a/Include/internal/pycore_dict.h +++ b/Include/internal/pycore_dict.h @@ -112,6 +112,7 @@ extern void _PyDictKeys_DecRef(PyDictKeysObject *keys); extern Py_ssize_t _Py_dict_lookup(PyDictObject *mp, PyObject *key, Py_hash_t hash, PyObject **value_addr); extern Py_ssize_t _Py_dict_lookup_threadsafe(PyDictObject *mp, PyObject *key, Py_hash_t hash, PyObject **value_addr); extern Py_ssize_t _Py_dict_lookup_threadsafe_stackref(PyDictObject *mp, PyObject *key, Py_hash_t hash, _PyStackRef *value_addr); +extern Py_ssize_t _Py_dict_lookup_unicode_threadsafe_stackref(PyDictObject *mp, PyObject *key, Py_hash_t hash, _PyStackRef *value_addr); extern Py_ssize_t _PyDict_LookupIndex(PyDictObject *, PyObject *); extern Py_ssize_t _PyDictKeys_StringLookup(PyDictKeysObject* dictkeys, PyObject *key); diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 0ed52ac5e87b6e..4462ad57b70df7 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -1581,33 +1581,41 @@ _Py_dict_lookup_threadsafe(PyDictObject *mp, PyObject *key, Py_hash_t hash, PyOb return ix; } -Py_ssize_t -_Py_dict_lookup_threadsafe_stackref(PyDictObject *mp, PyObject *key, Py_hash_t hash, _PyStackRef *value_addr) +static Py_ssize_t +unicodekeys_lookup_unicode_threadsafe_stackref(PyDictKeysObject* dk, PyObject *key, Py_hash_t hash, _PyStackRef *value_addr) { - PyDictKeysObject *dk = _Py_atomic_load_ptr(&mp->ma_keys); - if (dk->dk_kind == DICT_KEYS_UNICODE && PyUnicode_CheckExact(key)) { - Py_ssize_t ix = unicodekeys_lookup_unicode_threadsafe(dk, key, hash); - if (ix == DKIX_EMPTY) { + assert(PyUnicode_CheckExact(key)); + assert(dk->dk_kind == DICT_KEYS_UNICODE); + Py_ssize_t ix = unicodekeys_lookup_unicode_threadsafe(dk, key, hash); + if (ix == DKIX_EMPTY) { + *value_addr = PyStackRef_NULL; + return DKIX_EMPTY; + } else if (ix >= 0) { + PyObject **addr_of_value = &DK_UNICODE_ENTRIES(dk)[ix].me_value; + PyObject *value = _Py_atomic_load_ptr(addr_of_value); + if (value == NULL) { *value_addr = PyStackRef_NULL; + return DKIX_EMPTY; + } + if (_PyObject_HasDeferredRefcount(value)) { + *value_addr = (_PyStackRef){ .bits = (uintptr_t)value | Py_TAG_DEFERRED }; return ix; } - else if (ix >= 0) { - PyObject **addr_of_value = &DK_UNICODE_ENTRIES(dk)[ix].me_value; - PyObject *value = _Py_atomic_load_ptr(addr_of_value); - if (value == NULL) { - *value_addr = PyStackRef_NULL; - return DKIX_EMPTY; - } - if (_PyObject_HasDeferredRefcount(value)) { - *value_addr = (_PyStackRef){ .bits = (uintptr_t)value | Py_TAG_DEFERRED }; - return ix; - } - if (_Py_TryIncrefCompare(addr_of_value, value)) { - *value_addr = PyStackRef_FromPyObjectSteal(value); - return ix; - } + if (_Py_TryIncrefCompare(addr_of_value, value)) { + *value_addr = PyStackRef_FromPyObjectSteal(value); + return ix; } } +} + + +Py_ssize_t +_Py_dict_lookup_threadsafe_stackref(PyDictObject *mp, PyObject *key, Py_hash_t hash, _PyStackRef *value_addr) +{ + PyDictKeysObject *dk = _Py_atomic_load_ptr(&mp->ma_keys); + if (dk->dk_kind == DICT_KEYS_UNICODE && PyUnicode_CheckExact(key)) { + return unicodekeys_lookup_unicode_threadsafe_stackref(dk, key, hash, value_addr); + } PyObject *obj; Py_ssize_t ix = _Py_dict_lookup_threadsafe(mp, key, hash, &obj); @@ -1620,6 +1628,33 @@ _Py_dict_lookup_threadsafe_stackref(PyDictObject *mp, PyObject *key, Py_hash_t h return ix; } +// This is similar to _Py_dict_lookup_threadsafe_stackref() but +// it is used when dict is borrowed reference and key is known to be unicode. +// It avoids increfing the dict if dict only has unicode keys in which case +// the lookup is safe, otherwise it increfs the dict and lookups the key. + +Py_ssize_t +_Py_dict_lookup_unicode_threadsafe_stackref(PyDictObject *mp, PyObject *key, Py_hash_t hash, _PyStackRef *value_addr) +{ + assert(PyUnicode_CheckExact(key)); + PyDictKeysObject *dk = _Py_atomic_load_ptr(&mp->ma_keys); + if (dk->dk_kind == DICT_KEYS_UNICODE) { + return unicodekeys_lookup_unicode_threadsafe_stackref(dk, key, hash, value_addr); + } + + PyObject *obj; + Py_INCREF(mp); + Py_ssize_t ix = _Py_dict_lookup_threadsafe(mp, key, hash, &obj); + Py_DECREF(mp); + if (ix >= 0 && obj != NULL) { + *value_addr = PyStackRef_FromPyObjectSteal(obj); + } + else { + *value_addr = PyStackRef_NULL; + } + return ix; +} + #else // Py_GIL_DISABLED Py_ssize_t @@ -1644,6 +1679,23 @@ _Py_dict_lookup_threadsafe_stackref(PyDictObject *mp, PyObject *key, Py_hash_t h return ix; } +Py_ssize_t +_Py_dict_lookup_unicode_threadsafe_stackref(PyDictObject *mp, PyObject *key, Py_hash_t hash, _PyStackRef *value_addr) +{ + assert(PyUnicode_CheckExact(key)); + PyObject *val; + Py_INCREF(mp); + Py_ssize_t ix = _Py_dict_lookup(mp, key, hash, &val); + Py_DECREF(mp); + if (ix >= 0 && val != NULL) { + *value_addr = PyStackRef_FromPyObjectNew(val); + } + else { + *value_addr = PyStackRef_NULL; + } + return ix; +} + #endif int diff --git a/Objects/object.c b/Objects/object.c index a5eb287c02662f..c4ac9217f1b43f 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -1759,18 +1759,9 @@ _PyObject_GetMethodStackRef(PyThreadState *ts, PyObject *obj, assert(hash != -1); // ref is not visible to gc so there should be // no escaping calls before assigning it to method - PyDictObject *mp = (PyDictObject *)dict; - PyDictKeysObject *keys = FT_ATOMIC_LOAD_PTR_ACQUIRE(mp->ma_keys); - bool unicode_keys = DK_IS_UNICODE(keys); _PyStackRef ref; - if (!unicode_keys) { - Py_INCREF(mp); - } - Py_ssize_t ix = _Py_dict_lookup_threadsafe_stackref(mp, name, - hash, &ref); - if (!unicode_keys) { - Py_DECREF(mp); - } + Py_ssize_t ix = _Py_dict_lookup_unicode_threadsafe_stackref((PyDictObject *)dict, + name, hash, &ref); if (ix == DKIX_ERROR) { // error PyStackRef_CLEAR(*method); From 5e79d523986909f2b8dc60289aa60018810062ce Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Fri, 18 Jul 2025 16:04:26 +0530 Subject: [PATCH 7/9] fix merge --- Objects/dictobject.c | 71 ++++++++++++++++++++++++++------------------ 1 file changed, 42 insertions(+), 29 deletions(-) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 4462ad57b70df7..cd64cc97bc5f35 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -1581,40 +1581,33 @@ _Py_dict_lookup_threadsafe(PyDictObject *mp, PyObject *key, Py_hash_t hash, PyOb return ix; } -static Py_ssize_t -unicodekeys_lookup_unicode_threadsafe_stackref(PyDictKeysObject* dk, PyObject *key, Py_hash_t hash, _PyStackRef *value_addr) -{ - assert(PyUnicode_CheckExact(key)); - assert(dk->dk_kind == DICT_KEYS_UNICODE); - Py_ssize_t ix = unicodekeys_lookup_unicode_threadsafe(dk, key, hash); - if (ix == DKIX_EMPTY) { - *value_addr = PyStackRef_NULL; - return DKIX_EMPTY; - } else if (ix >= 0) { - PyObject **addr_of_value = &DK_UNICODE_ENTRIES(dk)[ix].me_value; - PyObject *value = _Py_atomic_load_ptr(addr_of_value); - if (value == NULL) { - *value_addr = PyStackRef_NULL; - return DKIX_EMPTY; - } - if (_PyObject_HasDeferredRefcount(value)) { - *value_addr = (_PyStackRef){ .bits = (uintptr_t)value | Py_TAG_DEFERRED }; - return ix; - } - if (_Py_TryIncrefCompare(addr_of_value, value)) { - *value_addr = PyStackRef_FromPyObjectSteal(value); - return ix; - } - } -} - Py_ssize_t _Py_dict_lookup_threadsafe_stackref(PyDictObject *mp, PyObject *key, Py_hash_t hash, _PyStackRef *value_addr) { PyDictKeysObject *dk = _Py_atomic_load_ptr(&mp->ma_keys); if (dk->dk_kind == DICT_KEYS_UNICODE && PyUnicode_CheckExact(key)) { - return unicodekeys_lookup_unicode_threadsafe_stackref(dk, key, hash, value_addr); + Py_ssize_t ix = unicodekeys_lookup_unicode_threadsafe(dk, key, hash); + if (ix == DKIX_EMPTY) { + *value_addr = PyStackRef_NULL; + return ix; + } + else if (ix >= 0) { + PyObject **addr_of_value = &DK_UNICODE_ENTRIES(dk)[ix].me_value; + PyObject *value = _Py_atomic_load_ptr(addr_of_value); + if (value == NULL) { + *value_addr = PyStackRef_NULL; + return DKIX_EMPTY; + } + if (_PyObject_HasDeferredRefcount(value)) { + *value_addr = (_PyStackRef){ .bits = (uintptr_t)value | Py_TAG_DEFERRED }; + return ix; + } + if (_Py_TryIncrefCompare(addr_of_value, value)) { + *value_addr = PyStackRef_FromPyObjectSteal(value); + return ix; + } + } } PyObject *obj; @@ -1639,7 +1632,27 @@ _Py_dict_lookup_unicode_threadsafe_stackref(PyDictObject *mp, PyObject *key, Py_ assert(PyUnicode_CheckExact(key)); PyDictKeysObject *dk = _Py_atomic_load_ptr(&mp->ma_keys); if (dk->dk_kind == DICT_KEYS_UNICODE) { - return unicodekeys_lookup_unicode_threadsafe_stackref(dk, key, hash, value_addr); + Py_ssize_t ix = unicodekeys_lookup_unicode_threadsafe(dk, key, hash); + if (ix == DKIX_EMPTY) { + *value_addr = PyStackRef_NULL; + return ix; + } + else if (ix >= 0) { + PyObject **addr_of_value = &DK_UNICODE_ENTRIES(dk)[ix].me_value; + PyObject *value = _Py_atomic_load_ptr(addr_of_value); + if (value == NULL) { + *value_addr = PyStackRef_NULL; + return DKIX_EMPTY; + } + if (_PyObject_HasDeferredRefcount(value)) { + *value_addr = (_PyStackRef){ .bits = (uintptr_t)value | Py_TAG_DEFERRED }; + return ix; + } + if (_Py_TryIncrefCompare(addr_of_value, value)) { + *value_addr = PyStackRef_FromPyObjectSteal(value); + return ix; + } + } } PyObject *obj; From bbc7fb836f0d2349a44aff4bed85d0b377bcecef Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Mon, 21 Jul 2025 18:43:00 +0000 Subject: [PATCH 8/9] Refactor --- Include/internal/pycore_dict.h | 3 +- Include/internal/pycore_stackref.h | 7 ++ Objects/dictobject.c | 132 +++++++++++++---------------- Objects/object.c | 20 +---- 4 files changed, 74 insertions(+), 88 deletions(-) diff --git a/Include/internal/pycore_dict.h b/Include/internal/pycore_dict.h index 251ed5c51ab2fc..6ab569393e5ce1 100644 --- a/Include/internal/pycore_dict.h +++ b/Include/internal/pycore_dict.h @@ -112,7 +112,8 @@ extern void _PyDictKeys_DecRef(PyDictKeysObject *keys); extern Py_ssize_t _Py_dict_lookup(PyDictObject *mp, PyObject *key, Py_hash_t hash, PyObject **value_addr); extern Py_ssize_t _Py_dict_lookup_threadsafe(PyDictObject *mp, PyObject *key, Py_hash_t hash, PyObject **value_addr); extern Py_ssize_t _Py_dict_lookup_threadsafe_stackref(PyDictObject *mp, PyObject *key, Py_hash_t hash, _PyStackRef *value_addr); -extern Py_ssize_t _Py_dict_lookup_unicode_threadsafe_stackref(PyDictObject *mp, PyObject *key, Py_hash_t hash, _PyStackRef *value_addr); + +extern int _PyDict_GetMethodStackRef(PyDictObject *dict, PyObject *name, _PyStackRef *method); extern Py_ssize_t _PyDict_LookupIndex(PyDictObject *, PyObject *); extern Py_ssize_t _PyDictKeys_StringLookup(PyDictKeysObject* dictkeys, PyObject *key); diff --git a/Include/internal/pycore_stackref.h b/Include/internal/pycore_stackref.h index 6bf82d8322f508..c4e8f10fe05276 100644 --- a/Include/internal/pycore_stackref.h +++ b/Include/internal/pycore_stackref.h @@ -839,6 +839,13 @@ _Py_TryXGetStackRef(PyObject **src, _PyStackRef *out) #endif +#define PyStackRef_XSETREF(dst, src) \ + do { \ + _PyStackRef _tmp_dst_ref = (dst); \ + (dst) = (src); \ + PyStackRef_XCLOSE(_tmp_dst_ref); \ + } while(0) + // Like Py_VISIT but for _PyStackRef fields #define _Py_VISIT_STACKREF(ref) \ do { \ diff --git a/Objects/dictobject.c b/Objects/dictobject.c index cd64cc97bc5f35..9e537f095c332c 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -1581,84 +1581,51 @@ _Py_dict_lookup_threadsafe(PyDictObject *mp, PyObject *key, Py_hash_t hash, PyOb return ix; } - -Py_ssize_t -_Py_dict_lookup_threadsafe_stackref(PyDictObject *mp, PyObject *key, Py_hash_t hash, _PyStackRef *value_addr) +static Py_ssize_t +lookup_threadsafe_unicode(PyDictKeysObject *dk, PyObject *key, Py_hash_t hash, _PyStackRef *value_addr) { - PyDictKeysObject *dk = _Py_atomic_load_ptr(&mp->ma_keys); - if (dk->dk_kind == DICT_KEYS_UNICODE && PyUnicode_CheckExact(key)) { - Py_ssize_t ix = unicodekeys_lookup_unicode_threadsafe(dk, key, hash); - if (ix == DKIX_EMPTY) { + assert(dk->dk_kind == DICT_KEYS_UNICODE); + assert(PyUnicode_CheckExact(key)); + + Py_ssize_t ix = unicodekeys_lookup_unicode_threadsafe(dk, key, hash); + if (ix == DKIX_EMPTY) { + *value_addr = PyStackRef_NULL; + return ix; + } + else if (ix >= 0) { + PyObject **addr_of_value = &DK_UNICODE_ENTRIES(dk)[ix].me_value; + PyObject *value = _Py_atomic_load_ptr(addr_of_value); + if (value == NULL) { *value_addr = PyStackRef_NULL; + return DKIX_EMPTY; + } + if (_PyObject_HasDeferredRefcount(value)) { + *value_addr = (_PyStackRef){ .bits = (uintptr_t)value | Py_TAG_DEFERRED }; return ix; } - else if (ix >= 0) { - PyObject **addr_of_value = &DK_UNICODE_ENTRIES(dk)[ix].me_value; - PyObject *value = _Py_atomic_load_ptr(addr_of_value); - if (value == NULL) { - *value_addr = PyStackRef_NULL; - return DKIX_EMPTY; - } - if (_PyObject_HasDeferredRefcount(value)) { - *value_addr = (_PyStackRef){ .bits = (uintptr_t)value | Py_TAG_DEFERRED }; - return ix; - } - if (_Py_TryIncrefCompare(addr_of_value, value)) { - *value_addr = PyStackRef_FromPyObjectSteal(value); - return ix; - } + if (_Py_TryIncrefCompare(addr_of_value, value)) { + *value_addr = PyStackRef_FromPyObjectSteal(value); + return ix; } + return DKIX_KEY_CHANGED; } - - PyObject *obj; - Py_ssize_t ix = _Py_dict_lookup_threadsafe(mp, key, hash, &obj); - if (ix >= 0 && obj != NULL) { - *value_addr = PyStackRef_FromPyObjectSteal(obj); - } - else { - *value_addr = PyStackRef_NULL; - } + assert(ix == DKIX_KEY_CHANGED); return ix; } -// This is similar to _Py_dict_lookup_threadsafe_stackref() but -// it is used when dict is borrowed reference and key is known to be unicode. -// It avoids increfing the dict if dict only has unicode keys in which case -// the lookup is safe, otherwise it increfs the dict and lookups the key. - Py_ssize_t -_Py_dict_lookup_unicode_threadsafe_stackref(PyDictObject *mp, PyObject *key, Py_hash_t hash, _PyStackRef *value_addr) +_Py_dict_lookup_threadsafe_stackref(PyDictObject *mp, PyObject *key, Py_hash_t hash, _PyStackRef *value_addr) { - assert(PyUnicode_CheckExact(key)); PyDictKeysObject *dk = _Py_atomic_load_ptr(&mp->ma_keys); - if (dk->dk_kind == DICT_KEYS_UNICODE) { - Py_ssize_t ix = unicodekeys_lookup_unicode_threadsafe(dk, key, hash); - if (ix == DKIX_EMPTY) { - *value_addr = PyStackRef_NULL; + if (dk->dk_kind == DICT_KEYS_UNICODE && PyUnicode_CheckExact(key)) { + Py_ssize_t ix = lookup_threadsafe_unicode(dk, key, hash, value_addr); + if (ix != DKIX_KEY_CHANGED) { return ix; } - else if (ix >= 0) { - PyObject **addr_of_value = &DK_UNICODE_ENTRIES(dk)[ix].me_value; - PyObject *value = _Py_atomic_load_ptr(addr_of_value); - if (value == NULL) { - *value_addr = PyStackRef_NULL; - return DKIX_EMPTY; - } - if (_PyObject_HasDeferredRefcount(value)) { - *value_addr = (_PyStackRef){ .bits = (uintptr_t)value | Py_TAG_DEFERRED }; - return ix; - } - if (_Py_TryIncrefCompare(addr_of_value, value)) { - *value_addr = PyStackRef_FromPyObjectSteal(value); - return ix; - } - } } PyObject *obj; - Py_INCREF(mp); Py_ssize_t ix = _Py_dict_lookup_threadsafe(mp, key, hash, &obj); - Py_DECREF(mp); if (ix >= 0 && obj != NULL) { *value_addr = PyStackRef_FromPyObjectSteal(obj); } @@ -1692,25 +1659,48 @@ _Py_dict_lookup_threadsafe_stackref(PyDictObject *mp, PyObject *key, Py_hash_t h return ix; } -Py_ssize_t -_Py_dict_lookup_unicode_threadsafe_stackref(PyDictObject *mp, PyObject *key, Py_hash_t hash, _PyStackRef *value_addr) +#endif + +// Looks up the unicode key `key` in the dictionary. Note that `*method` may +// already contain a valid value! See _PyObject_GetMethodStackRef(). +int +_PyDict_GetMethodStackRef(PyDictObject *mp, PyObject *key, _PyStackRef *method) { assert(PyUnicode_CheckExact(key)); - PyObject *val; + Py_hash_t hash = hash_unicode_key(key); + +#ifdef Py_GIL_DISABLED + PyDictKeysObject *dk = _Py_atomic_load_ptr_acquire(&mp->ma_keys); + if (dk->dk_kind == DICT_KEYS_UNICODE) { + _PyStackRef ref; // + Py_ssize_t ix = lookup_threadsafe_unicode(dk, key, hash, &ref); + if (ix >= 0) { + assert(!PyStackRef_IsNull(ref)); + PyStackRef_XSETREF(*method, ref); + return 1; + } + else if (ix == DKIX_EMPTY) { + return 0; + } + assert(ix == DKIX_KEY_CHANGED); + } +#endif + + PyObject *obj; Py_INCREF(mp); - Py_ssize_t ix = _Py_dict_lookup(mp, key, hash, &val); + Py_ssize_t ix = _Py_dict_lookup_threadsafe(mp, key, hash, &obj); Py_DECREF(mp); - if (ix >= 0 && val != NULL) { - *value_addr = PyStackRef_FromPyObjectNew(val); + if (ix == DKIX_ERROR) { + PyStackRef_CLEAR(*method); + return -1; } - else { - *value_addr = PyStackRef_NULL; + else if (ix >= 0 && obj != NULL) { + PyStackRef_XSETREF(*method, PyStackRef_FromPyObjectSteal(obj)); + return 1; } - return ix; + return 0; // not found } -#endif - int _PyDict_HasOnlyStringKeys(PyObject *dict) { diff --git a/Objects/object.c b/Objects/object.c index c4ac9217f1b43f..66996051973c91 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -1754,24 +1754,12 @@ _PyObject_GetMethodStackRef(PyThreadState *ts, PyObject *obj, } if (dict != NULL) { assert(PyUnicode_CheckExact(name)); - Py_hash_t hash = _PyObject_HashFast(name); - // cannot fail for exact unicode - assert(hash != -1); - // ref is not visible to gc so there should be - // no escaping calls before assigning it to method - _PyStackRef ref; - Py_ssize_t ix = _Py_dict_lookup_unicode_threadsafe_stackref((PyDictObject *)dict, - name, hash, &ref); - if (ix == DKIX_ERROR) { - // error - PyStackRef_CLEAR(*method); + int found = _PyDict_GetMethodStackRef(dict, name, method); + if (found < 0) { + assert(PyStackRef_IsNull(*method)); return -1; } - else if (!PyStackRef_IsNull(ref)) { - // found - _PyStackRef tmp = *method; - *method = ref; - PyStackRef_XCLOSE(tmp); + else if (found) { return 0; } } From 1a02ae6305819cb96204318d2f1b016998518b3b Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Tue, 22 Jul 2025 01:27:12 +0530 Subject: [PATCH 9/9] fix cast --- Objects/dictobject.c | 2 +- Objects/object.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 9e537f095c332c..928b905aaedb1b 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -1672,7 +1672,7 @@ _PyDict_GetMethodStackRef(PyDictObject *mp, PyObject *key, _PyStackRef *method) #ifdef Py_GIL_DISABLED PyDictKeysObject *dk = _Py_atomic_load_ptr_acquire(&mp->ma_keys); if (dk->dk_kind == DICT_KEYS_UNICODE) { - _PyStackRef ref; // + _PyStackRef ref; Py_ssize_t ix = lookup_threadsafe_unicode(dk, key, hash, &ref); if (ix >= 0) { assert(!PyStackRef_IsNull(ref)); diff --git a/Objects/object.c b/Objects/object.c index 66996051973c91..479f4176a46039 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -1754,7 +1754,7 @@ _PyObject_GetMethodStackRef(PyThreadState *ts, PyObject *obj, } if (dict != NULL) { assert(PyUnicode_CheckExact(name)); - int found = _PyDict_GetMethodStackRef(dict, name, method); + int found = _PyDict_GetMethodStackRef((PyDictObject *)dict, name, method); if (found < 0) { assert(PyStackRef_IsNull(*method)); return -1;