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-46841: Use *inline* caching for `BINARY_OP` by brandtbucher · Pull Request #31543 · python/cpython

Conversation

@brandtbucher

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

Uh oh!

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
brandtbucher added 5 commits February 23, 2022 14:51
@brandtbucher
Use inline caching for BINARY_OP instructions
42c9907
@brandtbucher
Bump the magic number
a446858
@brandtbucher
Fixup
0393b0d
@brandtbucher
*facepalm*
8fd41b7
@brandtbucher
blurb add
4cc3467
@brandtbucher
brandtbucher requested review from a team and markshannon as code owners February 24, 2022 02:18
@bedevere-bot bedevere-bot added the awaiting core review label Feb 24, 2022
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Feb 24, 2022
markshannon
markshannon reviewed Feb 24, 2022
View reviewed changes
Comment thread Python/ceval.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
markshannon
markshannon reviewed Feb 24, 2022
View reviewed changes
Comment thread Include/internal/pycore_code.h Resolved

Uh oh!

There was an error while loading. Please reload this page.
markshannon
markshannon reviewed Feb 24, 2022
View reviewed changes
Comment thread Python/specialize.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
markshannon
markshannon reviewed Feb 24, 2022
View reviewed changes
Comment thread Python/ceval.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
markshannon
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 markshannon Feb 24, 2022

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

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

@brandtbucher brandtbucher Feb 24, 2022

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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.

Sorry, something went wrong.

Uh oh!

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

@markshannon markshannon Feb 25, 2022

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@markshannon

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
brandtbucher added 2 commits February 24, 2022 11:36
@brandtbucher
Use dedicated cache structures
a59d97c
@brandtbucher
Catch up with main
6b0e5ba
@brandtbucher
brandtbucher requested a review from markshannon February 24, 2022 20:05
@brandtbucher
Use a run-time assert instead of a build-time one
1aa079c
@brandtbucher brandtbucher added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Feb 24, 2022
@bedevere-bot

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.
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 Feb 24, 2022
@markshannon

markshannon commented Feb 25, 2022

Copy link
Copy Markdown
Member

👍
Let the process of inlining the caches commence
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@markshannon
markshannon merged commit 0f41aac into python:main Feb 25, 2022
@bedevere-bot bedevere-bot removed the awaiting core review label Feb 25, 2022
asvetlov pushed a commit that referenced this pull request Feb 26, 2022
@brandtbucher @asvetlov
bpo-46841: Use *inline* caching for BINARY_OP (GH-31543)
404bca3
@brandtbucher brandtbucher mentioned this pull request Mar 2, 2022
Inline caching faster-cpython/ideas#310
Closed
21 of 26 tasks
@brandtbucher brandtbucher mentioned this pull request Apr 10, 2022
Inline bytecode caches #90997
Closed
@brandtbucher
brandtbucher deleted the inline-caches branch July 21, 2022 20:14
@ezio-melotti ezio-melotti mentioned this pull request Dec 9, 2022
Fix links to missing CPython files python/devguide#1003
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@markshannon markshannon Awaiting requested review from markshannon markshannon is a code owner

Uh oh!

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

Assignees

No one assigned

Labels

None yet

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.

4 participants

@brandtbucher @markshannon @bedevere-bot @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.