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

Test usage of proactor event loop #590

Closed
wants to merge 2 commits into from

Conversation

blink1073
Copy link
Contributor

@blink1073
Copy link
Contributor Author

@ccordoba12 do you have any concerns about this?

@blink1073
Copy link
Contributor Author

For context, the ability to set an event loop policy might be going away.

@ccordoba12
Copy link
Collaborator

I thought this was necessary on Windows for Python 3.8+. How did it get fix?

@blink1073
Copy link
Contributor Author

blink1073 commented Nov 15, 2023

Hey @ccordoba12, tornado 6.1 added support for the proactor loop, by starting a thread. The jupyter_server test suite runs in about 10 min with the selector loop, and 15 min with the proactor loop.

Guido suggested that if we can switch from ioloop.start() to calling asyncio.run(loop_factory=...) ourselves, we could retain the selector loop.

@ccordoba12
Copy link
Collaborator

@blink1073, thanks for the explanation. I must confess my ignorance about most asyncio related stuff because I haven't had time to learn about it.

So, if you consider this is no longer necessary then I'm ok with removing it.

@blink1073
Copy link
Contributor Author

blink1073 commented Nov 16, 2023

So, if you consider this is no longer necessary then I'm ok with removing it.

I still need to do more testing to make sure we're future-proof (if they take away set_event_loop_policy), but thanks!

@blink1073 blink1073 closed this Nov 16, 2023
@blink1073 blink1073 deleted the test-proactor-event-loop branch November 16, 2023 14:23
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 this pull request may close these issues.

2 participants