-
Notifications
You must be signed in to change notification settings - Fork 78
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
Add bench_async_func() #124
Conversation
pyperf/_runner.py
Outdated
# use fast local variables | ||
local_timer = time.perf_counter | ||
local_func = func | ||
loop = asyncio.get_event_loop() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to attempt reimplementing asyncio.run() here, but only measure run_until_complete() timing? I'm talking about creating a fresh event loop each time, and shutdown asynchronous generators and the default executor. Well, same things than asyncio.run().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I use asyncio.run() instead of reimplement it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please add a changelog entry in doc/changelog.rst? You need to add Version 2.3.1 section.
I tested the example on my Fedora 35 with Python 3.10. I get:
That's interesting :-) Using
|
Oh, sadly pyperf currently still supports Python 3.6: https://pyperf.readthedocs.io/en/latest/run_benchmark.html#install-pyperf |
pyperf/_runner.py
Outdated
return dt | ||
|
||
import asyncio | ||
try: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer to testing hasattr(asyncio, 'main')
, since main() can raise AttributeError.
doc/api.rst
Outdated
|
||
.. note:: | ||
If the ``func()`` leaves any tasks, it affects to next iteration. | ||
So you should care about task leak for reliable benchmark. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This note is now outdated, since a new event loop is created at each iteration, no?
With the proposed implementation (measure time inside asyncio.run and not outside), I agree that the implementation increases the accuracy and so it's worth it to add a new method ;-) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the updates!
I completed the changelog in commit 0814a4a. |
This change is now part of the just released pyperf 2.3.1. |
Ref #121.