gh-130373: Avoid locking in _LOAD_ATTR_WITH_HINT (#130372)

Avoid locking in _LOAD_ATTR_WITH_HINT

diff --git a/Include/internal/pycore_dict.h b/Include/internal/pycore_dict.h
index 973772a262e9f28db74be6e377341bd2cb272b63..754eb88a85c33c21bc2ec5e7e4b86e5ff8ad08d3 100644 (file)
--- a/Include/internal/pycore_dict.h
+++ b/Include/internal/pycore_dict.h
@@ -150,6 +150,10 @@ extern int _PyDict_Pop_KnownHash(
     Py_hash_t hash,
     PyObject **result);
 
+#ifdef Py_GIL_DISABLED
+PyAPI_FUNC(void) _PyDict_EnsureSharedOnRead(PyDictObject *mp);
+#endif
+
 #define DKIX_EMPTY (-1)
 #define DKIX_DUMMY (-2)  /* Used internally */
 #define DKIX_ERROR (-3)

diff --git a/Objects/dictobject.c b/Objects/dictobject.c
index 25ad525c7762b873b8f7fe0993b46ea894b83bed..a290eae6464e0724d853f42c07598e549d381d32 100644 (file)
--- a/Objects/dictobject.c
+++ b/Objects/dictobject.c
@@ -1326,6 +1326,12 @@ ensure_shared_on_read(PyDictObject *mp)
         Py_END_CRITICAL_SECTION();
     }
 }
+
+void
+_PyDict_EnsureSharedOnRead(PyDictObject *mp)
+{
+    ensure_shared_on_read(mp);
+}
 #endif
 
 static inline void

