Skip to content

gh-74929: Make containment checks more efficient in FrameLocalsProxy#118624

Merged
ncoghlan merged 1 commit into
python:mainfrom
gaogaotiantian:frameproxy-contain
May 6, 2024
Merged

gh-74929: Make containment checks more efficient in FrameLocalsProxy#118624
ncoghlan merged 1 commit into
python:mainfrom
gaogaotiantian:frameproxy-contain

Conversation

@gaogaotiantian

@gaogaotiantian gaogaotiantian commented May 6, 2024 •
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Member

Currently the more efficient contains function is not used when someone does key in frame.f_locals because that defaults to sq_contains and it's not properly hooked. The function is only called when using frame.f_locals.__contains__(key).

This hooks sq_contains so the in operation is much more efficient.
@gaogaotiantian gaogaotiantian requested review from gvanrossum and removed request for markshannon May 6, 2024 04:01
@bedevere-app bedevere-app Bot mentioned this pull request May 6, 2024
Closed
@gaogaotiantian gaogaotiantian requested a review from markshannon May 6, 2024 04:01
@gvanrossum

gvanrossum commented May 6, 2024

Copy link
Copy Markdown
Member

Is this a correctness issue or merely a performance issue?
@gaogaotiantian

gaogaotiantian commented May 6, 2024

Copy link
Copy Markdown
Member Author

This is merely a performance issue. Without this change, the CONTAINS_OP will treat the proxy as a normal iterable and check the keys one by one. The iter() method worked fine so the result is correct. A list of keys will be generated and iterated.
@ncoghlan ncoghlan changed the title gh-74929: Make contains work properly for FrameLocalsProxy May 6, 2024
ncoghlan
ncoghlan approved these changes May 6, 2024
View reviewed changes

@ncoghlan ncoghlan left a comment •
edited
Loading

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.

LGTM! (PR title tweaked to better reflect its purpose)
@ncoghlan ncoghlan merged commit afbe5bf into python:main May 6, 2024
@gaogaotiantian gaogaotiantian deleted the frameproxy-contain branch May 6, 2024 17:28
SonicField pushed a commit to SonicField/cpython that referenced this pull request May 8, 2024
@gaogaotiantian @SonicField
pythongh-74929: Make containment checks more efficient in `FrameLocal…
e01dae6 
…sProxy` (python#118624)

Properly implement the `sq_contains` slot for frame locals proxy containment checks.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip news

3 participants

@gaogaotiantian @gvanrossum @ncoghlan