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

Commit a22f723

Browse files
DinoVDinoV
committed
Address review feedback
1 parent 87c3446 commit a22f723

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--;
@@ -545,7 +549,7 @@ static PyDictKeysObject empty_keys_struct = {
545549
0, /* dk_log2_index_bytes */
546550
DICT_KEYS_UNICODE, /* dk_kind */
547551
#ifdef Py_GIL_DISABLED
548-
{0}, /* dk_lock */
552+
{0}, /* dk_mutex */
549553
#endif
550554
1, /* dk_version */
551555
0, /* dk_usable (immutable) */
@@ -825,9 +829,9 @@ shared_keys_usable_size(PyDictKeysObject *keys)
825829
{
826830
#ifdef Py_GIL_DISABLED
827831
// dk_usable will decrease for each instance that is created and each
828-
// value that is added. dk_entries will increase for each value that
832+
// value that is added. dk_nentries will increase for each value that
829833
// is added. We want to always return the right value or larger.
830-
// We therefore increase dk_entries first and we decrease dk_usable
834+
// We therefore increase dk_nentries first and we decrease dk_usable
831835
// second, and conversely here we read dk_usable first and dk_entries
832836
// second (to avoid the case where we read entries before the increment
833837
// and read usable after the decrement)
@@ -846,15 +850,13 @@ new_dict_with_shared_keys(PyInterpreterState *interp, PyDictKeysObject *keys)
846850
PyDictValues *values = new_values(size);
847851
if (values == NULL) {
848852
dictkeys_decref(interp, keys);
849-
UNLOCK_KEYS(keys);
850853
return PyErr_NoMemory();
851854
}
852855
((char *)values)[-2] = 0;
853856
for (size_t i = 0; i < size; i++) {
854857
values->values[i] = NULL;
855858
}
856-
PyObject *res = new_dict(interp, keys, values, 0, 1);
857-
return res;
859+
return new_dict(interp, keys, values, 0, 1);
858860
}
859861

860862

@@ -1266,7 +1268,7 @@ insertion_resize(PyInterpreterState *interp, PyDictObject *mp, int unicode)
12661268
}
12671269

