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

Poor collection performance due to expensive iscoroutinefunction check #720

Open
etripier opened this issue Dec 13, 2023 · 1 comment
Open
Milestone

Comments

@etripier
Copy link

Hello pytest-asyncio developers! We've been using your plugin for a while. So much so that we now have tens of thousands of tests and hundreds of thousands of fixture arguments and ... it takes over three minutes to collect our tests.

Of that time, roughly two and a half minutes is taken up by pytest-asyncio. Specifically, this piece of code:

if fixturedef in processed_fixturedefs or not _is_coroutine_or_asyncgen(
.

That ends up calling https://github.com/python/cpython/blob/3aea6c4823e90172c9bc36cd20dc51b295d8a3c4/Lib/inspect.py#L384, which unwraps each fixture argument, makes sure it's a function, and then finally checks its co_flags to see if it's been marked as a coroutine function. The thing is, because you're feeding fixtures through, they tend to all be functions.

We ended up forking the plugin and inlining iscoroutinefunction and isasyncgenfunction - just the bit masking lines. Then we removed the code that was attaching the scoped_event_loop fixture because we don't use it and it was compounding the issue described above. That took us down to one minute.

I think it would help a lot to make the same changes here upstream. You could also swap the ordering of the comparison with

if not _is_asyncio_fixture_function(func) and asyncio_mode == Mode.STRICT:
, which would allow strict mode users to shortcut the expensive hot path. Finally, making scoped_event_loop optional would bring your plugin even closer to baseline performance.

@seifertm
Copy link
Contributor

Thanks for the investigation! I suspected that pytest-asyncio will have to check its performance someday.

There's currently no setup to do any performance testing, though. I promise to look into it and I'm all for providing the functionality (and quality) so you don't have to maintain a custom fork, but I cannot give any time frame. The v0.23 release is plagued with functional issues, which are of higher priority at the moment.

Switching around the if statement as you suggested can be done immediately, though.

@seifertm seifertm added this to the v1.0 milestone Jan 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants