bpo-44337: Improve LOAD_ATTR specialization#26759
bpo-44337: Improve LOAD_ATTR specialization#26759markshannon merged 5 commits intopython:mainpython/cpython:mainfrom faster-cpython:improve-load-attr-specializationfaster-cpython/cpython:improve-load-attr-specializationCopy head branch name to clipboard
Conversation
* Specialize obj.__class__ with LOAD_ATTR_SLOT * Specialize instance attribute lookup with attribute on class, provided attribute on class is not an overriding descriptor.
|
Performance results: https://gist.github.com/markshannon/f938b10691f081b3a6cdad54521890fb |
There was a problem hiding this comment.
mostly LGTM
I did not notice any logical problems or bugs. I've left a number of clarifying questions and a few minor suggestions. I do not consider any of them to be blockers, especially since this PR is an improvement regardless of my comments. However, in a couple of the cases it would be worth having a bit of discussion before merging.
Specifically:
- should we also identify descriptors for type objects (currently ignored since they have
type.__getattribute__instead ofobject.__getattribute__)? - move
analyze_descriptor()to typeobject.c (and make it part of the internal API)?
Any thoughts on why some of those benchmarks are showing performance regressions? |
Not really, nor why unpack_sequence is 18% faster |
ericsnowcurrently
left a comment
There was a problem hiding this comment.
LGTM
Thanks for the feedback on my questions/comments. The changes make sense and analyze_descriptor() is a definite improvement.
Python/specialize.c
Outdated
| NON_DESCRIPTOR, /* Is not a descriptor, and is an instance of an immutable class */ | ||
| MUTABLE, /* Instance of a mutable class; might, or might not, be a descriptor */ | ||
| ABSENT, /* Attribute is not present on the class */ | ||
| __CLASS__, /* __class__ attribute */ |
There was a problem hiding this comment.
I would worry this (__CLASS__) would be a used or reserved name in some C implementation. Identifiers with __ are generally reserved by the C std.
We were being overly conservative by not specializing for instance attributes when the class also has an attribute.
It is OK to perform that specialization if the class attribute is not an overriding descriptor (and has an immutable class, so it will remain so).
This requires more detailed analysis of the class attribute, so this is factored out in a new function, which we can also use when we specialize
STORE_ATTR.This PR also specializes
obj.__class__usingLOAD_ATTR_SLOT.https://bugs.python.org/issue44337