12681270
static Py_ssize_t
1269-
insert_into_dictkeys(PyDictKeysObject *keys, PyObject *name)
1271+
insert_into_splitdictkeys(PyDictKeysObject *keys, PyObject *name)
12701272
{
12711273
assert(PyUnicode_CheckExact(name));
12721274
ASSERT_KEYS_LOCKED(keys);
@@ -1301,7 +1303,7 @@ insert_into_dictkeys(PyDictKeysObject *keys, PyObject *name)
13011303

13021304
static inline int
13031305
insert_combined_dict(PyInterpreterState *interp, PyDictObject *mp,
1304-
Py_hash_t hash, PyObject *key, PyObject *value, int unicode)
1306+
Py_hash_t hash, PyObject *key, PyObject *value)
13051307
{
13061308
if (mp->ma_keys->dk_usable <= 0) {
13071309
/* Need to resize. */
@@ -1313,7 +1315,7 @@ insert_combined_dict(PyInterpreterState *interp, PyDictObject *mp,
13131315
Py_ssize_t hashpos = find_empty_slot(mp->ma_keys, hash);
13141316
dictkeys_set_index(mp->ma_keys, hashpos, mp->ma_keys->dk_nentries);
13151317

1316-
if (unicode) {
1318+
if (DK_IS_UNICODE(mp->ma_keys)) {
13171319
PyDictUnicodeEntry *ep;
13181320
ep = &DK_UNICODE_ENTRIES(mp->ma_keys)[mp->ma_keys->dk_nentries];
13191321
ep->me_key = key;
@@ -1340,14 +1342,16 @@ insert_split_dict(PyInterpreterState *interp, PyDictObject *mp,
13401342
LOCK_KEYS(keys);
13411343
if (keys->dk_usable <= 0) {
13421344
/* Need to resize. */
1343-
if (insertion_resize(interp, mp, 1) < 0) {
1344-
UNLOCK_KEYS(keys);
1345+
dictkeys_incref(keys);
1346+
int ins = insertion_resize(interp, mp, 1);
1347+
dictkeys_decref(interp, keys);
1348+
UNLOCK_KEYS(keys);
1349+
if (ins < 0) {
13451350
return -1;
13461351
}
13471352
assert(!_PyDict_HasSplitTable(mp));
13481353
assert(DK_IS_UNICODE(keys));
1349-
UNLOCK_KEYS(keys);
1350-
return insert_combined_dict(interp, mp, hash, key, value, 1);
1354+
return insert_combined_dict(interp, mp, hash, key, value);
13511355
}
13521356

13531357
Py_ssize_t hashpos = find_empty_slot(keys, hash);
@@ -1425,9 +1429,8 @@ insertdict(PyInterpreterState *interp, PyDictObject *mp,
14251429
mp->ma_keys->dk_version = 0;
14261430
assert(old_value == NULL);
14271431

1428-
int unicode = DK_IS_UNICODE(mp->ma_keys);
1429-
if (!unicode || !_PyDict_HasSplitTable(mp)) {
1430-
if (insert_combined_dict(interp, mp, hash, key, value, unicode) < 0) {
1432+
if (!_PyDict_HasSplitTable(mp)) {
1433+
if (insert_combined_dict(interp, mp, hash, key, value) < 0) {
14311434
goto Fail;
14321435
}
14331436
} else {
@@ -3608,21 +3611,7 @@ dict_equal_lock_held(PyDictObject *a, PyDictObject *b)
36083611
/* can't be equal if # of entries differ */
36093612
return 0;
36103613
/* Same # of entries -- check all of 'em. Exit early on any diff. */
3611-
Py_ssize_t n_entries;
3612-
#ifdef Py_GIL_DISABLED
3613-
if (_PyDict_HasSplitTable(a)) {
3614-
// New entries can be appended, but existing ones won't change, and
3615-
// our keys won't change because the dict is locked. So capture
3616-
// the current number of keys at entry.
3617-
LOCK_KEYS(a->ma_keys);
3618-
n_entries = a->ma_keys->dk_nentries;
3619-
UNLOCK_KEYS(a->ma_keys);
3620-
} else
3621-
#endif
3622-
{
3623-
n_entries = a->ma_keys->dk_nentries;
3624-
}
3625-
for (i = 0; i < n_entries; i++) {
3614+
for (i = 0; i < LOAD_KEYS_NENTIRES(a->ma_keys); i++) {
36263615
PyObject *key, *aval;
36273616
Py_hash_t hash;
36283617
if (DK_IS_UNICODE(a->ma_keys)) {
@@ -3836,9 +3825,8 @@ dict_setdefault_ref_lock_held(PyObject *d, PyObject *key, PyObject *default_valu
38363825
mp->ma_keys->dk_version = 0;
38373826
value = default_value;
38383827

3839-
int unicode = DK_IS_UNICODE(mp->ma_keys);
3840-
if (!unicode || !_PyDict_HasSplitTable(mp)) {
3841-
if (insert_combined_dict(interp, mp, hash, Py_NewRef(key), Py_NewRef(value), unicode) < 0) {
3828+
if (!_PyDict_HasSplitTable(mp)) {
3829+
if (insert_combined_dict(interp, mp, hash, Py_NewRef(key), Py_NewRef(value)) < 0) {
38423830
Py_DECREF(key);
38433831
Py_DECREF(value);
38443832
if (result) {
@@ -6048,11 +6036,13 @@ _PyObject_InitInlineValues(PyObject *obj, PyTypeObject *tp)
60486036
PyDictKeysObject *keys = CACHED_KEYS(tp);
60496037
assert(keys != NULL);
60506038
#ifdef Py_GIL_DISABLED
6051-
Py_ssize_t usable = keys->dk_usable;
6052-
while (usable > 1) {
6053-
if (_Py_atomic_compare_exchange_ssize(&keys->dk_usable, &usable, usable - 1)) {
6054-
break;
6039+
Py_ssize_t usable = _Py_atomic_load_ssize_relaxed(&keys->dk_usable);
6040+
if (usable > 1) {
6041+
LOCK_KEYS(keys);
6042+
if (keys->dk_usable > 1) {
6043+
_Py_atomic_store_ssize(&keys->dk_usable, keys->dk_usable - 1);
60556044
}
6045+
UNLOCK_KEYS(keys);
60566046
}
60576047
#else
60586048
if (keys->dk_usable > 1) {
@@ -6176,7 +6166,7 @@ _PyObject_StoreInstanceAttribute(PyObject *obj, PyDictValues *values,
61766166
Py_ssize_t ix = DKIX_EMPTY;
61776167
if (PyUnicode_CheckExact(name)) {
61786168
LOCK_KEYS(keys);
6179-
ix = insert_into_dictkeys(keys, name);
6169+
ix = insert_into_splitdictkeys(keys, name);
61806170
#ifdef Py_STATS
61816171
if (ix == DKIX_EMPTY) {
61826172
if (PyUnicode_CheckExact(name)) {

0 commit comments

Comments
 (0)
0