From 5bc2469e313d47e8e0daa7ffd3333e5638cfcbf2 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Sat, 6 May 2023 17:55:41 -0600 Subject: [PATCH 01/10] Factor out _dict_state_INIT. --- Include/internal/pycore_dict_state.h | 7 +++++++ Include/internal/pycore_runtime_init.h | 4 +--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/Include/internal/pycore_dict_state.h b/Include/internal/pycore_dict_state.h index d608088ed2d7ce..459371a339f2d9 100644 --- a/Include/internal/pycore_dict_state.h +++ b/Include/internal/pycore_dict_state.h @@ -27,6 +27,8 @@ struct _Py_dict_state { uint64_t global_version; uint32_t next_keys_version; +// PyDictKeysObject empty_keys_struct; + #if PyDict_MAXFREELIST > 0 /* Dictionary reuse scheme to save calls to malloc and free */ PyDictObject *free_list[PyDict_MAXFREELIST]; @@ -38,6 +40,11 @@ struct _Py_dict_state { PyDict_WatchCallback watchers[DICT_MAX_WATCHERS]; }; +#define _dict_state_INIT \ + { \ + .next_keys_version = 2, \ + } + #ifdef __cplusplus } diff --git a/Include/internal/pycore_runtime_init.h b/Include/internal/pycore_runtime_init.h index a48461c0742872..3b1444f3429b9a 100644 --- a/Include/internal/pycore_runtime_init.h +++ b/Include/internal/pycore_runtime_init.h @@ -107,9 +107,7 @@ extern PyTypeObject _PyExc_MemoryError; }, \ }, \ .dtoa = _dtoa_state_INIT(&(INTERP)), \ - .dict_state = { \ - .next_keys_version = 2, \ - }, \ + .dict_state = _dict_state_INIT, \ .func_state = { \ .next_version = 1, \ }, \ From e472d944abd7bdccdeaae96623b46580072f53c9 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Sat, 6 May 2023 17:56:19 -0600 Subject: [PATCH 02/10] Immortalize Py_EMPTY_KEYS. --- Objects/dictobject.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 2ef520044340ee..303a951a6adac9 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -300,18 +300,26 @@ _PyDict_DebugMallocStats(FILE *out) static void free_keys_object(PyInterpreterState *interp, PyDictKeysObject *keys); +// XXX Switch to Py_INCREF()? static inline void dictkeys_incref(PyDictKeysObject *dk) { + if (dk->dk_refcnt == _Py_IMMORTAL_REFCNT) { + return; + } #ifdef Py_REF_DEBUG _Py_IncRefTotal(_PyInterpreterState_GET()); #endif dk->dk_refcnt++; } +// XXX Switch to Py_DECREF()? static inline void dictkeys_decref(PyInterpreterState *interp, PyDictKeysObject *dk) { + if (dk->dk_refcnt == _Py_IMMORTAL_REFCNT) { + return; + } assert(dk->dk_refcnt > 0); #ifdef Py_REF_DEBUG _Py_DecRefTotal(_PyInterpreterState_GET()); @@ -447,7 +455,7 @@ estimate_log2_keysize(Py_ssize_t n) * (which cannot fail and thus can do no allocation). */ static PyDictKeysObject empty_keys_struct = { - 1, /* dk_refcnt */ + _Py_IMMORTAL_REFCNT, /* dk_refcnt */ 0, /* dk_log2_size */ 0, /* dk_log2_index_bytes */ DICT_KEYS_UNICODE, /* dk_kind */ From 87351e0159fd9b758e715f2033e57cec9165cb5b Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 8 May 2023 07:57:48 -0600 Subject: [PATCH 03/10] Handle other uses of dk_refcnt. --- Objects/dictobject.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 303a951a6adac9..0429912f6e0a92 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -787,6 +787,7 @@ clone_combined_dict_keys(PyDictObject *orig) assert(PyDict_Check(orig)); assert(Py_TYPE(orig)->tp_iter == (getiterfunc)dict_iter); assert(orig->ma_values == NULL); + assert(orig->ma_keys != Py_EMPTY_KEYS); assert(orig->ma_keys->dk_refcnt == 1); size_t keys_size = _PyDict_KeysSize(orig->ma_keys); @@ -1540,11 +1541,7 @@ dictresize(PyInterpreterState *interp, PyDictObject *mp, #ifdef Py_REF_DEBUG _Py_DecRefTotal(_PyInterpreterState_GET()); #endif - if (oldkeys == Py_EMPTY_KEYS) { - oldkeys->dk_refcnt--; - assert(oldkeys->dk_refcnt > 0); - } - else { + if (oldkeys != Py_EMPTY_KEYS) { assert(oldkeys->dk_kind != DICT_KEYS_SPLIT); assert(oldkeys->dk_refcnt == 1); #if PyDict_MAXFREELIST > 0 @@ -2088,8 +2085,8 @@ PyDict_Clear(PyObject *op) dictkeys_decref(interp, oldkeys); } else { - assert(oldkeys->dk_refcnt == 1); - dictkeys_decref(interp, oldkeys); + assert(oldkeys->dk_refcnt == 1 || oldkeys == Py_EMPTY_KEYS); + dictkeys_decref(interp, oldkeys); } ASSERT_CONSISTENT(mp); } @@ -3573,7 +3570,7 @@ _PyDict_SizeOf(PyDictObject *mp) } /* If the dictionary is split, the keys portion is accounted-for in the type object. */ - if (mp->ma_keys->dk_refcnt == 1) { + if (mp->ma_keys->dk_refcnt == 1 || mp->ma_keys == Py_EMPTY_KEYS) { res += _PyDict_KeysSize(mp->ma_keys); } assert(res <= (size_t)PY_SSIZE_T_MAX); From f5da08808d8cc957989a9552ccdbf13897af9acc Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 8 May 2023 08:03:48 -0600 Subject: [PATCH 04/10] Add a note. --- Objects/dictobject.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 0429912f6e0a92..083225d4ec29ed 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -300,7 +300,12 @@ _PyDict_DebugMallocStats(FILE *out) static void free_keys_object(PyInterpreterState *interp, PyDictKeysObject *keys); -// XXX Switch to Py_INCREF()? +/* We might consider modifying PyDictKeysObject so we could use + Py_INCREF() instead of dictkeys_incref() (and likewise with + Py_DECREF() and dictkeys_decref()). However, there doesn't + seem to be enough benefit (performance or otherwise) to justify + the change. */ + static inline void dictkeys_incref(PyDictKeysObject *dk) { @@ -313,7 +318,6 @@ dictkeys_incref(PyDictKeysObject *dk) dk->dk_refcnt++; } -// XXX Switch to Py_DECREF()? static inline void dictkeys_decref(PyInterpreterState *interp, PyDictKeysObject *dk) { From db8612fbb0a1c75c7a1c1f605fc7d5ac733ffe16 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 8 May 2023 13:25:49 -0600 Subject: [PATCH 05/10] Drop an old line. --- Include/internal/pycore_dict_state.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/Include/internal/pycore_dict_state.h b/Include/internal/pycore_dict_state.h index 459371a339f2d9..ece0f10ca25170 100644 --- a/Include/internal/pycore_dict_state.h +++ b/Include/internal/pycore_dict_state.h @@ -27,8 +27,6 @@ struct _Py_dict_state { uint64_t global_version; uint32_t next_keys_version; -// PyDictKeysObject empty_keys_struct; - #if PyDict_MAXFREELIST > 0 /* Dictionary reuse scheme to save calls to malloc and free */ PyDictObject *free_list[PyDict_MAXFREELIST]; From 58585a9a983002c2d76c9524d60750d32c7d69ef Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 8 May 2023 13:27:47 -0600 Subject: [PATCH 06/10] Drop an unnecessary condition. --- Objects/dictobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 083225d4ec29ed..56efeb6f1f364c 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -3574,7 +3574,7 @@ _PyDict_SizeOf(PyDictObject *mp) } /* If the dictionary is split, the keys portion is accounted-for in the type object. */ - if (mp->ma_keys->dk_refcnt == 1 || mp->ma_keys == Py_EMPTY_KEYS) { + if (mp->ma_keys->dk_refcnt == 1) { res += _PyDict_KeysSize(mp->ma_keys); } assert(res <= (size_t)PY_SSIZE_T_MAX); From 6b5c952eddb429c7cddb513884e9613dfd8c76d9 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 9 May 2023 10:11:03 -0600 Subject: [PATCH 07/10] Drop an unnecessary assert. --- Objects/dictobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 56efeb6f1f364c..9d97f9700cc632 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -2089,7 +2089,7 @@ PyDict_Clear(PyObject *op) dictkeys_decref(interp, oldkeys); } else { - assert(oldkeys->dk_refcnt == 1 || oldkeys == Py_EMPTY_KEYS); + assert(oldkeys->dk_refcnt == 1); dictkeys_decref(interp, oldkeys); } ASSERT_CONSISTENT(mp); From bdbce5f693ffdb40e5887e600a1422a98b89ce69 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 9 May 2023 10:11:42 -0600 Subject: [PATCH 08/10] Do not bother inc/dec-refing Py_EMPTY_KEYS. --- Objects/dictobject.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 9d97f9700cc632..da1527846ae4f5 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -846,7 +846,7 @@ PyObject * PyDict_New(void) { PyInterpreterState *interp = _PyInterpreterState_GET(); - dictkeys_incref(Py_EMPTY_KEYS); + /* We don't incref Py_EMPTY_KEYS here because it is immortal. */ return new_dict(interp, Py_EMPTY_KEYS, NULL, 0, 0); } @@ -1344,7 +1344,7 @@ insert_to_emptydict(PyInterpreterState *interp, PyDictObject *mp, Py_DECREF(value); return -1; } - dictkeys_decref(interp, Py_EMPTY_KEYS); + /* We don't decref Py_EMPTY_KEYS here because it is immortal. */ mp->ma_keys = newkeys; mp->ma_values = NULL; From aab493ae2e76368af63581617591caec1be874e9 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 9 May 2023 10:12:30 -0600 Subject: [PATCH 09/10] Do not update Py_Reftotal for Py_EMPTY_KEYS. --- Objects/dictobject.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index da1527846ae4f5..d72a2a649054ac 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -1542,10 +1542,10 @@ dictresize(PyInterpreterState *interp, PyDictObject *mp, // We can not use free_keys_object here because key's reference // are moved already. + if (oldkeys != Py_EMPTY_KEYS) { #ifdef Py_REF_DEBUG - _Py_DecRefTotal(_PyInterpreterState_GET()); + _Py_DecRefTotal(_PyInterpreterState_GET()); #endif - if (oldkeys != Py_EMPTY_KEYS) { assert(oldkeys->dk_kind != DICT_KEYS_SPLIT); assert(oldkeys->dk_refcnt == 1); #if PyDict_MAXFREELIST > 0 From 5aabfd77b1bba5de8b5ff1a5fbac9a6e32e82dd6 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 9 May 2023 10:24:40 -0600 Subject: [PATCH 10/10] Clarify the comment about dictkeys_incref() and dictkeys_decref(). --- Objects/dictobject.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index d72a2a649054ac..7436c113f37c4a 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -300,11 +300,12 @@ _PyDict_DebugMallocStats(FILE *out) static void free_keys_object(PyInterpreterState *interp, PyDictKeysObject *keys); -/* We might consider modifying PyDictKeysObject so we could use - Py_INCREF() instead of dictkeys_incref() (and likewise with - Py_DECREF() and dictkeys_decref()). However, there doesn't - seem to be enough benefit (performance or otherwise) to justify - the change. */ +/* PyDictKeysObject has refcounts like PyObject does, so we have the + following two functions to mirror what Py_INCREF() and Py_DECREF() do. + (Keep in mind that PyDictKeysObject isn't actually a PyObject.) + Likewise a PyDictKeysObject can be immortal (e.g. Py_EMPTY_KEYS), + so we apply a naive version of what Py_INCREF() and Py_DECREF() do + for immortal objects. */ static inline void dictkeys_incref(PyDictKeysObject *dk)