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 — GH-91095: Specialize calls to normal python classes by markshannon · Pull Request #93221 · python/cpython

Conversation

@markshannon

@markshannon markshannon commented May 25, 2022 •
edited by bedevere-bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Performance impact is small, but positive

  • Issue: Specialize calls for Python classes #91095

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
markshannon added 15 commits December 13, 2021 14:33
@markshannon
Spacialize calls to normal Python classes.
158010e
@markshannon
Merge branch 'main' into specialize-calls-to-normal-python-classes
4f74e30
@markshannon
Merge branch 'main' into specialize-calls-to-normal-python-classes
a62f708
@markshannon
Don't change magic number.
c607dfe
@markshannon
Merge branch 'main' into specialize-calls-to-normal-python-classes
b9ee4f9
@markshannon
Add news
e6002c4
@markshannon
Merge branch 'main' into specialize-calls-to-normal-python-classes
2254d06
@markshannon
Merge branch 'main' into specialize-calls-to-normal-python-classes
2f32fcd
@markshannon
Fix line table for init cleanup function.
697bc4f
@markshannon
Fix refleaks
c6f8f8e
@markshannon
Add news
5edb423
@markshannon
Update summarize stats script.
7166184
@markshannon
Fix stats for inlined py calls
3260c42
@markshannon
Merge branch 'main' into specialize-calls-to-normal-python-classes
641c46e
@markshannon
Remove duplicate news item.
6749242
@markshannon
markshannon requested a review from methane as a code owner May 25, 2022 15:27
@bedevere-bot bedevere-bot added the awaiting core review label May 25, 2022
@markshannon
markshannon requested a review from Fidget-Spinner May 25, 2022 15:27
@markshannon
Tidy up
426b5b3
kumaraditya303
kumaraditya303 reviewed May 26, 2022
View reviewed changes
Comment thread Python/ceval.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
kumaraditya303
kumaraditya303 reviewed May 26, 2022
View reviewed changes
Comment thread Python/specialize.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Fidget-Spinner
Fidget-Spinner reviewed May 26, 2022
View reviewed changes

@Fidget-Spinner Fidget-Spinner left a comment •
edited
Loading

Uh oh!

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

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.

I get the rough idea of this, and to save some time for the other reviewers, here's how it goes:

Frames before a Python class instantiation
__main__ -> NULL

Frames when a Python class is instantiated:
__main__ -> shim -> __init__ -> NULL
(Shim is responsible for replacing None returned by __init__ with self. It contains a special instruction EXIT_INIT_CHECK to verify the None.)

I'm not confident that I can catch any subtle errors with this much code. Let's test this with buildbots.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@markshannon
Fix decrefs
e70711a
@markshannon markshannon added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label May 27, 2022
@bedevere-bot

bedevere-bot commented May 27, 2022

Copy link
Copy Markdown

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

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 May 27, 2022
markshannon added 3 commits May 27, 2022 13:50
@markshannon
Merge branch 'main' into specialize-calls-to-normal-python-classes
70e3e06
@markshannon
Make it explicit that shim frame is artificial and should not show up…
4a58e68 
… in tracebacks or sys._getframe()
@markshannon
Fixup sys._getframe
7c0876c
@markshannon markshannon added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label May 27, 2022
Fidget-Spinner
Fidget-Spinner approved these changes Jun 2, 2022
View reviewed changes

@Fidget-Spinner Fidget-Spinner left a comment

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.

The code LGTM. But whether the perf improvement is worth the +300 lines of code I don't feel so sure. You'll have to decide that for yourself.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Python/ceval.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels Jun 2, 2022
sweeneyde
sweeneyde reviewed Jun 3, 2022
View reviewed changes
Comment thread Python/ceval.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
markshannon added 3 commits June 7, 2022 11:45
@markshannon
Add explanatory comment to CALL_NO_KW_ALLOC_AND_ENTER_INIT.
365fceb
@markshannon
Add test for tracing when quickening calls to Python classes.
1ee2f1c
@markshannon
Merge branch 'main' into specialize-calls-to-normal-python-classes
b75b1aa
@markshannon markshannon added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jun 7, 2022
@bedevere-bot

