Simplify incref's around new_dict_with_shared_keys, fix stats

DinoV · DinoV · commit e7e5ed45b595 · 2024-05-06T11:04:19.000-07:00
Lock obj instead of using atomic, and fix reads/writes
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;
 }
 

Original file line number	Diff line number	Diff line change
`@@ -1797,11 +1797,9 @@ PyObject_GenericSetDict(PyObject obj, PyObject value, void *context)`
`1797`	`1797`	`"not a '%.200s'", Py_TYPE(value)->tp_name);`
`1798`	`1798`	`return -1;`
`1799`	`1799`	`}`
`1800`		`-#ifdef Py_GIL_DISABLED`
`1801`		`- Py_XDECREF(_Py_atomic_exchange_ptr(dictptr, Py_NewRef(value)));`
`1802`		`-#else`
	`1800`	`+ Py_BEGIN_CRITICAL_SECTION(obj);`
`1803`	`1801`	`Py_XSETREF(*dictptr, Py_NewRef(value));`
`1804`		`-#endif`
	`1802`	`+ Py_END_CRITICAL_SECTION();`
`1805`	`1803`	`return 0;`
`1806`	`1804`	`}`
`1807`	`1805`