8000 Address review feedback · python/cpython@ec5956e · GitHub
[go: up one dir, main page]
Skip to content

Commit ec5956e

Browse files
DinoVDinoV
committed
Address review feedback
1 parent a8cb497 commit ec5956e

File tree

5 files changed

+58
-41
lines changed

5 files changed

+58
-41
lines changed

‎Include/cpython/pyatomic.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -463,6 +463,9 @@ _Py_atomic_load_ptr_acquire(const void *obj);
463463
static inline void
464464
_Py_atomic_store_ptr_release(void *obj, void *value);
465465

466+
static inline void
467+
_Py_atomic_store_ssize_release(Py_ssize_t *obj, Py_ssize_t value);
468+
466469
static inline void
467470
_Py_atomic_store_int_release(int *obj, int value);
468471

‎Include/cpython/pyatomic_gcc.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -491,6 +491,10 @@ static inline void
491491
_Py_atomic_store_int_release(int *obj, int value)
492492
{ __atomic_store_n(obj, value, __ATOMIC_RELEASE); }
493493

494+
static inline void
495+
_Py_atomic_store_ssize_release(Py_ssize_t *obj, Py_ssize_t value)
496+
{ __atomic_store_n(obj, value, __ATOMIC_RELEASE); }
497+
494498
static inline int
495499
_Py_atomic_load_int_acquire(const int *obj)
496500
{ return __atomic_load_n(obj, __ATOMIC_ACQUIRE); }

‎Include/cpython/pyatomic_msc.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -925,6 +925,18 @@ _Py_atomic_store_int_release(int *obj, int value)
925925
#endif
926926
}
927927

928+
static inline void
929+
_Py_atomic_store_ssize_release(Py_ssize_t *obj, Py_ssize_t value)
930+
{
931+
#if defined(_M_X64) || defined(_M_IX86)
932+
*(Py_ssize_t volatile *)obj = value;
933+
#elif defined(_M_ARM64)
934+
__stlr64((unsigned __int64 volatile *)obj, (uintptr_t)value);
935+
#else
936+
# error "no implementation of _Py_atomic_store_int_release"
937+
#endif
938+
}
939+
928940
static inline int
929941
_Py_atomic_load_int_acquire(const int *obj)
930942
{

‎Include/cpython/pyatomic_std.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -862,6 +862,14 @@ _Py_atomic_store_int_release(int *obj, int value)
862862
memory_order_release);
863863
}
864864