diff --git a/Python/bytecodes.c b/Python/bytecodes.c
index b3b7441c31569a39c4eafba20803dc9ce742fba7..63367141c6951b635d0d6414ca4bee7937735278 100644 (file)
--- a/Python/bytecodes.c
+++ b/Python/bytecodes.c
@@ -2283,34 +2283,36 @@ dummy_func(
             assert(Py_TYPE(owner_o)->tp_flags & Py_TPFLAGS_MANAGED_DICT);
             PyDictObject *dict = _PyObject_GetManagedDict(owner_o);
             DEOPT_IF(dict == NULL);
+            PyDictKeysObject *dk = FT_ATOMIC_LOAD_PTR(dict->ma_keys);
             assert(PyDict_CheckExact((PyObject *)dict));
+#ifdef Py_GIL_DISABLED
+            DEOPT_IF(!_Py_IsOwnedByCurrentThread((PyObject *)dict) && !_PyObject_GC_IS_SHARED(dict));
+#endif
             PyObject *attr_o;
-            if (!LOCK_OBJECT(dict)) {
+            if (hint >= (size_t)FT_ATOMIC_LOAD_SSIZE_RELAXED(dk->dk_nentries)) {
                 DEOPT_IF(true);
             }
 
-            if (hint >= (size_t)dict->ma_keys->dk_nentries) {
-                UNLOCK_OBJECT(dict);
-                DEOPT_IF(true);
-            }
             PyObject *name = GETITEM(FRAME_CO_NAMES, oparg>>1);
-            if (dict->ma_keys->dk_kind != DICT_KEYS_UNICODE) {
-                UNLOCK_OBJECT(dict);
+            if (dk->dk_kind != DICT_KEYS_UNICODE) {
                 DEOPT_IF(true);
             }
-            PyDictUnicodeEntry *ep = DK_UNICODE_ENTRIES(dict->ma_keys) + hint;
-            if (ep->me_key != name) {
-                UNLOCK_OBJECT(dict);
+            PyDictUnicodeEntry *ep = DK_UNICODE_ENTRIES(dk) + hint;
+            if (FT_ATOMIC_LOAD_PTR_RELAXED(ep->me_key) != name) {
                 DEOPT_IF(true);
             }
-            attr_o = ep->me_value;
+            attr_o = FT_ATOMIC_LOAD_PTR(ep->me_value);
             if (attr_o == NULL) {
-                UNLOCK_OBJECT(dict);
                 DEOPT_IF(true);
             }
             STAT_INC(LOAD_ATTR, hit);
+#ifdef Py_GIL_DISABLED
+            if (!_Py_TryIncrefCompareStackRef(&ep->me_value, attr_o, &attr)) {
+                DEOPT_IF(true);
+            }
+#else
             attr = PyStackRef_FromPyObjectNew(attr_o);
-            UNLOCK_OBJECT(dict);
+#endif
             PyStackRef_CLOSE(owner);
         }
 

diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h
index 9306a6aea354358974f5f16e1011bccd810dcf3b..bf21c7bfcef3937e4f79135e711f99285dfb146e 100644 (file)
--- a/Python/executor_cases.c.h
+++ b/Python/executor_cases.c.h
@@ -3209,48 +3209,53 @@
                 UOP_STAT_INC(uopcode, miss);
                 JUMP_TO_JUMP_TARGET();
             }
+            PyDictKeysObject *dk = FT_ATOMIC_LOAD_PTR(dict->ma_keys);
             assert(PyDict_CheckExact((PyObject *)dict));
+            #ifdef Py_GIL_DISABLED
+            if (!_Py_IsOwnedByCurrentThread((PyObject *)dict) && !_PyObject_GC_IS_SHARED(dict)) {
+                UOP_STAT_INC(uopcode, miss);
+                JUMP_TO_JUMP_TARGET();
+            }
+            #endif
             PyObject *attr_o;
-            if (!LOCK_OBJECT(dict)) {
+            if (hint >= (size_t)FT_ATOMIC_LOAD_SSIZE_RELAXED(dk->dk_nentries)) {
                 if (true) {
                     UOP_STAT_INC(uopcode, miss);
                     JUMP_TO_JUMP_TARGET();
                 }
             }
-            if (hint >= (size_t)dict->ma_keys->dk_nentries) {
-                UNLOCK_OBJECT(dict);
+            PyObject *name = GETITEM(FRAME_CO_NAMES, oparg>>1);
+            if (dk->dk_kind != DICT_KEYS_UNICODE) {
                 if (true) {
                     UOP_STAT_INC(uopcode, miss);
                     JUMP_TO_JUMP_TARGET();
                 }
             }
-            PyObject *name = GETITEM(FRAME_CO_NAMES, oparg>>1);
-            if (dict->ma_keys->dk_kind != DICT_KEYS_UNICODE) {
-                UNLOCK_OBJECT(dict);
+            PyDictUnicodeEntry *ep = DK_UNICODE_ENTRIES(dk) + hint;
+            if (FT_ATOMIC_LOAD_PTR_RELAXED(ep->me_key) != name) {
                 if (true) {
                     UOP_STAT_INC(uopcode, miss);
                     JUMP_TO_JUMP_TARGET();
                 }
             }
-            PyDictUnicodeEntry *ep = DK_UNICODE_ENTRIES(dict->ma_keys) + hint;
-            if (ep->me_key != name) {
-                UNLOCK_OBJECT(dict);
+            attr_o = FT_ATOMIC_LOAD_PTR(ep->me_value);
+            if (attr_o == NULL) {
                 if (true) {
                     UOP_STAT_INC(uopcode, miss);
                     JUMP_TO_JUMP_TARGET();
                 }
             }
-            attr_o = ep->me_value;
-            if (attr_o == NULL) {
-                UNLOCK_OBJECT(dict);
+            STAT_INC(LOAD_ATTR, hit);
+            #ifdef Py_GIL_DISABLED
+            if (!_Py_TryIncrefCompareStackRef(&ep->me_value, attr_o, &attr)) {
                 if (true) {
                     UOP_STAT_INC(uopcode, miss);
                     JUMP_TO_JUMP_TARGET();
                 }
             }
-            STAT_INC(LOAD_ATTR, hit);
+            #else
             attr = PyStackRef_FromPyObjectNew(attr_o);
-            UNLOCK_OBJECT(dict);
+            #endif
             stack_pointer[-1] = attr;
             _PyFrame_SetStackPointer(frame, stack_pointer);
             PyStackRef_CLOSE(owner);

diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h
index f1e22f6d3dd70036cefb6c4606d59a576b402561..4e37c68e2bd8bc881b7b006c2bead73aa010e98f 100644 (file)
--- a/Python/generated_cases.c.h
+++ b/Python/generated_cases.c.h
@@ -8618,53 +8618,59 @@
                     assert(_PyOpcode_Deopt[opcode] == (LOAD_ATTR));
                     JUMP_TO_PREDICTED(LOAD_ATTR);
                 }
+                PyDictKeysObject *dk = FT_ATOMIC_LOAD_PTR(dict->ma_keys);
                 assert(PyDict_CheckExact((PyObject *)dict));
+                #ifdef Py_GIL_DISABLED
+                if (!_Py_IsOwnedByCurrentThread((PyObject *)dict) && !_PyObject_GC_IS_SHARED(dict)) {
+                    UPDATE_MISS_STATS(LOAD_ATTR);
+                    assert(_PyOpcode_Deopt[opcode] == (LOAD_ATTR));
+                    JUMP_TO_PREDICTED(LOAD_ATTR);
+                }
+                #endif
                 PyObject *attr_o;
-                if (!LOCK_OBJECT(dict)) {
+                if (hint >= (size_t)FT_ATOMIC_LOAD_SSIZE_RELAXED(dk->dk_nentries)) {
                     if (true) {
                         UPDATE_MISS_STATS(LOAD_ATTR);
                         assert(_PyOpcode_Deopt[opcode] == (LOAD_ATTR));
                         JUMP_TO_PREDICTED(LOAD_ATTR);
                     }
                 }
-                if (hint >= (size_t)dict->ma_keys->dk_nentries) {
-                    UNLOCK_OBJECT(dict);
+                PyObject *name = GETITEM(FRAME_CO_NAMES, oparg>>1);
+                if (dk->dk_kind != DICT_KEYS_UNICODE) {
                     if (true) {
                         UPDATE_MISS_STATS(LOAD_ATTR);
                         assert(_PyOpcode_Deopt[opcode] == (LOAD_ATTR));
                         JUMP_TO_PREDICTED(LOAD_ATTR);
                     }
                 }
-                PyObject *name = GETITEM(FRAME_CO_NAMES, oparg>>1);
-                if (dict->ma_keys->dk_kind != DICT_KEYS_UNICODE) {
-                    UNLOCK_OBJECT(dict);
+                PyDictUnicodeEntry *ep = DK_UNICODE_ENTRIES(dk) + hint;
+                if (FT_ATOMIC_LOAD_PTR_RELAXED(ep->me_key) != name) {
                     if (true) {
                         UPDATE_MISS_STATS(LOAD_ATTR);
                         assert(_PyOpcode_Deopt[opcode] == (LOAD_ATTR));
                         JUMP_TO_PREDICTED(LOAD_ATTR);
                     }
                 }
-                PyDictUnicodeEntry *ep = DK_UNICODE_ENTRIES(dict->ma_keys) + hint;
-                if (ep->me_key != name) {
-                    UNLOCK_OBJECT(dict);
+                attr_o = FT_ATOMIC_LOAD_PTR(ep->me_value);
+                if (attr_o == NULL) {
                     if (true) {
                         UPDATE_MISS_STATS(LOAD_ATTR);
                         assert(_PyOpcode_Deopt[opcode] == (LOAD_ATTR));
                         JUMP_TO_PREDICTED(LOAD_ATTR);
                     }
                 }
-                attr_o = ep->me_value;
-                if (attr_o == NULL) {
-                    UNLOCK_OBJECT(dict);
+                STAT_INC(LOAD_ATTR, hit);
+                #ifdef Py_GIL_DISABLED
+                if (!_Py_TryIncrefCompareStackRef(&ep->me_value, attr_o, &attr)) {
                     if (true) {
                         UPDATE_MISS_STATS(LOAD_ATTR);
                         assert(_PyOpcode_Deopt[opcode] == (LOAD_ATTR));
                         JUMP_TO_PREDICTED(LOAD_ATTR);
                     }
                 }
-                STAT_INC(LOAD_ATTR, hit);
+                #else
                 attr = PyStackRef_FromPyObjectNew(attr_o);
-                UNLOCK_OBJECT(dict);
+                #endif
                 stack_pointer[-1] = attr;
                 _PyFrame_SetStackPointer(frame, stack_pointer);
                 PyStackRef_CLOSE(owner);

diff --git a/Python/specialize.c b/Python/specialize.c
index 5634c601bc5da708bb68dbfdedf6edbe88816fa7..ac847b7c752b2823ae3c3867a858208409ba6c22 100644 (file)
--- a/Python/specialize.c
+++ b/Python/specialize.c
@@ -1009,6 +1009,9 @@ specialize_dict_access_hint(
     _PyAttrCache *cache = (_PyAttrCache *)(instr + 1);
 
     _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(dict);
+#ifdef Py_GIL_DISABLED
+    _PyDict_EnsureSharedOnRead(dict);
+#endif
 
     // We found an instance with a __dict__.
     if (_PyDict_HasSplitTable(dict)) {