gh-112075: Accessing a single element should optimistically avoid locking - #115109
gh-112075: Accessing a single element should optimistically avoid locking#115109DinoV merged 2 commits into
Conversation
Sorry, something went wrong.
| set_keys(mp, new_keys_object(interp, log2_newsize, unicode)); | ||
| if (mp->ma_keys == NULL) { | ||
| mp->ma_keys = oldkeys; | ||
| set_keys(mp, oldkeys); |
There was a problem hiding this comment.
I don't think the possible temporary assignment to NULL is thread-safe without the GIL. Let's assign the result of new_keys_object() to a temporary variable and check that before assigning it to mp->ma_keys.
Sorry, something went wrong.
There was a problem hiding this comment.
When I was looking at this function when you made a comment about it on #114741 I realized that the assignment of it to a new empty table isn't right at all! We really need to allocate the new table, copy everything over, and then we can publish it.
Sorry, something went wrong.
| if (hash == -1) | ||
| return NULL; | ||
| } | ||
| #ifdef Py_GIL_DISABLED |
There was a problem hiding this comment.
Does this differ from PyDict_Contains()? Can we just call that?
Sorry, something went wrong.
| PyObject *value; | ||
| Py_ssize_t ix; | ||
|
|
||
| #ifdef Py_GIL_DISABLED |
There was a problem hiding this comment.
This also looks like contains_known_hash
Sorry, something went wrong.
There was a problem hiding this comment.
Or does contains_known_hash look like this? 🤔
Sorry, something went wrong.
colesbury
left a comment
There was a problem hiding this comment.
LGTM
Sorry, something went wrong.
…id locking (python#115109) Makes accessing a single element thread safe and typically lock free
…id locking (python#115109) Makes accessing a single element thread safe and typically lock free
…id locking (python#115109) Makes accessing a single element thread safe and typically lock free
Makes accessing a single element thread safe.
Adds tracking for whether a dictionary is shared or not, but some of this gets duplicated in #115108.
Does not yet deal with all of the atomic assignments that need to happen to make this 100% right, that'll come in a separate PR.