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

Speed up Deferred: isinstance() is slow #12223

Closed
itamarst opened this issue Jun 26, 2024 · 3 comments · Fixed by #12224
Closed

Speed up Deferred: isinstance() is slow #12223

itamarst opened this issue Jun 26, 2024 · 3 comments · Fixed by #12224
Assignees

Comments

@itamarst
Copy link
Contributor

Deferred callback does an instance(result, Deferred) in the hot path, which is a pretty expensive operation.

@itamarst itamarst self-assigned this Jun 26, 2024
@glyph
Copy link
Member

glyph commented Jun 26, 2024

IIRC we used to have a type(...) is check here, and changed it because we didn't have benchmarks to justify it being all that slow, at around the time DeferredList got added. I gather inheriting from Awaitable might have changed the calculus here?

@arnimarj
Copy link

I got curious and created this:

import timeit


setup = 'from twisted.internet.defer import Deferred\nx = Deferred()\nb=False'

fast = '''
if x.__class__ is Deferred or isinstance(x, Deferred):
	b = True
'''


slow = '''
if isinstance(x, Deferred):
	b = True
'''

N = 10_000_000

T = timeit.timeit(stmt=fast, number=N, setup=setup)
print(T)

T = timeit.timeit(stmt=slow, number=N, setup=setup)
print(T)

...which gave me approximately this:

0.09645763300068211
0.1443577739992179

@glyph
Copy link
Member

glyph commented Jun 26, 2024

I got curious and created this:

The problem with this benchmark is that it goes faster only if x is a Deferred. In the case we are doing the isinstance check, it's actually much more common that x is not a Deferred.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants