GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-128002: use per threads tasks linked list in asyncio by kumaraditya303 · Pull Request #128869 · python/cpython

Conversation

@kumaraditya303

@kumaraditya303 kumaraditya303 commented Jan 15, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor
  • Issue: Audit asyncio thread safety  #128002

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_tasks is 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 bm-20250111-linux-x86_64-python-3a570c6d58bd5ad7d7c1-3.14.0a3+-3a570c6 bm-20250112-linux-x86_64-kumaraditya303-per_thread_tasks-3.14.0a3+-cca4057
async_tree_cpu_io_mixed_tg 556 ms 536 ms: 1.04x faster
async_tree_none_tg 303 ms 294 ms: 1.03x faster
coroutines 26.2 ms 25.5 ms: 1.03x faster
async_tree_memoization_tg 397 ms 387 ms: 1.03x faster
async_tree_cpu_io_mixed 598 ms 583 ms: 1.03x faster
async_generators 498 ms 486 ms: 1.02x faster
async_tree_io 748 ms 733 ms: 1.02x faster
async_tree_io_tg 696 ms 682 ms: 1.02x faster
async_tree_memoization 442 ms 436 ms: 1.02x faster
async_tree_none 349 ms 344 ms: 1.01x faster
Geometric mean (ref) 1.02x faster

Benchmark hidden because not significant (1): asyncio_websockets

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
kumaraditya303 added 4 commits January 15, 2025 13:12
@kumaraditya303
switch to per thread tasks list
1a39d41
@kumaraditya303
traverse linked lists of all threads
28735e7
@kumaraditya303
cleanup
3052330
@kumaraditya303
add comments
acef821
@kumaraditya303 kumaraditya303 added topic-asyncio skip news topic-free-threading labels Jan 15, 2025
@kumaraditya303
kumaraditya303 requested review from 1st1, asvetlov, ericsnowcurrently and willingc as code owners January 15, 2025 13:38
@bedevere-app bedevere-app Bot mentioned this pull request Jan 15, 2025
Audit asyncio thread safety #128002
Closed
9 tasks done
@bedevere-app bedevere-app Bot added the awaiting core review label Jan 15, 2025
@kumaraditya303
kumaraditya303 requested a review from colesbury January 15, 2025 15:35
@1st1
1st1 requested review from ambv and pablogsal January 15, 2025 18:26
@1st1

1st1 commented Jan 15, 2025

Copy link
Copy Markdown
Member

First, great work on this, this is legitimately a cool PR.

That said, I'm feeling really uneasy about _PyEval_StopTheWorld. Maybe instead of this approach we try spin locks + a custom mini hash map data structure? That would obviously be a lot more work, but a cleaner and ultimately better perf approach.

Also please wait for reviews from @pablogsal and @ambv. I'm curious if this would make external introspection harder.
ambv reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
colesbury
colesbury reviewed Jan 15, 2025
View reviewed changes
Comment thread Python/pystate.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
@graingert

graingert commented Jan 16, 2025

Copy link
Copy Markdown
Contributor

Does this work where an event loop is used on one thread, stopped then resumed on another thread?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@graingert

This comment was marked as resolved.

Sign in to view
kumaraditya303 added 9 commits January 16, 2025 17:36
@kumaraditya303
add a per interp tasks list
237a089
@kumaraditya303
fixup for regular builds
c12f271
@kumaraditya303
fix missing start world
d36270f
@kumaraditya303
Merge branch 'main' of https://github.com/python/cpython into per-thr…
f863cd5 
…ead-tasks
@kumaraditya303
add interp llist
d0fbbc2
@kumaraditya303
add more tests
5f5b95e
@kumaraditya303
fix test
708b410
@kumaraditya303
try fix compilation
2c1e93e
@kumaraditya303
remove supp
c0ce361
@kumaraditya303

kumaraditya303 commented Jan 21, 2025

Copy link
Copy Markdown
Contributor Author

I have pushed some more changes:

  • Added a interpreter list of tasks which gets used when a task is alive but the thread state gets deallocated as suggested by @colesbury
  • Added thread id tracking in free-threading builds to avoid locking in the general case of loops running independently in threads without sharing tasks
  • Added some tests

