Title: gh-74690: typing: Simplify and optimise `_ProtocolMeta.__instancecheck__` by AlexWaygood · Pull Request #103159 · python/cpython · GitHub
Open Graph Title: gh-74690: typing: Simplify and optimise `_ProtocolMeta.__instancecheck__` by AlexWaygood · Pull Request #103159 · python/cpython
X Title: gh-74690: typing: Simplify and optimise `_ProtocolMeta.__instancecheck__` by AlexWaygood · Pull Request #103159 · python/cpython
Description: This implements the optimisation suggested by @ilevkivskyi in #74690 (comment). However, I'm not sure if it's the right thing to do. I'm posting it here for discussion, and so that people can have a play around with it. This dramatically speeds up isinstance() checks for nominal subclasses of runtime-checkable protocols and subclasses registered via ABCMeta.register, i.e. situations like the following: from typing import Protocol, runtime_checkable @runtime_checkable class HasX(Protocol) x: int class Bar(HasX): def __init__(self): self.x = 42 class Baz: ... HasX.register(Baz) isinstance(Bar(), HasX) # this is ~12x faster isinstance(Baz(), HasX) # this is ~2x faster However, speeding these two up comes at the cost of the performance of isinstance() checks with "structural subclasses". These are all slightly slower: class Eggs: def __init__(self): self.x = 42 class Spam: x = 42 class Ham: @property def x(self): return 42 isinstance(Eggs(), HasX) isinstance(Spam(), HasX) isinstance(Ham(), HasX) So the question is: is it worth making isinstance() calls against nominal and registered subclasses much, much faster, at the cost of making those^ isinstance() calls a bit slower? I'm not sure it is, as kind of the "whole point" of runtime-checkable protocols is that an object x doesn't have to directly inherit from a protocol X in order it to be considered an instance of X. So I don't know how common it is for people to directly inherit from protocols in their own code. But, I'm very curious about what other people think. Here's a benchmarking script for people to play around with. The results of the benchmark script for the structural-subclass cases are a bit noisy on my machine, but consistently a little bit slower than on main: 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") Issue: gh-74690
Open Graph Description: This implements the optimisation suggested by @ilevkivskyi in #74690 (comment). However, I'm not sure if it's the right thing to do. I'm posting it here for discussion, and so that peop...
X Description: This implements the optimisation suggested by @ilevkivskyi in #74690 (comment). However, I'm not sure if it's the right thing to do. I'm posting it here for discussion, and ...
Opengraph URL: https://github.com/python/cpython/pull/103159
X: @github
Domain: github.com
| route-pattern | /:user_id/:repository/pull/:id/files(.:format) |
| route-controller | pull_requests |
| route-action | files |
| fetch-nonce | v2:e4e27d4f-ca2e-d587-6462-ce449fb4e2fa |
| current-catalog-service-hash | ae870bc5e265a340912cde392f23dad3671a0a881730ffdadd82f2f57d81641b |
| request-id | A6A2:3545E1:357221:45B229:696ADD8F |
| html-safe-nonce | 0e8d9808df9984969a13b2bd16b809bd23a76261d2f8857bbd55f6f997b646b4 |
| visitor-payload | eyJyZWZlcnJlciI6IiIsInJlcXVlc3RfaWQiOiJBNkEyOjM1NDVFMTozNTcyMjE6NDVCMjI5OjY5NkFERDhGIiwidmlzaXRvcl9pZCI6IjUxMDk5OTcyOTk5MjgwNjMzNzUiLCJyZWdpb25fZWRnZSI6ImlhZCIsInJlZ2lvbl9yZW5kZXIiOiJpYWQifQ== |
| visitor-hmac | 9b766d1e11f1393647a891270a8351b8547915190751de04076db48f112cf35d |
| hovercard-subject-tag | pull_request:1298449918 |
| github-keyboard-shortcuts | repository,pull-request-list,pull-request-conversation,pull-request-files-changed,copilot |
| google-site-verification | Apib7-x98H0j5cPqHWwSMm6dNU4GmODRoqxLiDzdx9I |
| octolytics-url | https://collector.github.com/github/collect |
| analytics-location | / |
| fb:app_id | 1401488693436528 |
| apple-itunes-app | app-id=1477376905, app-argument=https://github.com/python/cpython/pull/103159/files |
| twitter:image | https://avatars.githubusercontent.com/u/66076021?s=400&v=4 |
| twitter:card | summary_large_image |
| og:image | https://avatars.githubusercontent.com/u/66076021?s=400&v=4 |
| og:image:alt | This implements the optimisation suggested by @ilevkivskyi in #74690 (comment). However, I'm not sure if it's the right thing to do. I'm posting it here for discussion, and so that peop... |
| og:site_name | GitHub |
| og:type | object |
| hostname | github.com |
| expected-hostname | github.com |
| None | 5f99f7c1d70f01da5b93e5ca90303359738944d8ab470e396496262c66e60b8d |
| turbo-cache-control | no-preview |
| diff-view | unified |
| go-import | github.com/python/cpython git https://github.com/python/cpython.git |
| octolytics-dimension-user_id | 1525981 |
| octolytics-dimension-user_login | python |
| octolytics-dimension-repository_id | 81598961 |
| octolytics-dimension-repository_nwo | python/cpython |
| octolytics-dimension-repository_public | true |
| octolytics-dimension-repository_is_fork | false |
| octolytics-dimension-repository_network_root_id | 81598961 |
| octolytics-dimension-repository_network_root_nwo | python/cpython |
| turbo-body-classes | logged-out env-production page-responsive |
| disable-turbo | true |
| browser-stats-url | https://api.github.com/_private/browser/stats |
| browser-errors-url | https://api.github.com/_private/browser/errors |
| release | 524a93f2c1f36522a3b4be4c110467ee4172245d |
| ui-target | full |
| theme-color | #1e2327 |
| color-scheme | light dark |
Links:
Viewport: width=device-width