From db6d52efbc741c714ee2753f95db29ffabc5da55 Mon Sep 17 00:00:00 2001
From: Neil Schemenauer <nas@arctrix.com>
Date: Wed, 28 May 2025 14:03:11 -0700
Subject: [PATCH] GH-132380: Avoid locking in type lookup.

If the type lookup cache (mcache) is missed, do not acquire TYPE_LOCK
unless we need to assign a type version.  We actually don't need to lock
in order to do the MRO attribute lookup.
---
 ...-06-03-12-30-36.gh-issue-132380.vQ_wD4.rst |  4 ++
 Objects/typeobject.c                          | 47 +++++++++++++------
 2 files changed, 37 insertions(+), 14 deletions(-)
 create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2025-06-03-12-30-36.gh-issue-132380.vQ_wD4.rst

diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-06-03-12-30-36.gh-issue-132380.vQ_wD4.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-06-03-12-30-36.gh-issue-132380.vQ_wD4.rst
new file mode 100644
index 00000000000000..57ec44c9d3e0a6
--- /dev/null
+++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-06-03-12-30-36.gh-issue-132380.vQ_wD4.rst
@@ -0,0 +1,4 @@
+When doing a type attribute lookup, do not acquire ``TYPE_LOCK`` unless we need
+to assign a new type version.  While doing the MRO lookup, we don't need to
+hold the lock.  This reduces lock contention, expecially when looking up
+attributes that are not in the type lookup cache.
diff --git a/Objects/typeobject.c b/Objects/typeobject.c
index db923c164774b7..b3d55b7f4233b8 100644
--- a/Objects/typeobject.c
+++ b/Objects/typeobject.c
@@ -5880,6 +5880,26 @@ PyObject_GetItemData(PyObject *obj)
     return (char *)obj + Py_TYPE(obj)->tp_basicsize;
 }
 
+// Try to assign a new type version tag, return it if successful.  Return 0
+// if no version was assigned.
+static unsigned int
+type_assign_version(PyTypeObject *type)
+{
+    unsigned int version = FT_ATOMIC_LOAD_UINT32_RELAXED((type)->tp_version_tag);
+    if (version == 0) {
+        BEGIN_TYPE_LOCK();
+        PyInterpreterState *interp = _PyInterpreterState_GET();
+        if (assign_version_tag(interp, type)) {
+            version = type->tp_version_tag;
+        }
+        else {
+            version = 0;
+        }
+        END_TYPE_LOCK();
+    }
+    return version;
+}
+
 /* Internal API to look for a name through the MRO, bypassing the method cache.
    This returns a borrowed reference, and might set an exception.
    'error' is set to: -1: error with exception; 1: error without exception; 0: ok */
@@ -5892,15 +5912,17 @@ find_name_in_mro(PyTypeObject *type, PyObject *name, int *error)
         return NULL;
     }
 
-    /* Look in tp_dict of types in MRO */
-    PyObject *mro = lookup_tp_mro(type);
+    /* Look in tp_dict of types in MRO.  Keep a strong reference to mro
+       because type->tp_mro can be replaced during dict lookup, e.g. when
+       comparing to non-string keys. */
+    PyObject *mro = _PyType_GetMRO(type);
     if (mro == NULL) {
         if (!is_readying(type)) {
             if (PyType_Ready(type) < 0) {
                 *error = -1;
                 return NULL;
             }
-            mro = lookup_tp_mro(type);
+            mro = _PyType_GetMRO(type);
         }
         if (mro == NULL) {
             *error = 1;
@@ -5909,9 +5931,6 @@ find_name_in_mro(PyTypeObject *type, PyObject *name, int *error)
     }
 
     PyObject *res = NULL;
-    /* Keep a strong reference to mro because type->tp_mro can be replaced
-       during dict lookup, e.g. when comparing to non-string keys. */
-    Py_INCREF(mro);
     Py_ssize_t n = PyTuple_GET_SIZE(mro);
     for (Py_ssize_t i = 0; i < n; i++) {
         PyObject *base = PyTuple_GET_ITEM(mro, i);
@@ -6084,19 +6103,19 @@ _PyType_LookupStackRefAndVersion(PyTypeObject *type, PyObject *name, _PyStackRef
 
     PyObject *res;
     int error;
-    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;
+        assigned_version = type_assign_version(type);
     }
+    // Note that calling find_name_in_mro() might cause the type version to
+    // change.  For example, if a __hash__ or __eq__ method mutates the types.
+    // In that case, 'assigned_version' will be stale after this call.  Since
+    // that's expected to be rare and does not cause a correctness issue, we
+    // don't check for this case.
     res = find_name_in_mro(type, name, &error);
-    END_TYPE_LOCK();
 
     /* Only put NULL results into cache if there was no error. */
     if (error) {
@@ -6115,7 +6134,7 @@ _PyType_LookupStackRefAndVersion(PyTypeObject *type, PyObject *name, _PyStackRef
         return 0;
     }
 
-    if (has_version) {
+    if (assigned_version != 0) {
 #if Py_GIL_DISABLED
         update_cache_gil_disabled(entry, name, assigned_version, res);
 #else
@@ -6124,7 +6143,7 @@ _PyType_LookupStackRefAndVersion(PyTypeObject *type, PyObject *name, _PyStackRef
 #endif
     }
     *out = res ? PyStackRef_FromPyObjectSteal(res) : PyStackRef_NULL;
-    return has_version ? assigned_version : 0;
+    return assigned_version;
 }
 
 /* Internal API to look for a name through the MRO.