]> git.ipfire.org Git - thirdparty/Python/cpython.git/commitdiff
gh-145685: Improve scaling of type attribute lookups (gh-145774)
authorSam Gross <colesbury@gmail.com>
Thu, 12 Mar 2026 17:30:36 +0000 (13:30 -0400)
committerGitHub <noreply@github.com>
Thu, 12 Mar 2026 17:30:36 +0000 (13:30 -0400)
Avoid locking in the PyType_Lookup cache-miss path if the type's
tp_version_tag is already valid.

Include/internal/pycore_pyatomic_ft_wrappers.h
Misc/NEWS.d/next/Core_and_Builtins/2026-03-10-12-52-06.gh-issue-145685.80B7gK.rst [new file with mode: 0644]
Objects/typeobject.c

index 70a32db663b29355573103a50b451560bdc77c50..c0f859a23e10b893adf0bb11ef6e470dbcfa2856 100644 (file)
@@ -95,6 +95,8 @@ extern "C" {
     _Py_atomic_store_int_relaxed(&value, new_value)
 #define FT_ATOMIC_LOAD_INT_RELAXED(value) \
     _Py_atomic_load_int_relaxed(&value)
+#define FT_ATOMIC_LOAD_UINT(value) \
+    _Py_atomic_load_uint(&value)
 #define FT_ATOMIC_STORE_UINT_RELAXED(value, new_value) \
     _Py_atomic_store_uint_relaxed(&value, new_value)
 #define FT_ATOMIC_LOAD_UINT_RELAXED(value) \
@@ -167,6 +169,7 @@ extern "C" {
 #define FT_ATOMIC_STORE_INT(value, new_value) value = new_value
 #define FT_ATOMIC_LOAD_INT_RELAXED(value) value
 #define FT_ATOMIC_STORE_INT_RELAXED(value, new_value) value = new_value
+#define FT_ATOMIC_LOAD_UINT(value) value
 #define FT_ATOMIC_LOAD_UINT_RELAXED(value) value
 #define FT_ATOMIC_STORE_UINT_RELAXED(value, new_value) value = new_value
 #define FT_ATOMIC_LOAD_LONG_RELAXED(value) value
diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-03-10-12-52-06.gh-issue-145685.80B7gK.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-03-10-12-52-06.gh-issue-145685.80B7gK.rst
new file mode 100644 (file)
index 0000000..da34b67
--- /dev/null
@@ -0,0 +1,2 @@
+Improve scaling of type attribute lookups in the :term:`free-threaded build` by
+avoiding contention on the internal type lock.
index bb473dce68f65bc23fbe854c38f4790306fdbc7b..2a818f5f0205fd1c06356cd3cfad7eda4fa278b3 100644 (file)
@@ -52,8 +52,8 @@ class object "PyObject *" "&PyBaseObject_Type"
     MCACHE_HASH(FT_ATOMIC_LOAD_UINT_RELAXED((type)->tp_version_tag),   \
                 ((Py_ssize_t)(name)) >> 3)
 #define MCACHE_CACHEABLE_NAME(name)                             \
-        PyUnicode_CheckExact(name) &&                           \
-        (PyUnicode_GET_LENGTH(name) <= MCACHE_MAX_ATTR_SIZE)
+        (PyUnicode_CheckExact(name) &&                           \
+         (PyUnicode_GET_LENGTH(name) <= MCACHE_MAX_ATTR_SIZE))
 
 #define NEXT_VERSION_TAG(interp) \
     (interp)->types.next_version_tag
@@ -6134,6 +6134,14 @@ _PyType_LookupRefAndVersion(PyTypeObject *type, PyObject *name, unsigned int *ve
     return PyStackRef_AsPyObjectSteal(out);
 }
 
+static int
+should_assign_version_tag(PyTypeObject *type, PyObject *name, unsigned int version_tag)
+{
+    return (version_tag == 0
+        && FT_ATOMIC_LOAD_UINT16_RELAXED(type->tp_versions_used) < MAX_VERSIONS_PER_CLASS
+        && MCACHE_CACHEABLE_NAME(name));
+}
+
 unsigned int
 _PyType_LookupStackRefAndVersion(PyTypeObject *type, PyObject *name, _PyStackRef *out)
 {
@@ -6182,24 +6190,20 @@ _PyType_LookupStackRefAndVersion(PyTypeObject *type, PyObject *name, _PyStackRef
     /* We may end up clearing live exceptions below, so make sure it's ours. */
     assert(!PyErr_Occurred());
 
-    // We need to atomically do the lookup and capture the version before
-    // anyone else can modify our mro or mutate the type.
-
     int res;
     PyInterpreterState *interp = _PyInterpreterState_GET();
-    int has_version = 0;
-    unsigned int assigned_version = 0;
 
-    BEGIN_TYPE_LOCK();
-    // We must assign the version before doing the lookup.  If
-    // find_name_in_mro() blocks and releases the critical section
-    // then the type version can change.
-    if (MCACHE_CACHEABLE_NAME(name)) {
-        has_version = assign_version_tag(interp, type);
-        assigned_version = type->tp_version_tag;
-    }
-    res = find_name_in_mro(type, name, out);
-    END_TYPE_LOCK();
+    unsigned int version_tag = FT_ATOMIC_LOAD_UINT(type->tp_version_tag);
+    if (should_assign_version_tag(type, name, version_tag)) {
+        BEGIN_TYPE_LOCK();
+        assign_version_tag(interp, type);
+        version_tag = type->tp_version_tag;
+        res = find_name_in_mro(type, name, out);
+        END_TYPE_LOCK();
+    }
+    else {
+        res = find_name_in_mro(type, name, out);
+    }
 
     /* Only put NULL results into cache if there was no error. */
     if (res < 0) {
@@ -6207,16 +6211,18 @@ _PyType_LookupStackRefAndVersion(PyTypeObject *type, PyObject *name, _PyStackRef
         return 0;
     }
 
-    if (has_version) {
-        PyObject *res_obj = PyStackRef_AsPyObjectBorrow(*out);
+    if (version_tag == 0 || !MCACHE_CACHEABLE_NAME(name)) {
+        return 0;
+    }
+
+    PyObject *res_obj = PyStackRef_AsPyObjectBorrow(*out);
 #if Py_GIL_DISABLED
-        update_cache_gil_disabled(entry, name, assigned_version, res_obj);
+    update_cache_gil_disabled(entry, name, version_tag, res_obj);
 #else
-        PyObject *old_value = update_cache(entry, name, assigned_version, res_obj);
-        Py_DECREF(old_value);
+    PyObject *old_value = update_cache(entry, name, version_tag, res_obj);
+    Py_DECREF(old_value);
 #endif
-    }
-    return has_version ? assigned_version : 0;
+    return version_tag;
 }
 
 /* Internal API to look for a name through the MRO.