865+
static inline void
866+
_Py_atomic_store_ssize_release(Py_ssize_t *obj, Py_ssize_t value)
867+
{
868+
_Py_USING_STD;
869+
atomic_store_explicit((_Atomic(Py_ssize_t)*)obj, value,
870+
memory_order_release);
871+
}
872+
865873
static inline int
866874
_Py_atomic_load_int_acquire(const int *obj)
867875
{

‎Objects/dictobject.c

Lines changed: 31 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -161,14 +161,16 @@ ASSERT_DICT_LOCKED(PyObject *op)
161161
#define INCREF_KEYS(dk) _Py_atomic_add_ssize(&dk->dk_refcnt, 1)
162162
// Dec refs the keys object, giving the previous value
163163
#define DECREF_KEYS(dk) _Py_atomic_add_ssize(&dk->dk_refcnt, -1)
164+
#define LOAD_KEYS_NENTIRES(keys) _Py_atomic_load_ssize_relaxed(&keys->dk_nentries)
165+
164166
static inline void split_keys_entry_added(PyDictKeysObject *keys)
165167
{
166168
ASSERT_KEYS_LOCKED(keys);
167169

168170
// We increase before we decrease so we never get too small of a value
169171
// when we're racing with reads
170-
_Py_atomic_store_ssize(&keys->dk_nentries, keys->dk_nentries + 1);
171-
_Py_atomic_store_ssize(&keys->dk_usable, keys->dk_usable - 1);
172+
_Py_atomic_store_ssize_relaxed(&keys->dk_nentries, keys->dk_nentries + 1);
173+
_Py_atomic_store_ssize_release(&keys->dk_usable, keys->dk_usable - 1);
172174
}
173175

174176
#else /* Py_GIL_DISABLED */
@@ -181,6 +183,8 @@ static inline void split_keys_entry_added(PyDictKeysObject *keys)
181183
#define STORE_SHARED_KEY(key, value) key = value
182184
#define INCREF_KEYS(dk) dk->dk_refcnt++
183185
#define DECREF_KEYS(dk) dk->dk_refcnt--
186+
#define LOAD_KEYS_NENTIRES(keys) keys->dk_nentries
187+
184188
static inline void split_keys_entry_added(PyDictKeysObject *keys)
185189
{
186190
keys->dk_usable--;
@@ -534,7 +538,7 @@ static PyDictKeysObject empty_keys_struct = {
534538
0, /* dk_log2_index_bytes */
535539
DICT_KEYS_UNICODE, /* dk_kind */
536540
#ifdef Py_GIL_DISABLED
537-
{0}, /* dk_lock */
541+
{0}, /* dk_mutex */
538542
#endif
539543
1, /* dk_version */
540544
0, /* dk_usable (immutable) */
@@ -814,9 +818,9 @@ shared_keys_usable_size(PyDictKeysObject *keys)
814818
{
815819
#ifdef Py_GIL_DISABLED
816820
// dk_usable will decrease for each instance that is created and each
817-
// value that is added. dk_entries will increase for each value that
821+
// value that is added. dk_nentries will increase for each value that
818822
// is added. We want to always return the right value or larger.
819-
// We therefore increase dk_entries first and we decrease dk_usable
823+
// We therefore increase dk_nentries first and we decrease dk_usable
820824
// second, and conversely here we read dk_usable first and dk_entries
821825
// second (to avoid the case where we read entries before the increment
822826
// and read usable after the decrement)
@@ -835,15 +839,13 @@ new_dict_with_shared_keys(PyInterpreterState *interp, PyDictKeysObject *keys)
835839
PyDictValues *values = new_values(size);
836840
if (values == NULL) {
837841
dictkeys_decref(interp, keys);
838-
UNLOCK_KEYS(keys);
839842
return PyErr_NoMemory();
840843
}
841844
((char *)values)[-2] = 0;
842845
for (size_t i = 0; i < size; i++) {
843846
values->values[i] = NULL;
844847
}
845-
PyObject *res = new_dict(interp, keys, values, 0, 1);
846-
return res;
848+
return new_dict(interp, keys, values, 0, 1);
847849
}
848850

849851

@@ -1255,7 +1257,7 @@ insertion_resize(PyInterpreterState *interp, PyDictObject *mp, int unicode)
12551257
}
12561258

12571259
static Py_ssize_t
1258-
insert_into_dictkeys(PyDictKeysObject *keys, PyObject *name)
1260+
insert_into_splitdictkeys(PyDictKeysObject *keys, PyObject *name)
12591261
{
12601262
assert(PyUnicode_CheckExact(name));
12611263
ASSERT_KEYS_LOCKED(keys);
@@ -1290,7 +1292,7 @@ insert_into_dictkeys(PyDictKeysObject *keys, PyObject *name)
12901292

12911293
static inline int
12921294
insert_combined_dict(PyInterpreterState *interp, PyDictObject *mp,
1293-
Py_hash_t hash, PyObject *key, PyObject *value, int unicode)
1295+
Py_hash_t hash, PyObject *key, PyObject *value)
12941296
{
12951297
if (mp->ma_keys->dk_usable <= 0) {
12961298
/* Need to resize. */
@@ -1302,7 +1304,7 @@ insert_combined_dict(PyInterpreterState *interp, PyDictObject *mp,
13021304
Py_ssize_t hashpos = find_empty_slot(mp->ma_keys, hash);
13031305
dictkeys_set_index(mp->ma_keys, hashpos, mp->ma_keys->dk_nentries);
13041306

1305-
if (unicode) {
1307+
if (DK_IS_UNICODE(mp->ma_keys)) {
13061308
PyDictUnicodeEntry *ep;
13071309
ep = &DK_UNICODE_ENTRIES(mp->ma_keys)[mp->ma_keys->dk_nentries];
13081310
ep->me_key = key;
@@ -1329,14 +1331,16 @@ insert_split_dict(PyInterpreterState *interp, PyDictObject *mp,
13291331
LOCK_KEYS(keys);
13301332
if (keys->dk_usable <= 0) {
13311333
/* Need to resize. */
1332-
if (insertion_resize(interp, mp, 1) < 0) {
1333-
UNLOCK_KEYS(keys);
1334+
dictkeys_incref(keys);
1335+
int ins = insertion_resize(interp, mp, 1);
1336+
dictkeys_decref(interp, keys);
1337+
UNLOCK_KEYS(keys);
1338+
if (ins < 0) {
13341339
return -1;
13351340
}
13361341
assert(!_PyDict_HasSplitTable(mp));
13371342
assert(DK_IS_UNICODE(keys));
1338-
UNLOCK_KEYS(keys);
1339-
return insert_combined_dict(interp, mp, hash, key, value, 1);
1343+
return insert_combined_dict(interp, mp, hash, key, value);
13401344
}
13411345

13421346
Py_ssize_t hashpos = find_empty_slot(keys, hash);
@@ -1414,9 +1418,8 @@ insertdict(PyInterpreterState *interp, PyDictObject *mp,
14141418
mp->ma_keys->dk_version = 0;
14151419
assert(old_value == NULL);
14161420

1417-
int unicode = DK_IS_UNICODE(mp->ma_keys);
1418-
if (!unicode || !_PyDict_HasSplitTable(mp)) {
1419-
if (insert_combined_dict(interp, mp, hash, key, value, unicode) < 0) {
1421+
if (!_PyDict_HasSplitTable(mp)) {
1422+
if (insert_combined_dict(interp, mp, hash, key, value) < 0) {
14201423
goto Fail;
14211424
}
14221425
} else {
@@ -3597,21 +3600,7 @@ dict_equal_lock_held(PyDictObject *a, PyDictObject *b)
35973600
/* can't be equal if # of entries differ */
35983601
return 0;
35993602
/* Same # of entries -- check all of 'em. Exit early on any diff. */
3600-
Py_ssize_t n_entries;
3601-
#ifdef Py_GIL_DISABLED
3602-
if (_PyDict_HasSplitTable(a)) {
3603-
// New entries can be appended, but existing ones won't change, and
3604-
// our keys won't change because the dict is locked. So capture
3605-
// the current number of keys at entry.
3606-
LOCK_KEYS(a->ma_keys);
3607-
n_entries = a->ma_keys->dk_nentries;
3608-
UNLOCK_KEYS(a->ma_keys);
3609-
} else
3610-
#endif
3611-
{
3612-
n_entries = a->ma_keys->dk_nentries;
3613-
}
3614-
for (i = 0; i < n_entries; i++) {
3603+
for (i = 0; i < LOAD_KEYS_NENTIRES(a->ma_keys); i++) {
36153604
PyObject *key, *aval;
36163605
Py_hash_t hash;
36173606
if (DK_IS_UNICODE(a->ma_keys)) {
@@ -3825,9 +3814,8 @@ dict_setdefault_ref_lock_held(PyObject *d, PyObject *key, PyObject *default_valu
38253814
mp->ma_keys->dk_version = 0;
38263815
value = default_value;
38273816

3828-
int unicode = DK_IS_UNICODE(mp->ma_keys);
3829-
if (!unicode || !_PyDict_HasSplitTable(mp)) {
3830-
if (insert_combined_dict(interp, mp, hash, Py_NewRef(key), Py_NewRef(value), unicode) < 0) {
3817+
if (!_PyDict_HasSplitTable(mp)) {
3818+
if (insert_combined_dict(interp, mp, hash, Py_NewRef(key), Py_NewRef(value)) < 0) {
38313819
Py_DECREF(key);
38323820
Py_DECREF(value);
38333821
if (result) {
@@ -6037,11 +6025,13 @@ _PyObject_InitInlineValues(PyObject *obj, PyTypeObject *tp)
60376025
PyDictKeysObject *keys = CACHED_KEYS(tp);
60386026
assert(keys != NULL);
60396027
#ifdef Py_GIL_DISABLED
6040-
Py_ssize_t usable = keys->dk_usable;
6041-
while (usable > 1) {
6042-
if (_Py_atomic_compare_exchange_ssize(&keys->dk_usable, &usable, usable - 1)) {
6043-
break;
6028+
Py_ssize_t usable = _Py_atomic_load_ssize_relaxed(&keys->dk_usable);
6029+
if (usable > 1) {
6030+
LOCK_KEYS(keys);
6031+
if (keys->dk_usable > 1) {
6032+
_Py_atomic_store_ssize(&keys->dk_usable, keys->dk_usable - 1);
60446033
}
6034+
UNLOCK_KEYS(keys);
60456035
}
60466036
#else
60476037
if (keys->dk_usable > 1) {
@@ -6179,7 +6169,7 @@ _PyObject_StoreInstanceAttribute(PyObject *obj, PyDictValues *values,
61796169
Py_ssize_t ix = DKIX_EMPTY;
61806170
if (PyUnicode_CheckExact(name)) {
61816171
LOCK_KEYS(keys);
6182-
ix = insert_into_dictkeys(keys, name);
6172+
ix = insert_into_splitdictkeys(keys, name);
61836173
#ifdef Py_STATS
61846174
if (ix == DKIX_EMPTY) {
61856175
if (PyUnicode_CheckExact(name)) {

0 commit comments

Comments
 (0)
0