TODO: benchmark it before merging
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot added the awaiting changes label Jan 22, 2025
@bedevere-app

bedevere-app Bot commented Jan 22, 2025

Copy link
Copy Markdown

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@kumaraditya303

kumaraditya303 commented Jan 22, 2025

Copy link
Copy Markdown
Contributor Author

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.

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ambv ambv mentioned this pull request Jan 22, 2025
GH-91048: Add utils for capturing async call stack for asyncio programs and enable profiling #124640
Merged
5 of 6 tasks
@pablogsal
pablogsal dismissed their stale review January 22, 2025 17:00

.

@bedevere-app bedevere-app Bot added awaiting review and removed awaiting changes labels Jan 22, 2025
@pablogsal

pablogsal commented Jan 22, 2025

Copy link
Copy Markdown
Member

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!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
kumaraditya303 added 4 commits January 24, 2025 16:05
@kumaraditya303
Merge branch 'main' of https://github.com/python/cpython into per-thr…
aa4c2d1 
…ead-tasks
@kumaraditya303
Merge branch 'main' of https://github.com/python/cpython into per-thr…
fd3fc0b 
…ead-tasks
@kumaraditya303
fix merge
c45c9d7
@kumaraditya303
Merge branch 'main' into per-thread-tasks
f0a9e53
@kumaraditya303

kumaraditya303 commented Jan 27, 2025

Copy link
Copy Markdown
Contributor Author

@colesbury PTAL
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
colesbury
colesbury reviewed Jan 28, 2025
View reviewed changes
Comment thread Modules/_asynciomodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Python/pystate.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
kumaraditya303 added 3 commits January 29, 2025 12:37
@kumaraditya303
code review
5b33c45
@kumaraditya303
cleanup
1d73843
@kumaraditya303
add comment for asyncio_tasks_lock
4014e70
@kumaraditya303
kumaraditya303 requested a review from colesbury January 29, 2025 15:42
colesbury
colesbury reviewed Jan 30, 2025
View reviewed changes

@colesbury colesbury left a comment

Copy link
Copy Markdown
Contributor

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?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
ambv
ambv reviewed Feb 6, 2025
View reviewed changes
Comment thread Modules/_asynciomodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
ambv
ambv reviewed Feb 6, 2025
View reviewed changes
Comment thread Modules/_asynciomodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
ambv added 2 commits February 6, 2025 13:20
@ambv
Reword explanation
83aec26
@ambv
Merge branch 'main' into per-thread-tasks
e52741f
@ambv
ambv merged commit 0d68b14 into python:main Feb 6, 2025
@bedevere-app bedevere-app Bot removed the awaiting review label Feb 6, 2025
srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this pull request Feb 7, 2025
@kumaraditya303 @ambv @srinivasreddy
pythongh-128002: use per threads tasks linked list in asyncio (python…
1b70da5 
…#128869)

Co-authored-by: Łukasz Langa <[email protected]>
@kumaraditya303
kumaraditya303 deleted the per-thread-tasks branch February 7, 2025 11:59
cmaloney pushed a commit to cmaloney/cpython that referenced this pull request Feb 8, 2025
@kumaraditya303 @ambv @cmaloney
pythongh-128002: use per threads tasks linked list in asyncio (python…
d6bd7b7 
…#128869)

Co-authored-by: Łukasz Langa <[email protected]>
@krassowski krassowski mentioned this pull request Oct 8, 2026
asyncio Task created from a finalizer during PyThreadState_Clear() registers into memory that is about to be freed (3.14 regression) #159041
Closed
@taegyunkim taegyunkim mentioned this pull request Oct 21, 2026
Make asyncio debug offsets reachable from _PyRuntime and give them a cookie, version and size #160107
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ambv ambv ambv left review comments
@colesbury colesbury colesbury left review comments
@pablogsal pablogsal pablogsal left review comments
@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner
@1st1 1st1 Awaiting requested review from 1st1 1st1 is a code owner
@asvetlov asvetlov Awaiting requested review from asvetlov asvetlov is a code owner
@willingc willingc Awaiting requested review from willingc willingc is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news topic-asyncio topic-free-threading

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@kumaraditya303 @1st1 @graingert @pablogsal @ambv @colesbury
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.