-
-
Notifications
You must be signed in to change notification settings - Fork 32.6k
gh-132657: Avoid locks and refcounts in frozenset operations #136107
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
return NULL; | ||
if (table != so->table || entry->key != startkey) | ||
return set_lookkey(so, key, hash); | ||
if (frozenset) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: the special case for unicode a couple of lines above was introduced to avoid the check on mutated tables. See commit eendebakpt@93035c4 that moved the code inline.
With the check for an exact frozenset we avoid the same checks.
There is also GH-132290 which I think would do similar things. It's missing the refcount optimizations that this PR has. And it provides lock-free contains for both set and frozenset. |
I think the other PR now also has the refcount optimizations. So if that PR works, we can close this one. |
I think we should get this in first as the other PR is much larger and will take time. |
See the corresponding issue for the rationale.
The PR improves performance of the frozenset operations under the FT build. A benchmark:
Script
Results on main (interleaved benchmark)
Results on PR (interleaved benchmark)
The benchmark "frozenset" is improved since this triggers the path taken by the adaptive interpreter for
z in s
.The two dunder benchmarks use
s.__contains__(z)
.The FT scaling performance is not improved a lot. This is probably due to refcount contention on the global objects (the set and frozenset). Because the operation itself is faster, the scaling performance itself looks worse.
Script adapted from the ftscalingbench.py
Results on main
Results on PR