Skip to content
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-74690: typing._ProtocolMeta.__instancecheck__: Exit early for protocols that only have callable members #103310

Closed

Conversation

AlexWaygood
Copy link
Member

@AlexWaygood AlexWaygood commented Apr 6, 2023

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

@AlexWaygood AlexWaygood added type-feature A feature request or enhancement performance Performance or resource usage stdlib Python modules in the Lib dir topic-typing 3.12 bugs and security fixes labels Apr 6, 2023
@AlexWaygood AlexWaygood requested a review from carljm April 6, 2023 10:38
@AlexWaygood
Copy link
Member Author

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

@AlexWaygood
Copy link
Member Author

AlexWaygood commented Apr 6, 2023

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__.

@AlexWaygood
Copy link
Member Author

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.

@AlexWaygood AlexWaygood closed this Apr 6, 2023
@AlexWaygood AlexWaygood deleted the callable-protocols-optimisation branch April 6, 2023 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.12 bugs and security fixes awaiting core review DO-NOT-MERGE performance Performance or resource usage skip news stdlib Python modules in the Lib dir topic-typing type-feature A feature request or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants