Performance of typing._ProtocolMeta._get_protocol_attrs and isinstance

[orenbenkiki]

BPO	30505
Nosy	@ilevkivskyi, @orenbenkiki

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2017-05-29.09:16:49.884>
labels = ['library', 'performance']
title = 'Performance of typing._ProtocolMeta._get_protocol_attrs and isinstance'
updated_at = <Date 2017-06-02.17:44:59.311>
user = 'https://github.com/orenbenkiki'

bugs.python.org fields:

activity = <Date 2017-06-02.17:44:59.311>
actor = 'levkivskyi'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2017-05-29.09:16:49.884>
creator = 'orenbenkiki'
dependencies = []
files = []
hgrepos = []
issue_num = 30505
keywords = []
message_count = 2.0
messages = ['294686', '295044']
nosy_count = 2.0
nosy_names = ['levkivskyi', 'orenbenkiki']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'performance'
url = 'https://bugs.python.org/issue30505'
versions = ['Python 3.6']

Linked PRs

• gh-74690: typing: Don't unnecessarily call _get_protocol_attrs twice in _ProtocolMeta.__instancecheck__ #103141
• gh-74690: Micro-optimise typing._get_protocol_attrs #103152
• gh-74690: typing: Simplify and optimise _ProtocolMeta.__instancecheck__ #103159
• gh-74690: typing: Call _get_protocol_attrs and _callable_members_only at protocol class creation time, not during isinstance() checks #103160
• gh-74690: Further optimise typing._ProtocolMeta.__instancecheck__ #103280
• gh-74690: typing._ProtocolMeta.__instancecheck__: Exit early for protocols that only have callable members #103310
• gh-74690: Add more tests for runtime-checkable protocols #103347
• gh-74690: Document changes made to runtime-checkable protocols in 3.12 #103348
• [3.11] gh-74690: Add more tests for runtime-checkable protocols (GH-103347) #103350
• gh-74690: Don't set special protocol attributes on non-protocol subclasses of protocols #104622
• gh-74690: Make a typing test more resilient #104691
• gh-74690: Improve performance when testing against protocols with many attributes. #112157
• gh-74690: Optimise isinstance() and issubclass() calls against runtime-checkable protocols by avoiding costly super() calls #112708
• gh-74690: Avoid a costly type check where possible in _ProtocolMeta.__subclasscheck__ #112717

[orenbenkiki]

In 3.6.0, invocation of isinstance calls typing._ProtocolMeta._get_protocol_attrs.
This creates a set of all attributes in all base classes, loops on these attributes to check they exist, and discards the set. It is very slow.

My program uses isinstance to allow for flexibility in parameter types in certain key functions. I realize that using isinstance is frowned upon, but it seems to make sense in my case.

As a result, >95% of its run-time is inside typing._ProtocolMeta._get_protocol_attrs (!).

I have created a simple wrapper around isinstance which caches its result with a Dict[Tuple[type, type], bool]. This solved the performance problem, but introduced a different problem - type checking.

