◐ Shell
reader mode source ↗
Skip to content

bpo-44590: Lazily allocate frame objects#27077

Merged
markshannon merged 44 commits into
python:mainfrom
faster-cpython:lazy-frame
Jul 26, 2021
Merged

bpo-44590: Lazily allocate frame objects#27077
markshannon merged 44 commits into
python:mainfrom
faster-cpython:lazy-frame

Conversation

@markshannon

@markshannon markshannon commented Jul 9, 2021 •
edited
Loading

Copy link
Copy Markdown
Member

This PR:

  1. Moves all essential frame data to the (per-thread) stack
  2. Lazily allocates heap frame objects only when needed.

For most calls to Python functions, we can get the necessary memory for a stack frame by just bumping a pointer.
When we do need a frame object, for tracebacks and the like, it is lazily allocated.

According to my benchmarking run this PR produces a maximum speedup of 31% with a mean of 7%!

The performance numbers are suspiciously good.
If someone could run the comparison on their machine that would be much appreciated.

https://bugs.python.org/issue44590

markshannon added 30 commits June 21, 2021 18:50
@markshannon
Turn specials array into struct and add 'lasti' to it.
79aeaf6
@markshannon
Move stack-depth from frame object to specials struct.
3b6a4e8
@markshannon
Rename 'specials' to 'frame' as it now includes most of the data for … …
6f0ba40 
…a frame, except for the locals.
@markshannon
Refactor, pushing PyFrame upward toward _PyEval_Vector.
8543c27
@markshannon
Add pointer from stack frame to frame object and rename tstate.frame … …
423d403 
…to tstate.pyframe as preparation for lazily created frames.
@markshannon
More refactoring. Add tstate.frame for the top stack frame.
7bf6c30
@markshannon
Convert use of PyFrameObject to _PyFrame.
861a8d9
@markshannon
Replace most remaining uses frame object in the interpreter with stac… …
35e793c 
…k frame.
@markshannon
Convert more uses of frameobject to frame.
192094e
@markshannon
Move f_state from frame object to frame.
3f601a7
@markshannon
Compute f_back when on thread stack, only filling in value when frame… …
bd95c32 
… object outlives stack invocation.
@markshannon
Add NULL check
9961abc
@markshannon
Get lazy f_back working (it still leaks).
32af707
@markshannon
Use frames not frameobjects in sys._getframe()
ac7dbe8
@markshannon
NULL out frame->previous when leaving frame.
f33d291
@markshannon
Frames now include nlocalspuls, so they have valid layout after code … …
5c23a36 
…has been deleted.
@markshannon
Move ownership of frame in generator from frame object ot generator o… …
910e991 
…bject.
@markshannon
Remove localsptr field from frame object.
22e1c9b
@markshannon
Add new _PyEval_EvalNoFrame function for evaluating frames directly.
f84a3f0
@markshannon
Allow for lazily created frames.
1180a44
@markshannon
Do not create frame objects for Python calls.
c76de89
@markshannon
Don't create frame objects for generators.
15aeef1
@markshannon
Fix memory leak
1d2e1ce
@markshannon
Merge branch 'main' into lazy-frame-updated
1b19f8b
@markshannon
Restore support for PEP 523.
d619bae
@markshannon
Streamline pushing and popping stack frames a bit.
d147d03
@markshannon
Merge branch 'main' into lazy-frame
25c6a71
@markshannon
Add f_ prefix back to several frame fields to ease porting C code tha… …
618b094 
…t introspects frames.
@markshannon
Add NEWS
e5da338
@markshannon
Remove debugging artifact.
dda0b0c
93 hidden items Load more…
@markshannon markshannon requested a review from a team as a code owner July 21, 2021 12:26
pablogsal
pablogsal approved these changes Jul 22, 2021
View reviewed changes

@pablogsal pablogsal left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hide comment

LGTM Great job!

Let's run the buildbots and land!

@markshannon markshannon added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 26, 2021
@bedevere-bot

bedevere-bot commented Jul 26, 2021

Copy link
Copy Markdown

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

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 Jul 26, 2021
@markshannon

markshannon commented Jul 26, 2021

Copy link
Copy Markdown
Member Author

The failures on buildbot/AMD64 Windows10 PR look like something is wrong with that machine.

@markshannon markshannon merged commit ae0a2b7 into python:main Jul 26, 2021
@ncoghlan

ncoghlan commented Aug 1, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

One consequence of this change is that the code is now quite confusing, since the term "frame" is used ambiguously to refer to both frame objects and interpreter frames (specifically, the f_frame field on frame objects is now an interpreter frame reference, while tb_frame on tracebacks is still a full frame object).

@ncoghlan

ncoghlan commented Aug 1, 2021

Copy link
Copy Markdown
Contributor

The generator code uses gi_xframe rather than gi_frame to make the distinction a bit clearer, and I think doing that systematically (so there is consistent code level distinction between execution frames and introspection frames) would make the new code much easier to follow.

@ncoghlan

ncoghlan commented Aug 1, 2021

Copy link
Copy Markdown
Contributor

I've posted a proposed internal API refactoring design here: https://bugs.python.org/issue44800

@ncoghlan

ncoghlan commented Aug 1, 2021

Copy link
Copy Markdown
Contributor

And the PR implementing that proposed refactoring is up at #27525

@markshannon markshannon deleted the lazy-frame branch September 15, 2021 11:51
@brandtbucher brandtbucher mentioned this pull request Dec 15, 2021
Closed
@markshannon markshannon mentioned this pull request Apr 21, 2022
Closed
@The-Compiler The-Compiler mentioned this pull request May 26, 2022
Closed
@kumaraditya303 kumaraditya303 mentioned this pull request Jun 6, 2022
Merged
@andersk andersk mentioned this pull request Jun 28, 2023
Closed
@jburgy jburgy mentioned this pull request Oct 24, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@merwok merwok merwok left review comments

@Fidget-Spinner Fidget-Spinner Fidget-Spinner left review comments

@pablogsal pablogsal pablogsal approved these changes

@1st1 1st1 Awaiting requested review from 1st1

@gvanrossum gvanrossum Awaiting requested review from gvanrossum

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@markshannon @pablogsal @bedevere-bot @Fidget-Spinner @gvanrossum @ncoghlan @merwok @the-knights-who-say-ni