From 22d7bfbd62dedd5cfcbae34af0bb098417d6e913 Mon Sep 17 00:00:00 2001
From: Dino Viehland <dinoviehland@fb.com>
Date: Wed, 28 Feb 2024 16:09:30 -0800
Subject: [PATCH 1/2] Avoid locking shared keys on every assignment

---
 Objects/dictobject.c | 32 +++++++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/Objects/dictobject.c b/Objects/dictobject.c
index 5016e255f70ef9..2e6af7846bd0bf 100644
--- a/Objects/dictobject.c
+++ b/Objects/dictobject.c
@@ -1597,7 +1597,7 @@ insertion_resize(PyInterpreterState *interp, PyDictObject *mp, int unicode)
 }
 
 static Py_ssize_t
-insert_into_splitdictkeys(PyDictKeysObject *keys, PyObject *name)
+insert_into_splitdictkeys_locked(PyDictKeysObject *keys, PyObject *name)
 {
     assert(PyUnicode_CheckExact(name));
     ASSERT_KEYS_LOCKED(keys);
@@ -1629,6 +1629,15 @@ insert_into_splitdictkeys(PyDictKeysObject *keys, PyObject *name)
     return ix;
 }
 
+static Py_ssize_t
+insert_into_splitdictkeys(PyDictKeysObject *keys, PyObject *name)
+{
+    LOCK_KEYS(keys);
+    Py_ssize_t ix = insert_into_splitdictkeys_locked(keys, name);
+    UNLOCK_KEYS(keys);
+    return ix;
+}
+
 static inline int
 insert_combined_dict(PyInterpreterState *interp, PyDictObject *mp,
                      Py_hash_t hash, PyObject *key, PyObject *value)
@@ -6692,8 +6701,26 @@ _PyObject_StoreInstanceAttribute(PyObject *obj, PyDictValues *values,
     assert(Py_TYPE(obj)->tp_flags & Py_TPFLAGS_MANAGED_DICT);
     Py_ssize_t ix = DKIX_EMPTY;
     if (PyUnicode_CheckExact(name)) {
-        LOCK_KEYS(keys);
+#ifdef Py_GIL_DISABLED
+        Py_hash_t hash = unicode_get_hash(name);
+        if (hash == -1) {
+            hash = PyUnicode_Type.tp_hash(name);
+            if (hash == -1) {
+                PyErr_Clear();
+                return DKIX_EMPTY;
+            }
+        }
+
+        // Try a thread-safe lookup to see if the index is already allocated
+        ix = unicodekeys_lookup_unicode_threadsafe(keys, name, hash);
+        if (ix == DKIX_EMPTY) {
+            // Fall back to a version that will lock and maybe insert
+            ix = insert_into_splitdictkeys(keys, name);
+        }
+#else
         ix = insert_into_splitdictkeys(keys, name);
+#endif
+
 #ifdef Py_STATS
         if (ix == DKIX_EMPTY) {
             if (PyUnicode_CheckExact(name)) {
@@ -6709,7 +6736,6 @@ _PyObject_StoreInstanceAttribute(PyObject *obj, PyDictValues *values,
             }
         }
 #endif
-        UNLOCK_KEYS(keys);
     }
     if (ix == DKIX_EMPTY) {
         PyObject *dict = make_dict_from_instance_attributes(

From 8812f5b4e3629b7ff120562156f753855d77906b Mon Sep 17 00:00:00 2001
From: Dino Viehland <dinoviehland@fb.com>
Date: Thu, 29 Feb 2024 11:12:31 -0800
Subject: [PATCH 2/2] Assert we don't get -1 from hashing exact unicode, lock
 in _PyObject_StoreInstanceAttribute

---
 Objects/dictobject.c | 34 ++++++++--------------------------
 1 file changed, 8 insertions(+), 26 deletions(-)

diff --git a/Objects/dictobject.c b/Objects/dictobject.c
index 2e6af7846bd0bf..9b8fc4a958f7ad 100644
--- a/Objects/dictobject.c
+++ b/Objects/dictobject.c
@@ -1597,19 +1597,11 @@ insertion_resize(PyInterpreterState *interp, PyDictObject *mp, int unicode)
 }
 
 static Py_ssize_t
-insert_into_splitdictkeys_locked(PyDictKeysObject *keys, PyObject *name)
+insert_into_splitdictkeys(PyDictKeysObject *keys, PyObject *name, Py_hash_t hash)
 {
     assert(PyUnicode_CheckExact(name));
     ASSERT_KEYS_LOCKED(keys);
 
-    Py_hash_t hash = unicode_get_hash(name);
-    if (hash == -1) {
-        hash = PyUnicode_Type.tp_hash(name);
-        if (hash == -1) {
-            PyErr_Clear();
-            return DKIX_EMPTY;
-        }
-    }
     Py_ssize_t ix = unicodekeys_lookup_unicode(keys, name, hash);
     if (ix == DKIX_EMPTY) {
         if (keys->dk_usable <= 0) {
@@ -1629,15 +1621,6 @@ insert_into_splitdictkeys_locked(PyDictKeysObject *keys, PyObject *name)
     return ix;
 }
 
-static Py_ssize_t
-insert_into_splitdictkeys(PyDictKeysObject *keys, PyObject *name)
-{
-    LOCK_KEYS(keys);
-    Py_ssize_t ix = insert_into_splitdictkeys_locked(keys, name);
-    UNLOCK_KEYS(keys);
-    return ix;
-}
-
 static inline int
 insert_combined_dict(PyInterpreterState *interp, PyDictObject *mp,
                      Py_hash_t hash, PyObject *key, PyObject *value)
@@ -6701,24 +6684,23 @@ _PyObject_StoreInstanceAttribute(PyObject *obj, PyDictValues *values,
     assert(Py_TYPE(obj)->tp_flags & Py_TPFLAGS_MANAGED_DICT);
     Py_ssize_t ix = DKIX_EMPTY;
     if (PyUnicode_CheckExact(name)) {
-#ifdef Py_GIL_DISABLED
         Py_hash_t hash = unicode_get_hash(name);
         if (hash == -1) {
             hash = PyUnicode_Type.tp_hash(name);
-            if (hash == -1) {
-                PyErr_Clear();
-                return DKIX_EMPTY;
-            }
+            assert(hash != -1);
         }
 
+#ifdef Py_GIL_DISABLED
         // Try a thread-safe lookup to see if the index is already allocated
         ix = unicodekeys_lookup_unicode_threadsafe(keys, name, hash);
         if (ix == DKIX_EMPTY) {
-            // Fall back to a version that will lock and maybe insert
-            ix = insert_into_splitdictkeys(keys, name);
+            // Lock keys and do insert
+            LOCK_KEYS(keys);
+            ix = insert_into_splitdictkeys(keys, name, hash);
+            UNLOCK_KEYS(keys);
         }
 #else
-        ix = insert_into_splitdictkeys(keys, name);
+        ix = insert_into_splitdictkeys(keys, name, hash);
 #endif
 
 #ifdef Py_STATS