https://github.com/python/cpython/commit/2984ff9e5196aa575c7322c2040d55dd4d4eaa02
commit: 2984ff9e5196aa575c7322c2040d55dd4d4eaa02
branch: main
author: Dino Viehland <dinoviehl...@meta.com>
committer: DinoV <dinoviehl...@gmail.com>
date: 2025-03-28T15:16:41-07:00
summary:
gh-130373: Avoid locking in _LOAD_ATTR_WITH_HINT (#130372)
Avoid locking in _LOAD_ATTR_WITH_HINT
files:
M Include/internal/pycore_dict.h
M Objects/dictobject.c
M Python/bytecodes.c
M Python/executor_cases.c.h
M Python/generated_cases.c.h
M Python/specialize.c
diff --git a/Include/internal/pycore_dict.h b/Include/internal/pycore_dict.h
index 973772a262e9f2..754eb88a85c33c 100644
--- 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 25ad525c7762b8..a290eae6464e07 100644
--- 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 b3b7441c31569a..63367141c6951b 100644
--- 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 9306a6aea35435..bf21c7bfcef393 100644
--- 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 f1e22f6d3dd700..4e37c68e2bd8bc 100644
--- 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 5634c601bc5da7..ac847b7c752b28 100644
--- 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)) {
_______________________________________________
Python-checkins mailing list -- python-checkins@python.org
To unsubscribe send an email to python-checkins-le...@python.org
https://mail.python.org/mailman3/lists/python-checkins.python.org/
Member address: arch...@mail-archive.com
