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 — bpo-42197: Don't create `f_locals` dictionary unless we actually need it. by markshannon · Pull Request #32055 · python/cpython

Conversation

@markshannon

@markshannon markshannon commented Mar 22, 2022 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@markshannon
PyFrame_FastToLocalsWithError and PyFrame_LocalsToFast are no lon…
62be414 
…ger called during profile and tracing.

 (Contributed by Fabio Zadrozny)
@bedevere-bot bedevere-bot added the awaiting core review label Mar 22, 2022
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Mar 22, 2022
@markshannon
Fix formatting of news item.
d775a87
@gvanrossum

gvanrossum commented Mar 22, 2022 •
edited by bedevere-bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

@markshannon Is there a typo in the bpo number? bpo-42917 seems to be about the size limit of the block stack.
markshannon reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@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
vstinner
vstinner reviewed Mar 22, 2022
View reviewed changes
Comment thread Misc/NEWS.d/next/Core and Builtins/2022-03-22-15-12-28.bpo-42197.SwrrFO.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@markshannon markshannon mentioned this pull request Mar 22, 2022
bpo-42197: Disable automatic update of frame locals during tracing #23028
Closed
markshannon added 6 commits March 23, 2022 11:19
@markshannon
Make accesses to a frame's f_locals safe from C code, not relying o…
4bc0707 
…n calls to `PyFrame_FastToLocals` or `PyFrame_LocalsToFast`.
@markshannon
Remove unnecessary check for CO_NEWLOCALS flag.
4cf7efa
@markshannon
Remove empty line
d41f7ea
@markshannon
Update 'what's new'
723f354
@markshannon
Use nicer formatting
02f061a
@markshannon
Fix formatting
281305b
vstinner
vstinner reviewed Mar 23, 2022
View reviewed changes
Comment thread Objects/frameobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Objects/frameobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Objects/frameobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
@markshannon markshannon added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Mar 23, 2022
@bedevere-bot

bedevere-bot commented Mar 23, 2022

Copy link
Copy Markdown

🤖 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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Mar 23, 2022
kumaraditya303
kumaraditya303 reviewed Mar 23, 2022
View reviewed changes
Comment thread Misc/NEWS.d/next/Core and Builtins/2022-03-22-15-12-28.bpo-42197.SwrrFO.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@vstinner vstinner mentioned this pull request Mar 23, 2022
bpo-46836: Add Doc/c-api/frame.rst #32051
Merged
markshannon and others added 3 commits March 23, 2022 12:26
@markshannon @kumaraditya303
Update Misc/NEWS.d/next/Core and Builtins/2022-03-22-15-12-28.bpo-421…
8d3c911 
…97.SwrrFO.rst

Co-authored-by: Kumar Aditya <[email protected]>
@markshannon
Merge branch 'main' into dont-call-fasttolocals-all-the-time
56aa001
@markshannon
Document new PyFrame_GetLocals C-API function.
0e452a1
@markshannon markshannon added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Mar 23, 2022
@bedevere-bot

bedevere-bot commented Mar 23, 2022

Copy link
Copy Markdown

🤖 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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Mar 23, 2022
@ncoghlan

ncoghlan commented Mar 25, 2022

Copy link
Copy Markdown
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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
vstinner
vstinner reviewed Mar 25, 2022
View reviewed changes
Comment thread Doc/c-api/frame.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/c-api/frame.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Misc/NEWS.d/next/Core and Builtins/2022-03-22-15-12-28.bpo-42197.SwrrFO.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
markshannon added 2 commits March 25, 2022 11:20
@markshannon
Fix up docs.
65a7904
@markshannon
Make sure that modification to frame.f_locals outside of trace functi…
c017590 
…on do not show up after tracing.
vstinner
vstinner reviewed Mar 25, 2022
View reviewed changes
Comment thread Python/sysmodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
@markshannon
Add whatsnew section for debugger access to f_locals.
70a5717
@markshannon
markshannon merged commit d7163bb into python:main Mar 25, 2022
@fabioz fabioz mannequin mentioned this pull request Apr 10, 2022
Disable automatic update of frame locals during tracing #86363
Closed
@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

Reviewers

@vstinner vstinner vstinner left review comments
@kumaraditya303 kumaraditya303 kumaraditya303 left review comments

Uh oh!

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

Assignees

No one assigned

Labels

awaiting core review

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.

7 participants

@markshannon @gvanrossum @bedevere-bot @ncoghlan @vstinner @kumaraditya303 @the-knights-who-say-ni
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.