gh-121795: Improve performance of set membership testing from set arguments#121796
gh-121795: Improve performance of set membership testing from set arguments#121796rhettinger merged 10 commits intopython:mainfrom
Conversation
Yes :) Benchmark (on an M2 Macbook Air): Script: main branch result: my branch result: There will be no performance regression on the normal case. |
picnixz
left a comment
There was a problem hiding this comment.
Maybe like this? (for the rest, I'll leave it to Raymond)
erlend-aasland
left a comment
There was a problem hiding this comment.
This is a neat performance improvement, that contrary to most other such attempts, does not add to the complexity of the code; nice. AFAICS, this is good to go. I'll leave the landing of the PR to the code owner.
Thank you so much for your review. I have made changes to resolve the issues. |
|
We used to do something like this, but it caused bugs and had to be removed. See https://bugs.python.org/issue8757 and checkin 4d45c10 . IIRC there was also a reason that the set had to be made temporarily immutable during the lookup. I don't remember all the details now. Perhaps @serhiy-storchaka does. The original |
|
It was before I started contributing to CPython, so I have no memories about this case. The approach of this PR is better than the code used before bpo-8757. It leaves the original set key unmodified, so other threads are not affected if they only read it. The original reproducer should pass this test. But what if the set key is modified in other thread? I think this is safe if set comparison is thread-safe (and AFAIK it is). There is a new race condition: the set key can be changed during calculating its hash (currently creating a frozenset from a set is atomic, or it can be made atomic). But I do not think this is bad. We should not guarantee the correct result in such case. I think this change is safe. LGTM. |
Uh oh!
There was an error while loading. Please reload this page.