So for any given classklass,_shadowed_dict(klass) shouldalways return the same result.

The__dict__ can't change, but the__mro__ can :/

>>> class A: pass...>>> class B: pass...>>> class C(A): pass...>>> C.__mro__(<class '__main__.C'>, <class '__main__.A'>, <class 'object'>)>>> C.__bases__ = (B,)>>> C.__mro__(<class '__main__.C'>, <class '__main__.B'>, <class 'object'>)

So I don't think this is safe.

The__dict__ can't change, but the__mro__ can :/

Well, good that we're discovering holes ingetattr_static test coverage :)

What about something like this, where_shadowed_dict is only cached for the lifetime of a singlegetattr_static call? It's slower than this patch currently, as it turns out that the call to.cache_clear() is pretty expensive. But it's still noticeably faster thanmain:

# add @lru_cache() to _shadowed_dict, like in this PR currentlydefgetattr_static(obj,attr,default=_sentinel):try:# existing getattr_static implementation herefinally:_shadowed_dict.cache_clear()

How does the performance comparison look if you make_shadowed_dict take the mro tuple rather than the class? I.e. call sites would turn into_shadowed_dict(_static_getmro(klass)).

I think if you did that, then it would be safe to put an LRU cache on_shadowed_dict.

I think it should be a bounded cache, though, not unbounded.

The pathological scenario would be someone creating lots of transient classes inside a function, and callinggetattr_static on instances of them. It seems prettu unlikely that someone is doing all of that, and expecting the transient classes to be GCed. But I think we at least should ensure the memory use in such a case is bounded.

I think it should be a bounded cache, though, not unbounded.

@functools.lru_cache() creates a cache with size 128 by default, so my PR already proposes a bounded cache ;)

>>> functools.lru_cache(lambda:1).cache_info()CacheInfo(hits=0, misses=0, maxsize=128, currsize=0)

How does the performance comparison look if you make_shadowed_dict take the mro tuple rather than the class? I.e. call sites would turn into_shadowed_dict(_static_getmro(klass)).

It's not as fast as this PR, but it's alot faster thanmain. I'll switch to that approach.

Here's the new performance numbers using thegetattr_static benchmark:

type[Foo]                :   61 ±  0 nsFoo                      :  100 ±  0 nstype[Bar]                :   61 ±  0 nsBar                      :  100 ±  0 nsWithParentClassX         :  129 ±  0 nsBaz                      :  118 ±  0 nsWithParentX              :  146 ±  0 nstype[Missing]            :  148 ±  0 nsMissing                  :  126 ±  1 nsSlotted                  :  162 ±  2 nsMethod                   :  105 ±  0 nsStMethod                 :  106 ±  0 nsClsMethod                :  100 ±  0 ns

The same benchmark onmain:

type[Foo]                :   90 ±  0 nsFoo                      :  163 ±  0 nstype[Bar]                :   90 ±  0 nsBar                      :  162 ±  0 nsWithParentClassX         :  229 ±  0 nsBaz                      :  209 ±  0 nsWithParentX              :  276 ±  0 nstype[Missing]            :  258 ±  0 nsMissing                  :  213 ±  0 nsSlotted                  :  203 ±  0 nsMethod                   :  162 ±  0 nsStMethod                 :  163 ±  0 nsClsMethod                :  163 ±  0 ns

With this PRnow,isinstance() checks against runtime-checkable protocols are faster than they were on 3.11 as long as the protocol has 13 members or fewer. Onmain, they are only faster than on 3.11 with 6 members or fewer.

Onmain, the results of thetyping_runtime_protocols pyperformance benchmark are262 us +- 11 us. With this PR, they are178 us +- 9 us.

(Skipping news because, if this patch is correct, it shouldn't change behaviour at all other than improving performance. And the news entry inhttps://github.com/python/cpython/pull/103195/files should already cover the performance boost.)

Thanks@carljm :D

I listed you as a co-author on the commit; I don't think I would have thought of using the mro tuple as a cache key on my own :)