-
-
Notifications
You must be signed in to change notification settings - Fork 30.5k
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
gh-103193: Use LBYL idioms rather than EAFP in inspect.getattr_static
#103318
gh-103193: Use LBYL idioms rather than EAFP in inspect.getattr_static
#103318
Conversation
Skipping news, since the news entry in #103195 should suffice, I think. |
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.
It's less idiomatic code, but I think it's worth it for the very significant performance improvement on runtime protocol isinstance checking with many attributes.
I assume you've checked the performance of using .get(...)
and it's worse?
Hmm I haven't, actually -- not sure how you'd use |
Oh I think I see what you mean. Trying it out now. |
Yeah, I tried out doing this instead (diff is relative to my current patch, not to diff --git a/Lib/inspect.py b/Lib/inspect.py
index a317f0ca74..7fcaa13750 100644
--- a/Lib/inspect.py
+++ b/Lib/inspect.py
@@ -1787,8 +1787,10 @@ def _check_instance(obj, attr):
def _check_class(klass, attr):
for entry in _static_getmro(klass):
- if _shadowed_dict(type(entry)) is _sentinel and attr in entry.__dict__:
- return entry.__dict__[attr]
+ if _shadowed_dict(type(entry)) is _sentinel:
+ ret = entry.__dict__.get(attr, _sentinel)
+ if ret is not _sentinel:
+ return ret
return _sentinel
def _is_type(obj):
@@ -1800,13 +1802,13 @@ def _is_type(obj):
def _shadowed_dict(klass):
for entry in _static_getmro(klass):
- dunder_dict = _get_dunder_dict_of_class(entry)
- if '__dict__' in dunder_dict:
- class_dict = dunder_dict['__dict__']
- if not (type(class_dict) is types.GetSetDescriptorType and
- class_dict.__name__ == "__dict__" and
- class_dict.__objclass__ is entry):
- return class_dict
+ class_dict = _get_dunder_dict_of_class(entry).get('__dict__', _sentinel)
+ if class_dict is not _sentinel and not (
+ type(class_dict) is types.GetSetDescriptorType
+ and class_dict.__name__ == "__dict__"
+ and class_dict.__objclass__ is entry
+ ):
+ return class_dict
return _sentinel
def getattr_static(obj, attr, default=_sentinel):
@@ -1845,11 +1847,10 @@ def getattr_static(obj, attr, default=_sentinel):
if obj is klass:
# for types we check the metaclass too
for entry in _static_getmro(type(klass)):
- if (
- _shadowed_dict(type(entry)) is _sentinel
- and attr in entry.__dict__
- ):
- return entry.__dict__[attr]
+ if _shadowed_dict(type(entry)) is _sentinel:
+ ret = entry.__dict__.get(attr, _sentinel)
+ if ret is not _sentinel:
+ return ret @carljm is that the kind of thing you were talking about? :) |
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.
Good idea!
Thanks both! |
This PR significantly speeds up
inspect.getattr_static()
and, as a result,isinstance()
checks against runtime-checkable protocols.Here are benchmark results on main using @sobolevn's benchmark script from #103193 (comment):
And here are the benchmark results with this PR:
A result of this optimisation is that the following
isinstance()
call becomes around 1.5x faster:inspect.getattr_static
#103193