FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

bpo-46841: Use *inline* caching for `BINARY_OP` by brandtbucher · Pull Request #31543 · python/cpython · GitHub

/ cpython Public

bpo-46841: Use *inline* caching for BINARY_OP - #31543

Merged
markshannon merged 8 commits into
python:mainfrom
brandtbucher:inline-caches
Feb 25, 2022
ConversationCommits (8)ChecksFiles changed
Merged

bpo-46841: Use *inline* caching for BINARY_OP#31543
markshannon merged 8 commits into
python:mainfrom
brandtbucher:inline-caches

Conversation

brandtbucher commented Feb 24, 2022 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

Here's a sort-of-rough first pass at implementing this idea. BINARY_OP is easy to start with, since it only uses its cache for the adaptive counter member. Initial perf testing doesn't show any impact, which I think is good (it means that the basic idea works well, even when the cache isn't being used).

Related changes:

  • All of the dis utilities grow a show_caches parameter, defaulting to False.
  • wordcode_helpers.h has been moved into compile.c.

https://bugs.python.org/issue46841

brandtbucher requested review from a team and markshannon as code owners February 24, 2022 02:18
markshannon reviewed Feb 24, 2022
View reviewed changes
Comment thread Python/ceval.c Outdated
markshannon reviewed Feb 24, 2022
View reviewed changes
Comment thread Include/internal/pycore_code.h Outdated
markshannon reviewed Feb 24, 2022
View reviewed changes
Comment thread Python/specialize.c Outdated
markshannon reviewed Feb 24, 2022
View reviewed changes
Comment thread Python/ceval.c Outdated
markshannon reviewed Feb 24, 2022
View reviewed changes
Comment thread Doc/library/dis.rst
Use 2 bytes for each instruction. Previously the number of bytes varied
by instruction.

.. versionchanged:: 3.11

markshannon Feb 24, 2022

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Maybe don't document this until we are sure it's what we want?

brandtbucher Feb 24, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I'd prefer to keep it, and change it if/when dis changes (which is easy enough). That way we don't forget to document it.

markshannon Feb 25, 2022

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

OK. Once we've started doing this we will need to complete it before the beta release anyway.

markshannon commented Feb 24, 2022

Copy link
Copy Markdown
Member

Looks good in general.
I've commented on https://bugs.python.org/issue46841 about what interface we want to add to the dis module.

brandtbucher added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Feb 24, 2022

bedevere-bot commented Feb 24, 2022

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @brandtbucher for commit 1aa079c 🤖

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

bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Feb 24, 2022

markshannon commented Feb 25, 2022

Copy link
Copy Markdown
Member

👍
Let the process of inlining the caches commence

markshannon merged commit 0f41aac into python:main Feb 25, 2022
asvetlov pushed a commit that referenced this pull request Feb 26, 2022
bpo-46841: Use *inline* caching for BINARY_OP (GH-31543)
404bca3
brandtbucher mentioned this pull request Mar 2, 2022
Closed
26 tasks
brandtbucher mentioned this pull request Apr 10, 2022
Closed
brandtbucher deleted the inline-caches branch July 21, 2022 20:14
ezio-melotti mentioned this pull request Dec 9, 2022
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

markshannon Awaiting requested review from markshannon markshannon is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL