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-46564: Optimize ``super().meth()`` calls via adaptive superinstructions by Fidget-Spinner · Pull Request #30992 · python/cpython

Conversation

@Fidget-Spinner

@Fidget-Spinner Fidget-Spinner commented Jan 28, 2022 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

They should now have almost no overhead over a corresponding self.meth() call.

Summary of changes:

  • typeobject.c -- refactoring to reuse code during specialization, also use InterpreterFrame over PyFrameObject for lazy frame benefits. Some changes here are partially taken from bpo-43563 : Introduce dedicated opcodes for super calls #24936. All credits to @vladima (I've tried to properly include them in the news item too.)
  • specialize.c -- specialize for the 0-argument and 2-argument form of super().
  • ceval.c -- does both a CALL and LOAD_METHOD without intermediates (and both are specialized forms too).

TODO:
benchmarks!

https://bugs.python.org/issue46564

Sorry, something went wrong.

Uh oh!

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

All reactions
Fidget-Spinner added 2 commits January 29, 2022 00:45
@Fidget-Spinner
Near zero-cost super().meth() calls
3b12d1b
@Fidget-Spinner
remove py_stats
5b7a98d
@Fidget-Spinner
Fidget-Spinner requested a review from markshannon as a code owner January 28, 2022 16:47
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Jan 28, 2022
@bedevere-bot bedevere-bot added the awaiting core review label Jan 28, 2022
@Fidget-Spinner Fidget-Spinner mentioned this pull request Jan 28, 2022
Optimizing super().meth() via adaptive superinstructions faster-cpython/ideas#242
Closed
@Fidget-Spinner Fidget-Spinner changed the title bpo-46564: Optimize super().meth() calls bpo-46564: Optimize super().meth() calls via adaptive superinstructions Jan 28, 2022
Fidget-Spinner and others added 5 commits January 29, 2022 01:06
@Fidget-Spinner @vladima
Add vladima to co-authors
efb0ae3
Co-Authored-By: Vladimir Matveev <[email protected]>
@Fidget-Spinner
Merge remote-tracking branch 'upstream/main' into zero_cost_super
e037a56
@Fidget-Spinner
fix merge errors
9d64819
@Fidget-Spinner
Update test_frozenmain.h
ac19aa1
@Fidget-Spinner
fix formatting, correct specialization
1f9bb6c
@markshannon markshannon self-assigned this Jan 28, 2022
@Fidget-Spinner
Fidget-Spinner marked this pull request as draft January 28, 2022 18:14
@Fidget-Spinner

Fidget-Spinner commented Jan 28, 2022

Copy link
Copy Markdown
Member Author

Marking as draft as I need make this work with the new CALL convention.

All reactions

Sorry, something went wrong.

Uh oh!

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

@Fidget-Spinner
work with new call convention
f4cd3f9
@Fidget-Spinner
Fidget-Spinner marked this pull request as ready for review January 28, 2022 18:23
@Fidget-Spinner Fidget-Spinner mentioned this pull request Jan 29, 2022
bpo-46564: do not create frame object for super object #31002
Merged
markshannon
markshannon reviewed Jan 29, 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 Jan 29, 2022
View reviewed changes
Comment thread Python/ceval.c

DEOPT_IF(_PyType_CAST(super_callable) != &PySuper_Type, CALL);
/* super() - zero argument form */
if (_PySuper_GetTypeArgs(frame, frame->f_code, &su_type, &su_obj) < 0) {

@markshannon markshannon Jan 29, 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.

Can't we do this at specialization time? The number of locals, the index of "self", and whether it is a cell are all known then. Likewise the nature of __class__ is also known.

Sorry, something went wrong.

Uh oh!

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

All reactions
markshannon
markshannon reviewed Jan 29, 2022
View reviewed changes
Comment thread Python/ceval.c
}
assert(su_obj != NULL);
DEOPT_IF(lm_adaptive->version != Py_TYPE(su_obj)->tp_version_tag, CALL);
DEOPT_IF(cache0->version != su_type->tp_version_tag, CALL);

@markshannon markshannon Jan 29, 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.

When can this fail?
Isn't the next item in the MRO determined solely by type(self) and __class__, both of which are known at this point?

Sorry, something went wrong.

Uh oh!

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

All reactions

@Fidget-Spinner Fidget-Spinner Jan 29, 2022 •
edited
Loading

Uh oh!

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

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 wanted assurance that __class__ didn't change.. Then again, I'm not sure if it can?

Sorry, something went wrong.

Uh oh!

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

All reactions
@markshannon

markshannon commented Jan 29, 2022

Copy link
Copy Markdown
Member

Maybe we should merge #31002 first, as that PR is simpler.
It would also allow us to compare the performance of just the specialization, without the removal of frame object allocation.

All reactions

Sorry, something went wrong.

Uh oh!

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

@Fidget-Spinner

Fidget-Spinner commented Jan 29, 2022

Copy link
Copy Markdown
Member Author

Maybe we should merge #31002 first, as that PR is simpler. It would also allow us to compare the performance of just the specialization, without the removal of frame object allocation.

👍

All reactions

Sorry, something went wrong.

Uh oh!

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

