GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-134953: Make the True/False/None check more efficient by ambv · Pull Request #138931 · python/cpython

Conversation

@ambv

@ambv ambv commented Sep 15, 2025 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor
  • Issue: Add more granularity to _pyrepl syntax theming for "keywords" #134953

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@ambv
ambv requested review from lysnikolaou and pablogsal as code owners September 15, 2025 15:12
@bedevere-app bedevere-app Bot added the awaiting core review label Sep 15, 2025
@bedevere-app bedevere-app Bot mentioned this pull request Sep 15, 2025
Add more granularity to _pyrepl syntax theming for "keywords" #134953
Closed
@ambv
pythongh-134953: Make the True/False/None check more efficient
8e85d33
@ambv
ambv force-pushed the gh-134953-followup branch from 04f98d4 to 8e85d33 Compare September 15, 2025 15:13
@ambv ambv added the skip news label Sep 15, 2025
StanFromIreland
StanFromIreland approved these changes Sep 15, 2025
View reviewed changes

@StanFromIreland StanFromIreland left a comment

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
eendebakpt
eendebakpt reviewed Sep 15, 2025
View reviewed changes
Comment thread Lib/_pyrepl/utils.py
ZERO_WIDTH_BRACKET = re.compile(r"\x01.*?\x02")
ZERO_WIDTH_TRANS = str.maketrans({"\x01": "", "\x02": ""})
IDENTIFIERS_AFTER = {"def", "class"}
KEYWORD_CONSTANTS = {"True", "False", "None"}

@eendebakpt eendebakpt Sep 15, 2025

Copy link
Copy Markdown
Contributor

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.

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

Is even faster.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@pablogsal pablogsal Sep 15, 2025

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@auscompgeek auscompgeek Sep 16, 2025

Copy link
Copy Markdown
Contributor

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@eendebakpt eendebakpt Sep 16, 2025

Copy link
Copy Markdown
Contributor

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, 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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
ambv and efimov-mikhail reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

@ambv ambv Sep 16, 2025

Copy link
Copy Markdown
Contributor Author

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.

@eendebakpt I'll accept a PR to wrap all global sets in _pyrepl.utils as frozensets. There's quite a few of them.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
StanFromIreland reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@eendebakpt eendebakpt Sep 16, 2025

Copy link
Copy Markdown
Contributor

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@ambv
ambv merged commit 811acc8 into python:main Sep 15, 2025
@bedevere-app bedevere-app Bot removed the awaiting core review label Sep 15, 2025
@ambv ambv added the needs backport to 3.14 bugs and security fixes label Sep 15, 2025
@miss-islington-app

miss-islington-app Bot commented Sep 15, 2025

Copy link
Copy Markdown

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 15, 2025
@ambv @miss-islington
pythongh-134953: Make the True/False/None check more efficient (pytho…
569db03 
…nGH-138931)

(cherry picked from commit 811acc8)

Co-authored-by: Łukasz Langa <[email protected]>
@bedevere-app

bedevere-app Bot commented Sep 15, 2025

Copy link
Copy Markdown

GH-138939 is a backport of this pull request to the 3.14 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the needs backport to 3.14 bugs and security fixes label Sep 15, 2025
@eendebakpt eendebakpt mentioned this pull request Sep 16, 2025
gh-139003: Use frozenset for module level attributes in _pyrepl.utils #139004
Merged
pablogsal pushed a commit that referenced this pull request Sep 29, 2025
@miss-islington
[3.14] gh-134953: Make the True/False/None check more efficient (GH-1…
4372a05 
…38931) (#138939)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@eendebakpt eendebakpt eendebakpt left review comments
@StanFromIreland StanFromIreland StanFromIreland approved these changes
@pablogsal pablogsal Awaiting requested review from pablogsal pablogsal is a code owner
@lysnikolaou lysnikolaou Awaiting requested review from lysnikolaou lysnikolaou is a code owner
+1 more reviewer
@auscompgeek auscompgeek auscompgeek left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@ambv @auscompgeek @eendebakpt @pablogsal @StanFromIreland
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.