gh-141510: Optimize hash(frozendict) - #144919
Conversation
Sorry, something went wrong.
hash(frozendict) no longer creates a temporary items view and a temporary frozenset object. Copy frozenset_hash() code to frozendict_hash().
| Py_hash_t hash = FT_ATOMIC_LOAD_SSIZE_RELAXED(self->ma_hash); | ||
| if (hash != -1) { | ||
| return hash; | ||
| Py_hash_t shash = FT_ATOMIC_LOAD_SSIZE_RELAXED(self->ma_hash); |
There was a problem hiding this comment.
Why atomic operation is needed?
Sorry, something went wrong.
There was a problem hiding this comment.
PyFrozenDictObject.ma_hash is mutable and so it needs a lock to handle properly concurrent access.
Sorry, something went wrong.
There was a problem hiding this comment.
It's the same for the Unicode strings hash: the string is immutable, but the hash member is mutable. So unicode_hash() uses atomic operations to get and set the hash member.
Sorry, something went wrong.
There was a problem hiding this comment.
Ah got it
Sorry, something went wrong.
There was a problem hiding this comment.
Yeah, the hash is "lazily" computed so we need to use relaxed atomics here.
Sorry, something went wrong.
corona10
left a comment
There was a problem hiding this comment.
lgtm!
Sorry, something went wrong.
|
Merged, thanks for reviews! |
Sorry, something went wrong.
hash(frozendict) no longer creates a temporary items view and a temporary frozenset object. Copy frozenset_hash() code to frozendict_hash().
hash(frozendict) no longer creates a temporary items view and a temporary frozenset object. Copy frozenset_hash() code to frozendict_hash().
hash(frozendict) no longer creates a temporary items view and a temporary frozenset object.
Copy frozenset_hash() code to frozendict_hash().