gh-112075: Accessing a single element should optimistically avoid locking#115109
Uh oh!
There was an error while loading. Please reload this page.
Conversation
DinoVforce-pushed the nogil_dict_get_threadsafe branch from 1266799 to 2f46a39CompareDinoVforce-pushed the nogil_dict_get_threadsafe branch 8 times, most recently from ef7c6ea to 54095deCompare
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Objects/dictobject.c Outdated
| 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.
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.
Objects/dictobject.c Outdated
| if (hash==-1) | ||
| returnNULL; | ||
| } | ||
| #ifdefPy_GIL_DISABLED |
There was a problem hiding this comment.
Does this differ from PyDict_Contains()? Can we just call that?
Uh oh!
There was an error while loading. Please reload this page.
| PyObject*value; | ||
| Py_ssize_tix; | ||
| #ifdefPy_GIL_DISABLED |
There was a problem hiding this comment.
This also looks like contains_known_hash
There was a problem hiding this comment.
Or does contains_known_hash look like this? 🤔
DinoVforce-pushed the nogil_dict_get_threadsafe branch from 54095de to af7ddb9Compare
DinoVforce-pushed the nogil_dict_get_threadsafe branch from af7ddb9 to d608fb3Compare
…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
2 participants
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.
dictobjects thread-safe in--disable-gilbuilds #112075