bedevere-bot commented Jun 7, 2022

Copy link
Copy Markdown

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

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 Jun 7, 2022
markshannon added 10 commits June 20, 2022 18:35
@markshannon
Merge branch 'main' into specialize-calls-to-normal-python-classes
224c4ac
@markshannon
Merge branch 'main' into specialize-calls-to-normal-python-classes
f368a30
@markshannon
Remove is_artificial and use _co_firsttraceable instead.
b5cf97d
@markshannon
Merge branch 'main' into specialize-calls-to-normal-python-classes
ba83e76
@markshannon
Explain unusual structure of _Py_InitCleanupFunc code.
8a5c7e6
@markshannon
Merge branch 'main' into specialize-calls-to-normal-python-classes
80444e4
@markshannon
Do not include incomplete frames in tracebacks.
cd41308
@markshannon
Make init-cleanup function per-interpreter, rather than global.
9ffd10d
@markshannon
Merge branch 'main' into specialize-calls-to-normal-python-classes
3645ea4
@markshannon
Fix whitespace
255e1c1
iritkatriel
iritkatriel reviewed Jul 26, 2022
View reviewed changes
Comment thread Lib/opcode.py
def_op('UNARY_NOT', 12)

def_op('UNARY_INVERT', 15)
def_op('EXIT_INIT_CHECK', 16)

@iritkatriel iritkatriel Jul 26, 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.

This needs to be added to the opcodes list in the docs.

Sorry, something went wrong.

Uh oh!

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

@markshannon markshannon Jul 26, 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 rather not. It shouldn't appear in any disassembly or traceback.
Once we have some sort of assembler, I want to replace

    EXIT_INIT_CHECK
    RETURN_VALUE

with

    POP_JUMP_IF_NOT_NONE error
    RETURN_VALUE
error:
    PUSH_NULL
    LOAD_CONST TypeError
    LOAD_CONST "__init__() should not return a value"
    CALL 1
    RAISE_VARARGS 1

So I expect it to removed before 3.12beta.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@markshannon markshannon mentioned this pull request Aug 15, 2022
Handle long tail of specialization for LOAD_ATTR and CALL with trampolines. faster-cpython/ideas#447
Open
markshannon added 3 commits August 24, 2022 13:24
@markshannon
Merge branch 'main' into specialize-calls-to-normal-python-classes
72e50e0
@markshannon
Merge branch 'main' into specialize-calls-to-normal-python-classes
5b94ad6
@markshannon
Use code object instead of function as trampoline.
ff3ffef
@markshannon markshannon mentioned this pull request Oct 20, 2022
GH-96421: Insert shim frame on entry to interpreter #96319
Merged
@markshannon markshannon mentioned this pull request Nov 10, 2022
GH-91095: Specialize calls to normal Python classes. #99331
Merged
@markshannon

markshannon commented Nov 10, 2022

Copy link
Copy Markdown
Member Author

There were too many conflicts after the many changes to the interpreter.
Closing in favour of #99331
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@markshannon markshannon closed this Nov 10, 2022
@markshannon
markshannon deleted the specialize-calls-to-normal-python-classes branch September 26, 2023 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iritkatriel iritkatriel iritkatriel left review comments
@Fidget-Spinner Fidget-Spinner Fidget-Spinner approved these changes
@sweeneyde sweeneyde sweeneyde left review comments
@kumaraditya303 kumaraditya303 kumaraditya303 left review comments
@methane methane Awaiting requested review from methane methane is a code owner

Uh oh!

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

Assignees

No one assigned

Labels

awaiting merge

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.

6 participants

@markshannon @bedevere-bot @iritkatriel @Fidget-Spinner @sweeneyde @kumaraditya303
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 01402520 — 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.