Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.7k
gh-134953: Make the True/False/None check more efficient#138931
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
StanFromIreland left a comment
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.
LGTM
| ZERO_WIDTH_BRACKET=re.compile(r"\x01.*?\x02") | ||
| ZERO_WIDTH_TRANS=str.maketrans({"\x01":"","\x02":""}) | ||
| IDENTIFIERS_AFTER= {"def","class"} | ||
| KEYWORD_CONSTANTS= {"True","False","None"} |
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.
| KEYWORD_CONSTANTS= {"True","False","None"} | |
| KEYWORD_CONSTANTS=frozenset({"True","False","None"}) |
Is even faster.
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.
The compiler already does this automatically:
>>> def foo():... KEYWORD_CONSTANTS = {"True", "False", "None"}...>>> foo.__code__.co_consts(None, frozenset({'False', 'True', 'None'}))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.
The compiler only does that for function locals, right? A module variable could be mutated from other modules.
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.
@auscompgeek You are right, for module level variables the optimization is not performed, soKEYWORD_CONSTANTS stays a set. For the free-threading build the performance difference is measurable:
Python 3.15.0a0 free-threading build (heads/main-dirty:2a54acf3c3d, Sep 1 2025, 14:33:50) [MSC v.1939 64 bit (AMD64)] on win32Type "help", "copyright", "credits" or "license" for more information.>>> import timeit>>> timeit.timeit('[10 in s for ii in range(100)]', setup='from _pyrepl.utils import KEYWORD_CONSTANTS as s; f = frozenset(s)')8.864965499844402>>> timeit.timeit('[10 in f for ii in range(100)]', setup='from _pyrepl.utils import KEYWORD_CONSTANTS as s; f = frozenset(s)')8.371516299899668There 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.
@eendebakpt I'll accept a PR to wrap all global sets in_pyrepl.utils as frozensets. There's quite a few of them.
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.
I created a bit more general issue:#139003. For_pyrepl I'll make a PR.
811acc8 intopython:mainUh oh!
There was an error while loading.Please reload this page.
Thanks@ambv for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14. |
…nGH-138931)(cherry picked from commit811acc8)Co-authored-by: Łukasz Langa <lukasz@langa.pl>
GH-138939 is a backport of this pull request to the3.14 branch. |
Uh oh!
There was an error while loading.Please reload this page.