Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
gh-128002: use per threads tasks linked list in asyncio#128869
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
kumaraditya303 commented Jan 15, 2025 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
1st1 commented Jan 15, 2025
First, great work on this, this is legitimately a cool PR. That said, I'm feeling really uneasy about Also please wait for reviews from @pablogsal and @ambv. I'm curious if this would make external introspection harder. |
Uh oh!
There was an error while loading. Please reload this page.
graingert commented Jan 16, 2025
Does this work where an event loop is used on one thread, stopped then resumed on another thread? |
This comment was marked as resolved.
This comment was marked as resolved.
kumaraditya303 commented Jan 21, 2025
I have pushed some more changes:
TODO: benchmark it before merging |
pablogsal left a comment • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
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.
If I am not mistaken this solution seems incompatible with the asyncio introspecction workflow we are adding on #124640. Please, ensure that this change is compatible with the changes in that PR to avoid problems in the future.
When you're done making the requested changes, leave the comment: |
kumaraditya303 commented Jan 22, 2025
This PR has nothing to do with asyncio introspection. As I said in other PR, the change which would affect that is moving current task to per-loop which isn't done in this PR. |
pablogsal commented Jan 22, 2025
Hummm, I must have misread how this affects the task management. Let me dismiss my request for changes meanwhile. Thanks for the patience with this @kumaraditya303! |
kumaraditya303 commented Jan 27, 2025
@colesbury PTAL |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
colesbury left a comment
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 LGTM, but would you please also get this reviewed by another asyncio expert?
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
0d68b14 into python:mainUh oh!
There was an error while loading. Please reload this page.
…#128869) Co-authored-by: Łukasz Langa <lukasz@langa.pl>
…#128869) Co-authored-by: Łukasz Langa <lukasz@langa.pl>
Use per-thread linked list of tasks in asyncio. This design allows for lock free register/unregister of tasks of loops running concurrently in different threads. It uses the stop the world pause to traverse the list of tasks from all threads from the thread where
all_tasksis called. This has no performance impact on regular builds as per benchmarks and performs a bit faster on free-threading benchmarks. pyperformance benchmarks aren't good for this because it uses just one thread so there is little lock contention, this however performs much better when multiple threads are running.On free-threading:
Benchmark hidden because not significant (1): asyncio_websockets