Skip to content

gh-132657: optimize PySet_Contains for frozensets - #141183

Merged
kumaraditya303 merged 2 commits into
python:mainfrom
kumaraditya303:set-optimize
Nov 11, 2025
Merged

gh-132657: optimize PySet_Contains for frozensets#141183
kumaraditya303 merged 2 commits into
python:mainfrom
kumaraditya303:set-optimize

Conversation

@kumaraditya303

@kumaraditya303kumaraditya303 commented Nov 7, 2025•
edited
Loading

Copy link
Copy Markdown
Contributor

Avoid taking critical section if the object is an exact frozenset.

@bedevere-appbedevere-appBot mentioned this pull request Nov 7, 2025
Closed
Carreau
Carreau reviewed Nov 7, 2025
View reviewed changes
Comment threadObjects/setobject.c Outdated
eendebakpt
eendebakpt approved these changes Nov 7, 2025
View reviewed changes

@eendebakpteendebakpt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Also see #132290, which also removes locks for set arguments to PySet_Contains.

@Carreau

Carreau commented Nov 7, 2025

Copy link
Copy Markdown
Contributor

Do you want to include a variation of my proposed test in testcapi/set.c ? With the proper Null checks and Pydecref of course. This will avoid regressions.

@kumaraditya303

kumaraditya303 commented Nov 11, 2025

Copy link
Copy Markdown
ContributorAuthor

Do you want to include a variation of my proposed test in testcapi/set.c ? With the proper Null checks and Pydecref of course. This will avoid regressions.

Would you like to propose a PR for that? That could be backported as well but since this PR is perf optimization it won't be backported.

@kumaraditya303
kumaraditya303 merged commit 7211a34 into python:mainNov 11, 2025
44 checks passed
@kumaraditya303
kumaraditya303 deleted the set-optimize branch November 11, 2025 14:32
Carreau pushed a commit to Carreau/cpython that referenced this pull request Nov 11, 2025
User
Add regression test for PySet_Contains
c518806
As discussed in pythongh-141183, the test suite did not
test that PySet_Contains does not convert unhashable key
into a frozenset.
This commit adds a regression test for this
behavior, to ensure that any behavior change is
caught by the test suite.
Carreau added a commit to Carreau/cpython that referenced this pull request Nov 11, 2025
@CarreauUser
Add regression test for PySet_Contains
24b39e7
As discussed in pythongh-141183, the test suite did not
test that PySet_Contains does not convert unhashable key
into a frozenset.
This commit adds a regression test for this
behavior, to ensure that any behavior change is
caught by the test suite.
Carreau added a commit to Carreau/cpython that referenced this pull request Nov 11, 2025
@Carreau
Add regression test for PySet_Contains
a9af13f
As discussed in pythongh-141183, the test suite did not
test that PySet_Contains does not convert unhashable key
into a frozenset.
This commit adds a regression test for this
behavior, to ensure that any behavior change is
caught by the test suite.
@CarreauCarreau mentioned this pull request Nov 11, 2025
Merged
@Carreau

Carreau commented Nov 11, 2025•
edited
Loading

Copy link
Copy Markdown
Contributor

Done as #141411

Carreau added a commit to Carreau/cpython that referenced this pull request Nov 11, 2025
@Carreau
Add regression test for PySet_Contains
9d02192
As discussed in pythongh-141183, the test suite did not
test that PySet_Contains does not convert unhashable key
into a frozenset.
This commit adds a regression test for this
behavior, to ensure that any behavior change is
caught by the test suite.
@naschemenascheme added the needs backport to 3.14 bugs and security fixes label Nov 19, 2025
@miss-islington-app

miss-islington-appBot commented Nov 19, 2025

Copy link
Copy Markdown

Thanks @kumaraditya303 for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 19, 2025
@kumaraditya303@miss-islington
pythongh-132657: optimizePySet_Containsforfrozenset(pythonGH-…
9d1cfc1
…141183)
(cherry picked from commit 7211a34)
Co-authored-by: Kumar Aditya <kumaraditya@python.org>
@bedevere-app

bedevere-appBot commented Nov 19, 2025

Copy link
Copy Markdown

GH-141773 is a backport of this pull request to the 3.14 branch.

@bedevere-appbedevere-appBot removed the needs backport to 3.14 bugs and security fixes label Nov 19, 2025
nascheme pushed a commit that referenced this pull request Nov 20, 2025
@miss-islington@kumaraditya303
[3.14]gh-132657: optimizePySet_Containsforfrozenset(GH-141183)…
072eeaf
… (gh-141773)
(cherry picked from commit 7211a34)
Co-authored-by: Kumar Aditya <kumaraditya@python.org>
StanFromIreland pushed a commit to StanFromIreland/cpython that referenced this pull request Dec 6, 2025
@kumaraditya303@StanFromIreland
pythongh-132657: optimizePySet_Containsforfrozenset(python#14…
0049b04
…1183)
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@eendebakpteendebakpteendebakpt approved these changes
@rhettingerrhettingerAwaiting requested review from rhettingerrhettinger is a code owner
+1 more reviewer
@CarreauCarreauCarreau approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

skip newstopic-free-threading

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kumaraditya303@Carreau@eendebakpt@nascheme