I use mypy and type annotations, and my code cleanly type-checks (with the occasional # type: ignore). If I switch to using my own isinstance function, then mypy's type inference no longer treats it as special, so it starts complaining about all uses of values protected by if isinstance(value, SomeType): ...

I propose that either the private typing._ProtocolMeta.__subclasscheck__ (which invokes _get_protocol_attrs), or the public isinstance, would be modified to cache their results.

I can create a PR for either approach, if this is acceptable.

[ilevkivskyi]

Thanks for reporting!

The runtime implementation of protocol classes will be thoroughly reworked as a part of PEP-544, see also python/typing#417 for a proof of concept runtime implementation.

Also, there is another ongoing discussion python/typing#432 about a global refactoring of typing module that will significantly improve performance.

Therefore, I would wait with any large PRs until these two stories are settled. If you still want to propose a small PR, you can do this at the upstream typing repo https://github.com/python/typing

[iritkatriel]

@orenbenkiki @ilevkivskyi What is the status of this issue, five years on?

[ilevkivskyi]

TBH I am not sure this issue is still relevant (at least I didn't hear any recent complaints about Protocol performance). @orenbenkiki is performance still bad for your application?

[AlexWaygood]

I've seen recent complaints about the performance of Protocol from users of the beartype third-party library, and by @posita here. (I'm not sure I've ever seen a proper write-up of the performance problems in a CPython issue, however.)

[ilevkivskyi]

Hm, actually the short-circuiting proposed in that comment may be a good idea (unless I forgot something) for a simple perf optimization (btw FWIW this is what mypy does internally, it always checks nominal subtypig first).

[AlexWaygood]

@posita appears to have implemented optimised versions of ProtocolMeta here and here.

(I haven't studied the code for either link, but may be worth looking at!)

[posita]

@posita appears to have implemented optimised versions of ProtocolMeta here and here.

(I haven't studied the code for either link, but may be worth looking at!)

For color, the current implementation lives in Beartype and is divided among two classes:

• beartype.typing._typingpep544._CachingProtocolMeta; and
• beartype.typing._typingpep544.Protocol

The meat is in _CachingProtocolMeta whereas the Protocol is basically a hack to work around some standard library implementation details and could likely be eliminated. At a high level, _CachingProtocolMeta overrides __instancecheck__ to perform an examination of whether an object "satisfies" a Protocol definition similar to typing._ProtocolMeta.__instancecheck__ and then caches the result by that object's type. Note the asymmetry: the examination is performed on the object, but it is assumed that all objects of the first-examined object's type are consistent. This makes the implementation ill-suited for runtime-composed "types" or monkey-patched objects.

Happy to discuss further, if desired.

[ilevkivskyi]

Caching may need a more careful consideration (especially w.r.t. monkey-patching classes). I was specifically referring to this idea

class _ProtocolMeta(ABCMeta):
    # …
    def __instancecheck__(cls, instance):
        if super().__instancecheck__(instance):
            # Short circuit for direct inheritors
            return True
        else:
            # … existing implementation that checks method names, etc. …
            return False

@posita Do you have any good code base where you can benchmark this w.r.t. current implementation?

[posita]

I can probably do something crude with numerary's and dyce's respective batteries of unit tests. Those are what motivated beartype.typing._typingpep544._CachingProtocolMeta in the first place. Give me a few days and I can probably throw something together.

[posita]

Okay, this was a bit more involved than I thought it would be, but I have some rough numbers. My approach was to create a metaclass deriving from typing._ProtocolMeta but implementing its own __instancecheck__ method:

from typing import _ProtocolMeta as ProtocolMeta

class ProtocolMetaShortCircuit(ProtocolMeta):
    def __instancecheck__(cls, instance):
        return super(ProtocolMeta, cls).__instancecheck__(instance) or super(
            ProtocolMetaShortCircuit, cls
        ).__instancecheck__(instance)

This is used to create a special version of the big, scary (non-caching) SupportsLotsOfNumberStuff protocol borrowed from the numerary docs.

@runtime_checkable
class SupportsLotsOfNumberStuffShortCircuit(
    SupportsLotsOfNumberStuff,
    Protocol,
    metaclass=ProtocolMetaShortCircuit,
): pass

# ABC registration
assert isinstance(Fraction(), SupportsLotsOfNumberStuffShortCircuit)
SupportsLotsOfNumberStuffShortCircuit.register(Fraction)

# Direct derivation
class FractionShortCircuit(Fraction, SupportsLotsOfNumberStuffShortCircuit): pass

Initial results:

====
%timeit isinstance(1, SupportsLotsOfNumberStuff)
50 µs ± 616 ns per loop (mean ± std. dev. of 7 runs, 10,000 loops each)
%timeit isinstance(1, SupportsLotsOfNumberStuffShortCircuit)
54.4 µs ± 785 ns per loop (mean ± std. dev. of 7 runs, 10,000 loops each)
====
%timeit isinstance(2.0, SupportsLotsOfNumberStuff)
48.6 µs ± 464 ns per loop (mean ± std. dev. of 7 runs, 10,000 loops each)
%timeit isinstance(2.0, SupportsLotsOfNumberStuffShortCircuit)
51.3 µs ± 508 ns per loop (mean ± std. dev. of 7 runs, 10,000 loops each)
====
%timeit isinstance(Decimal('3'), SupportsLotsOfNumberStuff)
48.3 µs ± 288 ns per loop (mean ± std. dev. of 7 runs, 10,000 loops each)
%timeit isinstance(Decimal('3'), SupportsLotsOfNumberStuffShortCircuit)
52.1 µs ± 247 ns per loop (mean ± std. dev. of 7 runs, 10,000 loops each)
====
%timeit isinstance(Fraction(4, 1), SupportsLotsOfNumberStuff)
52 µs ± 318 ns per loop (mean ± std. dev. of 7 runs, 10,000 loops each)
%timeit isinstance(Fraction(4, 1), SupportsLotsOfNumberStuffShortCircuit)
26.1 µs ± 271 ns per loop (mean ± std. dev. of 7 runs, 10,000 loops each)
====
%timeit isinstance(FractionShortCircuit(5, 1), SupportsLotsOfNumberStuff)
52.7 µs ± 346 ns per loop (mean ± std. dev. of 7 runs, 10,000 loops each)
%timeit isinstance(FractionShortCircuit(5, 1), SupportsLotsOfNumberStuffShortCircuit)
262 ns ± 3.02 ns per loop (mean ± std. dev. of 7 runs, 1,000,000 loops each)

For built-in types, the short-circuit implementation imposes a small overhead of ~300 ns on my machine (which could likely be reduced), and is about twice as fast¹ for ABCMeta.registered types, but (as anticipated) is way faster for types that explicitly derive from the short-circuiting protocol.

You can play around with the notebook in Binder: 👈 [might take awhile to load]

I tinkered with using this metaclass in my battery of unit tests for numerary and dyce. Anecdotally, I didn't notice much of a difference. I think this is largely because those tests rarely hit cases where this approach is beneficial (even fewer than I had anticipated).

---

¹ The registry check is likely O(1) due to caching (if I'm reading that implementation right).