GeezerVison is showing github.com — gh-74690: `typing._ProtocolMeta.__instancecheck__`: Exit early for protocols that only have callable members by AlexWaygood · Pull Request #103310 · python/cpython
Conversation
AlexWaygood
commented
Apr 6, 2023
•
edited by bedevere-bot
Loading
Uh oh!
There was an error while loading. Please reload this page.
This speeds up this isinstance() call 12x, while maintaining all invariants:
from typing import SupportsInt
isinstance(object(), SupportsInt)
The performance of other kinds of isinstance() calls is not impacted.
Benchmark script
import time
from typing import Protocol, runtime_checkable, SupportsInt
@runtime_checkable
class HasX(Protocol):
x: int
@runtime_checkable
class SupportsIntAndX(Protocol):
x: int
def __int__(self) -> int: ...
class Empty:
description = "Empty class with no attributes"
class Registered:
description = "Subclass registered using ABCMeta.register"
HasX.register(Registered)
SupportsInt.register(Registered)
SupportsIntAndX.register(Registered)
class PropertyX:
description = "Class with a property x"
@property
def x(self) -> int:
return 42
class HasIntMethod:
description = "Class with an __int__ method"
def __int__(self):
return 42
class PropertyXWithInt:
description = "Class with a property x and an __int__ method"
@property
def x(self) -> int:
return 42
def __int__(self):
return 42
class ClassVarX:
description = "Class with a ClassVar x"
x = 42
class ClassVarXWithInt:
description = "Class with a ClassVar x and an __int__ method"
x = 42
def __int__(self):
return 42
class InstanceVarX:
description = "Class with an instance var x"
def __init__(self):
self.x = 42
class InstanceVarXWithInt:
description = "Class with an instance var x and an __int__ method"
def __init__(self):
self.x = 42
def __int__(self):
return 42
class NominalX(HasX):
description = "Class that explicitly subclasses HasX"
def __init__(self):
self.x = 42
class NominalSupportsInt(SupportsInt):
description = "Class that explicitly subclasses SupportsInt"
def __int__(self):
return 42
class NominalXWithInt(SupportsIntAndX):
description = "Class that explicitly subclasses NominalXWithInt"
def __init__(self):
self.x = 42
num_instances = 500_000
classes = {}
for cls in (
Empty, Registered, PropertyX, PropertyXWithInt, ClassVarX, ClassVarXWithInt,
InstanceVarX, InstanceVarXWithInt, NominalX, NominalXWithInt, HasIntMethod,
NominalSupportsInt
):
classes[cls] = [cls() for _ in range(num_instances)]
def bench(objs, title, protocol):
start_time = time.perf_counter()
for obj in objs:
isinstance(obj, protocol)
elapsed = time.perf_counter() - start_time
print(f"{title}: {elapsed:.2f}")
print("Protocols with no callable members\n")
for cls in Empty, Registered, PropertyX, ClassVarX, InstanceVarX, NominalX:
bench(classes[cls], cls.description, HasX)
print("\nProtocols with only callable members\n")
for cls in Empty, Registered, HasIntMethod, NominalSupportsInt:
bench(classes[cls], cls.description, SupportsInt)
print("\nProtocols with callable and non-callable members\n")
for cls in (
Empty, Registered, PropertyXWithInt, ClassVarXWithInt, InstanceVarXWithInt,
NominalXWithInt
):
bench(classes[cls], cls.description, SupportsIntAndX)
Full benchmark results on `main`
Protocols with no callable members
Empty class with no attributes: 2.46
Subclass registered using ABCMeta.register: 0.45
Class with a property x: 1.40
Class with a ClassVar x: 1.44
Class with an instance var x: 6.41
Class that explicitly subclasses HasX: 0.63
Protocols with only callable members
Empty class with no attributes: 8.41
Subclass registered using ABCMeta.register: 3.32
Class with an __int__ method: 0.61
Class that explicitly subclasses SupportsInt: 0.61
Protocols with callable and non-callable members
Empty class with no attributes: 8.43
Subclass registered using ABCMeta.register: 1.61
Class with a property x and an __int__ method: 9.19
Class with a ClassVar x and an __int__ method: 9.22
Class with an instance var x and an __int__ method: 11.27
Class that explicitly subclasses NominalXWithInt: 0.62
Full benchmark results with this PR
Protocols with no callable members
Empty class with no attributes: 2.46
Subclass registered using ABCMeta.register: 0.45
Class with a property x: 1.41
Class with a ClassVar x: 1.41
Class with an instance var x: 6.40
Class that explicitly subclasses HasX: 0.61
Protocols with only callable members
Empty class with no attributes: 0.68
Subclass registered using ABCMeta.register: 3.36
Class with an __int__ method: 0.63
Class that explicitly subclasses SupportsInt: 0.61
Protocols with callable and non-callable members
Empty class with no attributes: 8.33
Subclass registered using ABCMeta.register: 1.60
Class with a property x and an __int__ method: 9.14
Class with a ClassVar x and an __int__ method: 9.17
Class with an instance var x and an __int__ method: 11.59
Class that explicitly subclasses NominalXWithInt: 0.62
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
…r protocols that only have callable members
while maintaining all invariants
Argh, I was wrong. While this PR has no impact on behaviour in 99% of common cases, there is a behaviour change in this edge case:
>>> from typing import *
>>> @runtime_checkable
... class Foo(Protocol):
... def meth(self): ...
...
>>> class Bar:
... def __init__(self):
... self.meth = lambda: None
...
>>> isinstance(Bar(), Foo) # True on `main`, False with this PR
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
AlexWaygood
commented
Apr 6, 2023
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
|
Strictly speaking, I suppose the optimisation would be safe for dunder methods, which are always looked up on the class instead of the instance. But not sure if this optimisation is worth complicating the logic such that we start differentiating between dunder and non-dunder methods in __instancecheck__.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Strictly speaking, I suppose the optimisation would be safe for dunder methods
I hate this idea, and can't come up with a better one that provides this optimisation. So, closing for now; will add more tests in a different PR.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
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.
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.
|