gh-145685: Improve scaling of type attribute lookups (gh-145774)

colesbury · miss-islington · commit 1509eb7f87a6 · 2026-03-12T17:30:45.000Z
Avoid locking in the PyType_Lookup cache-miss path if the type's tp_version_tag is already valid. (cherry picked from commit cd52172) Co-authored-by: Sam Gross <colesbury@gmail.com>
diff --git a/Include/internal/pycore_pyatomic_ft_wrappers.h b/Include/internal/pycore_pyatomic_ft_wrappers.h
@@ -87,6 +87,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) \
@@ -158,6 +160,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
@@ -0,0 +1,2 @@
+Improve scaling of type attribute lookups in the :term:`free-threaded build` by
+avoiding contention on the internal type lock.
diff --git a/Objects/typeobject.c b/Objects/typeobject.c
@@ -51,8 +51,8 @@ class object "PyObject *" "&PyBaseObject_Type"
     MCACHE_HASH(FT_ATOMIC_LOAD_UINT32_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
@@ -5860,6 +5860,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)
 {
@@ -5908,41 +5916,39 @@ _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) {
         *out = PyStackRef_NULL;
         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.

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	+Improve scaling of type attribute lookups in the :term:`free-threaded build` by
	`2`	`+avoiding contention on the internal type lock.`