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

perf: avoid returning future if requester wants #2955

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

maartenbreddels
Copy link
Contributor

Hi,

I was profiling with viztracer, and found this (the pink part on the right, ensure_future->create_task):
image

Writing to a websocket will always return a Future, but it's relatively costly, when sending a lot of them (63us).

I'm not sure this is a nice solution though.

@bdarnell
Copy link
Member

A boolean argument that changes the return type is not a great solution (and mypy doesn't like it).

I think a better long-term solution is to split the write_message interface into two variants: a coroutine version that must be awaited (and doesn't need to create a Future), and a plain-method version that cannot be awaited (and maybe keep the current may be awaited version as a third variant?) The biggest problem with this plan is just how to name the methods for it all to make sense.

Or there may be a better way to implement this "optional await" pattern. IOStream.write has essentially the same interface, but it doesn't call ensure_future, so it appears to be much more efficient. There's probably a way to do something similar here.

Also note that recent versions of python have sped up a lot of the asyncio internals, so if that profile wasn't from 3.9 it may look different on the latest version.

@maartenbreddels
Copy link
Contributor Author

Hi Ben,

Yes, I don't like the solution either, and indeed naming is hard. I think from a user perspective, I like the 'may be awaited for' as a default (that means keeping it as it is). The two others might be, ugly but explicit write_message_and_forget and write_message_async (and will show up in alphabetic ordering and tab-completion).

I'll also try out py39, thanks for the tip.

cheers,

Maarten

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

Successfully merging this pull request may close these issues.

2 participants