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-103193: Speedup and inline inspect._is_type #103321

Merged
merged 1 commit into from
Apr 6, 2023

Conversation

AlexWaygood
Copy link
Member

@AlexWaygood AlexWaygood commented Apr 6, 2023

(Using @sobolevn's benchmark script from #103193 (comment).)

Benchmarks on main:

type[Foo]                :   74 ±  0 ns
Foo                      :  177 ±  1 ns
type[Bar]                :   74 ±  0 ns
Bar                      :  177 ±  0 ns
WithParentClassX         :  229 ±  0 ns
Baz                      :  216 ±  0 ns
WithParentX              :  266 ±  1 ns
type[Missing]            :  210 ±  0 ns
Missing                  :  219 ±  1 ns
Slotted                  :  214 ±  0 ns
Method                   :  178 ±  0 ns
StMethod                 :  178 ±  0 ns
ClsMethod                :  178 ±  1 ns

Benchmarks with this PR:

type[Foo]                :   74 ±  0 ns
Foo                      :  134 ±  0 ns
type[Bar]                :   74 ±  0 ns
Bar                      :  133 ±  0 ns
WithParentClassX         :  187 ±  0 ns
Baz                      :  171 ±  0 ns
WithParentX              :  220 ±  0 ns
type[Missing]            :  210 ±  1 ns
Missing                  :  174 ±  0 ns
Slotted                  :  169 ±  0 ns
Method                   :  133 ±  0 ns
StMethod                 :  133 ±  0 ns
ClsMethod                :  133 ±  0 ns

This speeds up this isinstance() call 1.25x relative to main:

from typing import *

@runtime_checkable
class Foo(Protocol):
    a: int
    b: int
    c: int
    d: int
    e: int
    f: int
    g: int
    h: int
    i: int
    j: int

class Bar:
    def __init__(self):
        for attrname in 'abcdefghij':
            setattr(self, attrname, 42)

isinstance(Bar(), Foo)

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

@carljm carljm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The improvements keep on coming!

Comment on lines +1818 to +1820
objtype = type(obj)
if type not in _static_getmro(objtype):
klass = objtype
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has equivalent semantics with less code and fewer assignments, but perhaps it muddies the semantics of the klass var a bit (since it will temporarily be "wrong" in the case that obj is already a type.) No strong feelings, whatever you prefer.

Suggested change
objtype = type(obj)
if type not in _static_getmro(objtype):
klass = objtype
klass = type(obj)
if type not in _static_getmro(klass):

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. Yeah, I see what you mean, but I think I'd prefer to keep the semantics of klass clear here :)

@AlexWaygood AlexWaygood merged commit dca7d17 into python:main Apr 6, 2023
@AlexWaygood AlexWaygood deleted the speedup-inspect-istype branch April 6, 2023 20:50
@AlexWaygood
Copy link
Member Author

The improvements keep on coming!

Optimisations will continue until morale improves!

@namka279

This comment was marked as spam.

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 performance Performance or resource usage skip news stdlib Python modules in the Lib dir type-feature A feature request or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants