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.
GH-91048: Add utils for capturing async call stack for asyncio programs and enable profiling #124640
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
GH-91048: Add utils for capturing async call stack for asyncio programs and enable profiling #124640
Changes from 48 commits
1b01a91
0fc5511
1d20a51
c8be18e
abf2cb9
20ceab7
72d9321
c9475f6
e1099e9
817f88b
54386ac
98434f0
485c166
8802be7
1ddc9cf
bc9beb8
fd141d4
2d72f24
391defa
d6357fd
bb3b6df
54c99ec
c1a4f09
027d522
c2d5ec6
08d09eb
fe3113b
18ec26d
e4cc462
d5cdc36
83606f2
5edac41
8dc6d34
30884ea
1317658
81b0a31
258ce3d
b9ecefb
b77dcb0
8867946
87d2524
b47bef1
230b7ec
b1d6158
ac51364
c7e59eb
9eba5e1
59121f6
f8f48f0
74c5ad1
067c043
9f04911
0774805
3048493
1f42873
7799391
03ed5c1
21f9ea9
8a43dfa
b3fae68
d0aedf0
df0032a
0ce241b
8f126f6
966d84e
f56468a
404b88a
911fed8
ab511a4
c3c685a
785adeb
a577328
064129a
ce332d9
d6d943f
703ff46
e867863
9cb5b29
61b2b7b
9533ab9
596191d
ad9152e
066bf21
4caeec4
38f061d
a8dd667
cf8f5e5
eda9c7c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 is adding yet another field generators for the use of async.
Remember that generators are used as iterators, probably more often than they are used for coroutines.
For code like
any(expr for var in seq)
andtuple(expr for var in seq)
this is unwanted expense.Is there no way to add this tasks, not generators?
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.
Also, I don't see how it is safe for this to be a borrowed reference. What guarantee is there that the object passed to
_PyCoro_SetTask
will outlive the generator?If the answer is "_PyCoro_SetTask is an internal function", then why is it used by asyncio which is a module?
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.
AFAICT there is too much entanglement between generators and coroutines to split their implementation entirely to avoid this. How can we measure what this unwanted expense actually is?
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.
Splitting their implementations is desirable, but I appreciate that it won't happen in this PR.
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.
cr_task
is a borrowed reference, and is exposed through a public API, which is unsafe.So it will need to made a strong reference.
That means that it not only does it takes space and needs initializing, it will also need to be cleared when the generator is destroyed.
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.
@markshannon we agree with your feedback. I'll refactor the PR to remove the need for
cr_task
(and therefore thegi_task
debug offset). We'll maintain a list of running tasks in _asynciomodule.c since this is asyncio-specific anyway.