Simplify incref's around new_dict_with_shared_keys, fix stats

Lock obj instead of using atomic, and fix reads/writes
python · May 6, 2024 · e7e5ed4 · e7e5ed4
1 parent 56a79e6
commit e7e5ed4
Show file tree

Hide file tree

Showing 2 changed files with 7 additions and 17 deletions.
diff --git a/Objects/dictobject.c b/Objects/dictobject.c
@@ -931,9 +931,9 @@ new_dict_with_shared_keys(PyInterpreterState *interp, PyDictKeysObject *keys)
     size_t size = shared_keys_usable_size(keys);
     PyDictValues *values = new_values(size);
     if (values == NULL) {
-        dictkeys_decref(interp, keys, false);
         return PyErr_NoMemory();
     }
+    dictkeys_incref(keys);
     for (size_t i = 0; i < size; i++) {
         values->values[i] = NULL;
     }
@@ -6666,8 +6666,6 @@ materialize_managed_dict_lock_held(PyObject *obj)
 {
     _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(obj);
 
-    OBJECT_STAT_INC(dict_materialized_on_request);
-
     PyDictValues *values = _PyObject_InlineValues(obj);
     PyInterpreterState *interp = _PyInterpreterState_GET();
     PyDictKeysObject *keys = CACHED_KEYS(Py_TYPE(obj));
@@ -7178,8 +7176,6 @@ ensure_managed_dict(PyObject *obj)
                 goto done;
             }
 #endif
-            OBJECT_STAT_INC(dict_materialized_on_request);
-            dictkeys_incref(CACHED_KEYS(tp));
             dict = (PyDictObject *)new_dict_with_shared_keys(_PyInterpreterState_GET(),
                                                              CACHED_KEYS(tp));
             FT_ATOMIC_STORE_PTR_RELEASE(_PyObject_ManagedDictPointer(obj)->dict,
@@ -7199,7 +7195,7 @@ ensure_nonmanaged_dict(PyObject *obj, PyObject **dictptr)
 {
     PyDictKeysObject *cached;
 
-    PyObject *dict = FT_ATOMIC_LOAD_PTR_RELAXED(*dictptr);
+    PyObject *dict = FT_ATOMIC_LOAD_PTR_ACQUIRE(*dictptr);
     if (dict == NULL) {
 #ifdef Py_GIL_DISABLED
         Py_BEGIN_CRITICAL_SECTION(obj);
@@ -7209,19 +7205,15 @@ ensure_nonmanaged_dict(PyObject *obj, PyObject **dictptr)
         }
 #endif
         PyTypeObject *tp = Py_TYPE(obj);
-        if ((tp->tp_flags & Py_TPFLAGS_HEAPTYPE) && (cached = CACHED_KEYS(tp))) {
+        if (_PyType_HasFeature(tp, Py_TPFLAGS_HEAPTYPE) && (cached = CACHED_KEYS(tp))) {
             PyInterpreterState *interp = _PyInterpreterState_GET();
             assert(!_PyType_HasFeature(tp, Py_TPFLAGS_INLINE_VALUES));
-            dictkeys_incref(cached);
             dict = new_dict_with_shared_keys(interp, cached);
-            if (dict == NULL) {
-                dictkeys_decref(interp, cached, false);
-            }
         }
         else {
             dict = PyDict_New();
         }
-        FT_ATOMIC_STORE_PTR_RELAXED(*dictptr, dict);
+        FT_ATOMIC_STORE_PTR_RELEASE(*dictptr, dict);
 #ifdef Py_GIL_DISABLED
 done:
         Py_END_CRITICAL_SECTION();
@@ -7264,8 +7256,8 @@ _PyObjectDict_SetItem(PyTypeObject *tp, PyObject *obj, PyObject **dictptr,
 
     Py_BEGIN_CRITICAL_SECTION(dict);
     res = set_or_del_lock_held((PyDictObject *)dict, key, value);
-    Py_END_CRITICAL_SECTION();
     ASSERT_CONSISTENT(dict);
+    Py_END_CRITICAL_SECTION();
     return res;
 }
 

diff --git a/Objects/object.c b/Objects/object.c
@@ -1797,11 +1797,9 @@ PyObject_GenericSetDict(PyObject *obj, PyObject *value, void *context)
                      "not a '%.200s'", Py_TYPE(value)->tp_name);
         return -1;
     }
-#ifdef Py_GIL_DISABLED
-    Py_XDECREF(_Py_atomic_exchange_ptr(dictptr, Py_NewRef(value)));
-#else
+    Py_BEGIN_CRITICAL_SECTION(obj);
     Py_XSETREF(*dictptr, Py_NewRef(value));
-#endif
+    Py_END_CRITICAL_SECTION();
     return 0;
 }