Skip to content

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Sep 15, 2025
Merged

gh-134953: Make the True/False/None check more efficient #138931

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions Lib/_pyrepl/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
ZERO_WIDTH_BRACKET = re.compile(r"\x01.*?\x02")
ZERO_WIDTH_TRANS = str.maketrans({"\x01": "", "\x02": ""})
IDENTIFIERS_AFTER = {"def", "class"}
KEYWORD_CONSTANTS = {"True", "False", "None"}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
KEYWORD_CONSTANTS = {"True", "False", "None"}
KEYWORD_CONSTANTS = frozenset({"True", "False", "None"})

Is even faster.

Copy link
Member

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'}))

Copy link

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.

Copy link
Contributor

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, so KEYWORD_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 win32
Type "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.371516299899668

Copy link
Contributor Author

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.

Copy link
Contributor

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.

BUILTINS = {str(name) for name in dir(builtins) if not name.startswith('_')}


Expand Down Expand Up @@ -196,12 +197,12 @@ def gen_colors_from_token_stream(
is_def_name = False
span = Span.from_token(token, line_lengths)
yield ColorSpan(span, "definition")
elif token.string in ("True", "False", "None"):
span = Span.from_token(token, line_lengths)
yield ColorSpan(span, "keyword_constant")
elif keyword.iskeyword(token.string):
span_cls = "keyword"
if token.string in KEYWORD_CONSTANTS:
span_cls = "keyword_constant"
span = Span.from_token(token, line_lengths)
yield ColorSpan(span, "keyword")
yield ColorSpan(span, span_cls)
if token.string in IDENTIFIERS_AFTER:
is_def_name = True
elif (
Expand Down
Loading