Fidget-Spinner added 3 commits January 29, 2022 21:27
@Fidget-Spinner
partially address review: fix tests, add asserts, add cache modificat…
19880a9
…ion guards
@Fidget-Spinner
fix refleak
696a0e8
@Fidget-Spinner
Merge remote-tracking branch 'upstream/main' into zero_cost_super
01202ee
@Fidget-Spinner

Fidget-Spinner commented Feb 1, 2022

Copy link
Copy Markdown
Member Author

Mark, I'm going to run benchmarks on deltablue first since it uses 2-argument form super. I'll address your optimization ideas for the 0-arg form once those results come back. Fingers crossed.

All reactions

Sorry, something went wrong.

Uh oh!

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

arhadthedev
arhadthedev requested changes Feb 1, 2022
View reviewed changes

@arhadthedev arhadthedev 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.

A couple of indentation-related inconsistencies:

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread Objects/typeobject.c
super_init_without_args(InterpreterFrame *cframe, PyCodeObject *co,
int
_PySuper_GetTypeArgs(InterpreterFrame *cframe, PyCodeObject *co,
PyTypeObject **type_p, PyObject **obj_p)

@arhadthedev arhadthedev Feb 1, 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.

Suggested change
PyTypeObject **type_p, PyObject **obj_p)
PyTypeObject **type_p, PyObject **obj_p)

The line was aligned with an opening parenthesis of a parameter list.

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread Include/internal/pycore_code.h
Comment on lines +278 to +279
PyObject *kwnames, SpecializedCacheEntry *cache, PyObject *builtins,
PyObject **stack_pointer, InterpreterFrame *frame, PyObject *names);

@arhadthedev arhadthedev Feb 1, 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.

Suggested change
PyObject *kwnames, SpecializedCacheEntry *cache, PyObject *builtins,
PyObject **stack_pointer, InterpreterFrame *frame, PyObject *names);
PyObject *kwnames, SpecializedCacheEntry *cache, PyObject *builtins,
PyObject **stack_pointer, InterpreterFrame *frame, PyObject *names);

as in a removed line, or even:

Suggested change
PyObject *kwnames, SpecializedCacheEntry *cache, PyObject *builtins,
PyObject **stack_pointer, InterpreterFrame *frame, PyObject *names);
PyObject *kwnames, SpecializedCacheEntry *cache, PyObject *builtins,
PyObject **stack_pointer, InterpreterFrame *frame, PyObject *names);

as in _Py_Specialize_BinaryOp right below.

Sorry, something went wrong.

Uh oh!

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

All reactions
@Fidget-Spinner

Fidget-Spinner commented Feb 1, 2022

Copy link
Copy Markdown
Member Author

Mark, I'm going to run benchmarks on deltablue first since it uses 2-argument form super. I'll address your optimization ideas for the 0-arg form once those results come back. Fingers crossed.

Well that was depressing. deltablue only shows 1.03x speedup. Looking closer at the code, super isn't called in any tight loops so that might be why. Maybe I need to pull out microbenchmarks now.

All reactions

Sorry, something went wrong.

Uh oh!

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

@Fidget-Spinner

Fidget-Spinner commented Feb 1, 2022 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member Author

Microbenchmarks show that super() has sped up by more than 2.2x. This is faster than that other attempt because there's also speedups from the LOAD_METHOD_CACHED:
(Extremely unscientific, I'm short on time to set up pyperf right now)

import timeit

setup = """
class A:
    def f(self): pass
class B(A):
    def g(self): super().f()
    def h(self): self.f()

b = B()
"""

# super() call
print(timeit.timeit("b.g()", setup=setup, number=20_000_000))
# reference
print(timeit.timeit("b.h()", setup=setup, number=20_000_000))

Results:

# Main
5.796037399995839
2.4094066999969073

# This branch
2.4578273000006448
2.3718886000060593

So super().meth() is now only ~10% slowly than the corresponding self.meth() call whereas it was nearly 2x as slow previously. If I manage to incorporate your suggestions correctly, this will effectively just be a competition between LOAD_GLOBAL_BUILTIN (super) and LOAD_FAST (self).

All reactions

Sorry, something went wrong.

Uh oh!

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

@Fidget-Spinner Fidget-Spinner mentioned this pull request Mar 5, 2022
bpo-46921: Vectorcall support for super() #31687
Merged
@ezio-melotti ezio-melotti removed the CLA signed label Jul 13, 2022
@eendebakpt eendebakpt mentioned this pull request Aug 31, 2022
Triage the open "performance" issues on cpython repo faster-cpython/ideas#440
Closed
@iritkatriel iritkatriel added the performance Performance or resource usage label Aug 31, 2022
@carljm carljm mentioned this pull request Apr 13, 2023
gh-87729: add LOAD_SUPER_ATTR instruction for faster super() #103497
Merged
@Fidget-Spinner Fidget-Spinner closed this Apr 27, 2023
@Fidget-Spinner Fidget-Spinner mentioned this pull request Apr 27, 2023
Near zero-cost super().meth() calls via adaptive superinstructions #90722
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@markshannon markshannon markshannon left review comments
@arhadthedev arhadthedev arhadthedev requested changes

Uh oh!

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

Assignees

@markshannon markshannon

Labels

awaiting core review performance Performance or resource usage

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.

7 participants

@Fidget-Spinner @markshannon @arhadthedev @iritkatriel @the-knights-who-say-ni @ezio-melotti @bedevere-bot
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.