From b6bca55db1dab1f1f38850a44fd169f8aa583cb8 Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Thu, 8 May 2025 00:03:52 -0700 Subject: [PATCH 1/4] Use unicode hash/compare for mcache. This allows cache to work with non-interned strings. --- Objects/typeobject.c | 51 ++++++++++++++++++++++++++++++++++++-------- 1 file changed, 42 insertions(+), 9 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index a7ab69fef4c721..917187e9db2bf2 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -43,13 +43,46 @@ class object "PyObject *" "&PyBaseObject_Type" MCACHE_MAX_ATTR_SIZE, since it might be a problem if very large strings are used as attribute names. */ #define MCACHE_MAX_ATTR_SIZE 100 -#define MCACHE_HASH(version, name_hash) \ - (((unsigned int)(version) ^ (unsigned int)(name_hash)) \ - & ((1 << MCACHE_SIZE_EXP) - 1)) -#define MCACHE_HASH_METHOD(type, name) \ - MCACHE_HASH(FT_ATOMIC_LOAD_UINT32_RELAXED((type)->tp_version_tag), \ - ((Py_ssize_t)(name)) >> 3) +static inline unsigned int +mcache_name_hash(PyTypeObject *type, PyObject *name) +{ + unsigned int name_hash; +#if Py_GIL_DISABLED + // Cache misses are relatively more expensive for the free-threaded build. + // So we use the unicode string hash and unicode compare for caching + // names. This allows caching of non-interned strings. + if (PyUnicode_CheckExact(name)) { + name_hash = (unsigned int)_PyObject_HashFast(name); + } + else { + name_hash = 0; + } +#else + // Use the pointer value of the string for the hash and the compare. This + // is faster but non-interned strings can't use the cache. + name_hash = ((Py_ssize_t)(name)) >> 3; +#endif + unsigned int version = FT_ATOMIC_LOAD_UINT32_RELAXED((type)->tp_version_tag); + return (((unsigned int)(version) ^ (unsigned int)(name_hash)) & + ((1 << MCACHE_SIZE_EXP) - 1)); +} + +static inline int +mcache_name_eq(PyObject *entry_name, PyObject *name) +{ +#ifdef Py_GIL_DISABLED + if (entry_name == NULL || entry_name == Py_None) { + return 0; + } + assert(PyUnicode_CheckExact(entry_name)); + assert(PyUnicode_Check(name)); + return _PyUnicode_Equal(entry_name, name); +#else + return entry_name == name; +#endif +} + #define MCACHE_CACHEABLE_NAME(name) \ PyUnicode_CheckExact(name) && \ (PyUnicode_GET_LENGTH(name) <= MCACHE_MAX_ATTR_SIZE) @@ -5721,7 +5754,7 @@ _PyType_LookupRefAndVersion(PyTypeObject *type, PyObject *name, unsigned int *ve unsigned int _PyType_LookupStackRefAndVersion(PyTypeObject *type, PyObject *name, _PyStackRef *out) { - unsigned int h = MCACHE_HASH_METHOD(type, name); + unsigned int h = mcache_name_hash(type, name); struct type_cache *cache = get_type_cache(); struct type_cache_entry *entry = &cache->hashtable[h]; #ifdef Py_GIL_DISABLED @@ -5731,7 +5764,7 @@ _PyType_LookupStackRefAndVersion(PyTypeObject *type, PyObject *name, _PyStackRef uint32_t entry_version = _Py_atomic_load_uint32_acquire(&entry->version); uint32_t type_version = _Py_atomic_load_uint32_acquire(&type->tp_version_tag); if (entry_version == type_version && - _Py_atomic_load_ptr_relaxed(&entry->name) == name) { + mcache_name_eq(_Py_atomic_load_ptr_relaxed(&entry->name), name)) { OBJECT_STAT_INC_COND(type_cache_hits, !is_dunder_name(name)); OBJECT_STAT_INC_COND(type_cache_dunder_hits, is_dunder_name(name)); if (_Py_TryXGetStackRef(&entry->value, out)) { @@ -5752,7 +5785,7 @@ _PyType_LookupStackRefAndVersion(PyTypeObject *type, PyObject *name, _PyStackRef } } #else - if (entry->version == type->tp_version_tag && entry->name == name) { + if (entry->version == type->tp_version_tag && mcache_name_eq(entry->name, name)) { assert(type->tp_version_tag); OBJECT_STAT_INC_COND(type_cache_hits, !is_dunder_name(name)); OBJECT_STAT_INC_COND(type_cache_dunder_hits, is_dunder_name(name)); From 3ebc1e8ca48e621b3e48b0336e3806a06e261683 Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Thu, 8 May 2025 11:23:02 -0700 Subject: [PATCH 2/4] Add NEWS. --- .../2025-05-08-11-22-57.gh-issue-132380._9vB7H.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2025-05-08-11-22-57.gh-issue-132380._9vB7H.rst diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-05-08-11-22-57.gh-issue-132380._9vB7H.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-05-08-11-22-57.gh-issue-132380._9vB7H.rst new file mode 100644 index 00000000000000..87eae80497f7c4 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-05-08-11-22-57.gh-issue-132380._9vB7H.rst @@ -0,0 +1,2 @@ +For free-threaded build, allow non-interned strings to be cached in the type +lookup cache. This helps lock contention by reducing cache misses. From 8335fa1e7a348f3b78d1d09f3258856a9100d5a3 Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Thu, 8 May 2025 12:14:23 -0700 Subject: [PATCH 3/4] Only look in cache for exact unicode strings. Ensure we don't read the cache in the case the 'name' argument is a non-exact unicode string. It's possible to have an overridden `__eq__` method and it using `_PyUnicode_Equal()` in that case would be wrong. --- Objects/typeobject.c | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 917187e9db2bf2..9e66abba350fe8 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -47,27 +47,37 @@ class object "PyObject *" "&PyBaseObject_Type" static inline unsigned int mcache_name_hash(PyTypeObject *type, PyObject *name) { - unsigned int name_hash; + Py_hash_t name_hash; #if Py_GIL_DISABLED // Cache misses are relatively more expensive for the free-threaded build. // So we use the unicode string hash and unicode compare for caching // names. This allows caching of non-interned strings. - if (PyUnicode_CheckExact(name)) { - name_hash = (unsigned int)_PyObject_HashFast(name); - } - else { - name_hash = 0; - } + assert(PyUnicode_CheckExact(name)); + name_hash = _PyObject_HashFast(name); + // should not fail to hash an exact unicode object + assert(name_hash != -1); #else // Use the pointer value of the string for the hash and the compare. This // is faster but non-interned strings can't use the cache. - name_hash = ((Py_ssize_t)(name)) >> 3; + name_hash = ((Py_hash_t)(name)) >> 3; #endif unsigned int version = FT_ATOMIC_LOAD_UINT32_RELAXED((type)->tp_version_tag); return (((unsigned int)(version) ^ (unsigned int)(name_hash)) & ((1 << MCACHE_SIZE_EXP) - 1)); } +static inline struct type_cache_entry * +mcache_get_entry(PyTypeObject *type, PyObject *name, struct type_cache *cache) +{ +#ifdef Py_GIL_DISABLED + if (!PyUnicode_CheckExact(name)) { + return NULL; + } +#endif + unsigned int h = mcache_name_hash(type, name); + return &cache->hashtable[h]; +} + static inline int mcache_name_eq(PyObject *entry_name, PyObject *name) { @@ -76,7 +86,7 @@ mcache_name_eq(PyObject *entry_name, PyObject *name) return 0; } assert(PyUnicode_CheckExact(entry_name)); - assert(PyUnicode_Check(name)); + assert(PyUnicode_CheckExact(name)); return _PyUnicode_Equal(entry_name, name); #else return entry_name == name; @@ -5754,12 +5764,11 @@ _PyType_LookupRefAndVersion(PyTypeObject *type, PyObject *name, unsigned int *ve unsigned int _PyType_LookupStackRefAndVersion(PyTypeObject *type, PyObject *name, _PyStackRef *out) { - unsigned int h = mcache_name_hash(type, name); struct type_cache *cache = get_type_cache(); - struct type_cache_entry *entry = &cache->hashtable[h]; + struct type_cache_entry *entry = mcache_get_entry(type, name, cache); #ifdef Py_GIL_DISABLED // synchronize-with other writing threads by doing an acquire load on the sequence - while (1) { + while (entry != NULL) { uint32_t sequence = _PySeqLock_BeginRead(&entry->sequence); uint32_t entry_version = _Py_atomic_load_uint32_acquire(&entry->version); uint32_t type_version = _Py_atomic_load_uint32_acquire(&type->tp_version_tag); @@ -5837,6 +5846,7 @@ _PyType_LookupStackRefAndVersion(PyTypeObject *type, PyObject *name, _PyStackRef if (has_version) { #if Py_GIL_DISABLED + assert(entry != NULL); update_cache_gil_disabled(entry, name, assigned_version, res); #else PyObject *old_value = update_cache(entry, name, assigned_version, res); From 7723cebeb4e4df80725b657d300c10303a205897 Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Mon, 12 May 2025 06:43:05 -0700 Subject: [PATCH 4/4] Optimize mcache_name_eq(). Handle common cases early. --- Objects/typeobject.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 9e66abba350fe8..f54c49625f972a 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -82,11 +82,17 @@ static inline int mcache_name_eq(PyObject *entry_name, PyObject *name) { #ifdef Py_GIL_DISABLED + if (entry_name == name) { + return 1; + } if (entry_name == NULL || entry_name == Py_None) { return 0; } assert(PyUnicode_CheckExact(entry_name)); assert(PyUnicode_CheckExact(name)); + if (_PyObject_HashFast(entry_name) != _PyObject_HashFast(name)) { + return 0; + } return _PyUnicode_Equal(entry_name, name); #else return entry_name == name;