Skip to content
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

bpo-45753: Interpreter internal tweaks #29575

Merged
merged 18 commits into from
Dec 1, 2021

Conversation

markshannon
Copy link
Member

@markshannon markshannon commented Nov 16, 2021

The overall purpose of this PR is to help future work on inter-bytecode optimization and efficient support for tracing events.

This is a bit of an ugly PR as it attempts to do a few things which are conceptually only loosely related, but are interwoven in the code.

  1. Specify a set of interpreter "registers", that is the parts of the interpreter state that are worth caching in local variables. This is nothing new, just a bit more explicit. These variables are currently:
    • frame. Pointer to the current InterpreterFrame
    • names and consts. Pointers to the names and consts tuples of the current code object. These could be merged into one in future (Merge co_names and co_consts into a single array. faster-cpython/ideas#92)
    • first_instr and next_instr. Pointers to the first and next instructions.
    • stack_pointer. The evaluation stack pointer.
  2. Make all tracing events explicit as macros, and move them into instructions where appropriate.
  3. Inline frame exit, entry and resumption where necessary to support above goals.

No meaningful change in performance

See faster-cpython/ideas#112 for motivation

https://bugs.python.org/issue45753

@markshannon
Copy link
Member Author

Skipping NEWS as this is just a refactor. There is no change to behavior, API, or performance.

@markshannon
Copy link
Member Author

The Windows failure appears to be a glitch.

@markshannon
Copy link
Member Author

@pablogsal Could you take a look at the rearrangement of the code in RETURN_VALUE and YIELD_ instructions.
Does it seem reasonable to you?

@pablogsal
Copy link
Member

@pablogsal Could you take a look at the rearrangement of the code in RETURN_VALUE and YIELD_ instructions.
Does it seem reasonable to you?

I'm currently unavailable until this Friday. Can you wait until then?

@markshannon
Copy link
Member Author

@pablogsal Could you take a look at the rearrangement of the code in RETURN_VALUE and YIELD_ instructions.
Does it seem reasonable to you?

I'm currently unavailable until this Friday. Can you wait until then?

No problem

Copy link
Member

@brandtbucher brandtbucher left a comment

Choose a reason for hiding this comment

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

Some comments while you wait for Pablo. Many are probably just for improving my own understanding of the changes here:

@bedevere-bot
Copy link

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

@markshannon markshannon added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Nov 29, 2021
@bedevere-bot
Copy link

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

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 Nov 29, 2021
Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

I have a few questions and nits but I think I understand this well enough to approve once you've replied to those -- you can take them or leave them.

@markshannon markshannon merged commit 49444fb into python:main Dec 1, 2021
@markshannon markshannon deleted the interpreter-internal-tweaks branch September 26, 2023 12:47
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.

6 participants