Skip to content

bpo-42197: Don't create f_locals dictionary unless we actually need it. #32055

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

markshannon
Copy link
Member

@markshannon markshannon commented Mar 22, 2022

This tiny change seems to double the speed of coverage.py https://bugs.python.org/issue42197#msg411210

Original implementation by @fabioz. I've dropped his modifications to the sys docs, as the f_locals field is now internal, so should not be accessed directly: https://github.com/python/cpython/blob/main/Doc/whatsnew/3.11.rst

https://bugs.python.org/issue42197

…ger called during profile and tracing.

 (Contributed by Fabio Zadrozny)
@gvanrossum
Copy link
Member

gvanrossum commented Mar 22, 2022

@markshannon Is there a typo in the bpo number? bpo-42917 seems to be about the size limit of the block stack.

@markshannon markshannon changed the title bpo-42917: Don't create f_locals dictionary unless we actually need it. bpo-42197: Don't create f_locals dictionary unless we actually need it. Mar 22, 2022
@markshannon markshannon added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Mar 23, 2022
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @markshannon for commit 281305b 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Mar 23, 2022
@markshannon markshannon added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Mar 23, 2022
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @markshannon for commit 0e452a1 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Mar 23, 2022
@ncoghlan
Copy link
Contributor

The breaking change here is technically in scope for the two fast locals PEPs, but given that 3.11 is breaking that API anyway by making frame objects private, I agree it makes sense to just go ahead and make this change independently of those.

+1 for PyFrame_GetLocals() as the replacement public API.

@markshannon markshannon merged commit d7163bb into python:main Mar 25, 2022
@markshannon markshannon deleted the dont-call-fasttolocals-all-the-time branch September 26, 2023 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants