gh-74690: typing: Don't unnecessarily call _get_protocol_attrs twice in _ProtocolMeta.instancecheck#103141

AlexWaygood

This simple optimisation achieves speedups for all kinds of isinstance() checks against runtime-checkable protocols. For structural subtypes that set attributes in __init__ methods, the speedup is dramatic.

Benchmark script

import time
from typing import Protocol, runtime_checkable

@runtime_checkable
class HasX(Protocol):
    x: int

class Foo:
    @property
    def x(self) -> int:
        return 42

class Bar:
    x = 42

class Baz:
    def __init__(self):
        self.x = 42

class Egg: ...

class Nominal(HasX):
    def __init__(self):
        self.x = 42

class Registered: ...

HasX.register(Registered)

num_instances = 500_000
foos = [Foo() for _ in range(num_instances)]
bars = [Bar() for _ in range(num_instances)]
bazzes = [Baz() for _ in range(num_instances)]
basket = [Egg() for _ in range(num_instances)]
nominals = [Nominal() for _ in range(num_instances)]
registereds = [Registered() for _ in range(num_instances)]


def bench(objs, title):
    start_time = time.perf_counter()
    for obj in objs:
        isinstance(obj, HasX)
    elapsed = time.perf_counter() - start_time
    print(f"{title}: {elapsed:.2f}")


bench(foos, "Time taken for objects with a property")
bench(bars, "Time taken for objects with a classvar")
bench(bazzes, "Time taken for objects with an instance var")
bench(basket, "Time taken for objects with no var")
bench(nominals, "Time taken for nominal subclass instances")
bench(registereds, "Time taken for registered subclass instances")

Results on 01a49d1 (non-debug build, PGO-optimised):

Time taken for objects with a property: 3.63 Time taken for objects with a classvar: 3.82 Time taken for objects with an instance var: 12.22 Time taken for objects with no var: 13.73 Time taken for nominal subclass instances: 12.46 Time taken for registered subclass instances: 20.62

Results with this PR:

Time taken for objects with a property: 2.10 Time taken for objects with a classvar: 2.07 Time taken for objects with an instance var: 2.06 Time taken for objects with no var: 6.94 Time taken for nominal subclass instances: 7.54 Time taken for registered subclass instances: 16.06

(We may want to wait until #103034 is merged and then re-measure.)

Cc. @leycec and @posita, as people who I know care a lot about Protocol performance :)

Issue: Performance of typing._ProtocolMeta._get_protocol_attrs and isinstance #74690

We could possibly cache the result of this call on the protocol class in a __protocol_attrs__ class attribute. But I think I'd rather look at that in a separate PR so that we can evaluate the impact of it in isolation. I'd also want to check to see if there were any behaviour changes, and compare different methods of caching the result of _get_protocol_attrs.

We could possibly cache the result of this call on the protocol class in a __protocol_attrs__ class attribute. ...

If you decide to tackle this, @beartype provides a caching mechanism for __instancecheck__ results that may be worth a look, if only for inspiration. I believe some of the same caveats would apply. (Full disclosure: I am an author. Happy to chat, if helpful.)

@ilevkivskyi

I share @ilevkivskyi's reservations in #74690 (comment) about caching the whole call to __instancecheck__ in the way that I believe beartype does it, but I think caching the call to _get_protocol_attrs could get us a large speedup without the same risk of changing the behaviour. (Apologies if I was unclear in my comment above!) Anyway, TBD in a future PR :)

Is monkey-patching of the protocol class itself something we need to care about here?

Pretty unlikely imo. But yeah, it's that kind of thing that I'd want to think through, which is why I'd probably like to leave that to another PR :)

AlexWaygood

Some local experimentation (by combining the two branches) indicates that this PR doesn't quite make up for the performance hit from #103034, but does significantly mitigate it.

carljm

This change LGTM. Nice find!

AlexWaygood

I discussed with @carljm offline, and we decided it's probably better to skip news for this one. On its own it's a performance improvement, but it'll likely be accompanied by #103034(which hurts performance of runtime-checkable protocols), and possibly one or two other optimisations to this code that I'm eyeing up. So the overall impact on performance for runtime-checkable protocols is pretty uncertain at the moment, which would mean including a news entry at this stage would probably just cause confusion to readers of the changelog.

hauntsaninja

Nice find! For what it's worth, I've found users often depend on undocumented aspects of typing.py, so I'd have still included a vague news entry on the lines of "Changed the performance characteristics of _ProtocolMeta.__instancecheck__ by avoiding a redundant call to _get_protocol_attrs" (i.e. make clear something changed but not promise any improvement)

AlexWaygood

I've found users often depend on undocumented aspects of typing.py

Speaking from experience? ;)

pythongh-74690 : typing: Don't unnecessarily call `_get_protocol_attrs… …

51ca233

…` twice in `_ProtocolMeta.__instancecheck__`

AlexWaygood added type-feature A feature request or enhancement performance Performance or resource usage topic-typing 3.12 only security fixes labels Mar 30, 2023

AlexWaygood requested a review from carljm March 30, 2023 23:00

AlexWaygood requested review from Fidget-Spinner, JelleZijlstra and gvanrossum as code owners March 30, 2023 23:00

bedevere-bot mentioned this pull request Mar 30, 2023

Performance of typing._ProtocolMeta._get_protocol_attrs and isinstance #74690

Closed

bedevere-bot added the awaiting core review label Mar 30, 2023

JelleZijlstra reviewed Mar 30, 2023

View reviewed changes

Comment thread Lib/typing.py Outdated Show resolved Hide resolved

Address review

285a41b

AlexWaygood requested a review from JelleZijlstra March 31, 2023 09:33

AlexWaygood commented Mar 31, 2023

View reviewed changes

carljm approved these changes Mar 31, 2023

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting core review labels Mar 31, 2023

AlexWaygood added the skip news label Mar 31, 2023

AlexWaygood added 2 commits March 31, 2023 18:06

Delete 2023-03-30-23-51-22.gh-issue-74690.p2c4Co.rst

b6d1c95

Merge branch 'main' into protocol-attrs-collection

0f498a9

ilevkivskyi approved these changes Mar 31, 2023

View reviewed changes

AlexWaygood merged commit 9048d73 into python:main Mar 31, 2023

bedevere-bot removed the awaiting merge label Mar 31, 2023

AlexWaygood deleted the protocol-attrs-collection branch March 31, 2023 17:37

AlexWaygood mentioned this pull request Mar 31, 2023

gh-74690: Micro-optimise typing._get_protocol_attrs #103152

Merged

This was referenced Apr 2, 2023

gh-102433: Use inspect.getattr_static in typing._ProtocolMeta.__instancecheck__ #103034

Merged

gh-74690: Document changes made to runtime-checkable protocols in 3.12 #103348

Merged

AlexWaygood mentioned this pull request Apr 12, 2023

Backport performance improvements to runtime-checkable protocols python/typing_extensions#137

Merged

AlexWaygood mentioned this pull request May 24, 2023

[Bug] typing_extensions.Protocol unsupported under typing_extensions ≥ 4.6.0 beartype/beartype#241

Closed

                           return True
-                      if cls._is_protocol:
+                      protocol_attrs = _get_protocol_attrs(cls)

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

gh-74690: typing: Don't unnecessarily call _get_protocol_attrs twice in _ProtocolMeta.__instancecheck__#103141