Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32.1k
bpo-46564: Optimizesuper().meth() calls via adaptive superinstructions#30992
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
super().meth() callssuper().meth() calls via adaptive superinstructionsCo-Authored-By: Vladimir Matveev <v2matveev@outlook.com>
Marking as draft as I need make this work with the new |
Uh oh!
There was an error while loading.Please reload this page.
| 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) { |
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.
| } | ||
| 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); |
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 bytype(self) and__class__, both of which are known at this point?
Fidget-SpinnerJan 29, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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?
Maybe we should merge#31002 first, as that PR is simpler. |
👍 |
Mark, I'm going to run benchmarks on |
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:
| staticint | ||
| super_init_without_args(InterpreterFrame *cframe, PyCodeObject *co, | ||
| int | ||
| _PySuper_GetTypeArgs(InterpreterFrame *cframe, PyCodeObject *co, | ||
| PyTypeObject **type_p, PyObject **obj_p) |
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.
| PyTypeObject**type_p,PyObject**obj_p) | |
| PyTypeObject**type_p,PyObject**obj_p) |
The line was aligned with an opening parenthesis of a parameter list.
| PyObject *kwnames, SpecializedCacheEntry *cache, PyObject *builtins, | ||
| PyObject **stack_pointer, InterpreterFrame *frame, PyObject *names); |
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.
| 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:
| 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.
Well that was depressing. |
Fidget-Spinner commentedFeb 1, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Microbenchmarks show that importtimeitsetup="""class A: def f(self): passclass B(A): def g(self): super().f() def h(self): self.f()b = B()"""# super() callprint(timeit.timeit("b.g()",setup=setup,number=20_000_000))# referenceprint(timeit.timeit("b.h()",setup=setup,number=20_000_000)) Results: So |
Uh oh!
There was an error while loading.Please reload this page.
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 useInterpreterFrameoverPyFrameObjectfor lazy frame benefits. Some changes here are partially taken frombpo-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 ofsuper().ceval.c-- does both aCALLandLOAD_METHODwithout intermediates (and both are specialized forms too).TODO:
benchmarks!
https://bugs.python.org/issue46564