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.
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
Implement task dumps for current-thread runtime. #5608
Implement task dumps for current-thread runtime. #5608
Changes from 16 commits
1222dfb
d9aa093
66cd9c9
206b3d4
b998f74
8a727a1
80dbaf7
c132b33
9c86604
306a547
725bf3f
c20af63
52c8ca9
fdc01e1
144f378
c824389
0c655bb
578c3c4
ac7fa6b
a246b11
a62f9ee
f695499
bb6990f
51d15f0
178bf74
0a8d240
b6d093a
1ff355b
9e0dd91
9a5e7fa
f536db5
d3cda94
a29974d
51f970f
8a8dceb
b0b1160
8318d93
0dc4b68
04bd5e1
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.
One possible improvement here is to use impl Trait to simplify the definition of methods that return iterators. For example:
'_ means that the lifetime of the returned iterator is bound to the lifetime of the Tasks object
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.
I don't require this change for the PR to land, but generally, these inline cfg flags are hard to maintain. I tend to prefer using cfg to define APIs and then using the API regardless of the cfg in code like this. For example,
Trace::root
would always be defined, but when the task dump capability is not enabled, it would be a no-op and return the argument without modifying it.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.
Similarly, many of these cfgs could be reduced to a single
#[cfg(tokio_taskdump)]
sincelib.rs
contains acompile_error!
if you have#[cfg(tokio_taskdump)]
without the other options also set.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.
I'm concerned that reducing these to just
#[cfg(tokio_taskdump)]
wouldn't prevent errors in these locations under odd configurations, and thus would drown out the lone explicitcompile_error
inlib.rs
. I think Carl's suggestion is probably the way to go.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.
Not critical for this PR, but we probably want to reconsider the API details before stabilization.
Today,
tokio::runtime
does not have any public submodules. Instead, it is flat. That would apply to task dumps as well. We would need to spend a bit of time bikeshedding type names, though.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.
You will most likely have to send a signal somehow.