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

task: add task size to tracing instrumentation #6881

Merged
merged 6 commits into from
Oct 8, 2024
Merged

task: add task size to tracing instrumentation #6881

merged 6 commits into from
Oct 8, 2024

Conversation

hds
Copy link
Contributor

@hds hds commented Oct 1, 2024

Motivation

In Tokio, the futures for tasks are stored on the stack unless they are
explicitly boxed, either by the user or auto-boxed by Tokio when they
are especially large. Auto-boxing now also occurs in release mode
(since #6826).

Having very large futures can be problematic as it can cause a stack
overflow. In some cases it might be desireable to have smaller futures,
even if they are placed on the heap.

Solution

This change adds the size of the future driving an async task or the
function driving a blocking task to the tracing instrumentation. In the
case of a future that is auto-boxed by Tokio, both the final size as well
the original size before boxing is included.

To do this, a new struct SpawnMeta gets passed down from where a
future might get boxed to where the instrumentation is added. This
contains the task name (optionally) and the original future or function
size. If the tokio_unstable cfg flag and the tracing feature aren't both
enabled, then this struct will be zero sized, which is a small improvement
on the previous behavior of unconditionally passing down an Option<&str>
for the name.

This will make this information immediately available in Tokio Console,
and will enable new lints which will warn users if they have large futures
(just for async tasks).

We have some tests under the tracing-instrumentation crate which test
that the size.bytes and original_size.bytes fields are set correctly.

The minimal version of tracing required for Tokio has been bumped from
0.1.25 to 0.1.29 to get the Value impl on Option<T>. Given that the current
version is 0.1.40, this seems reasonable, especially given that Tracing's MSRV
is still lower than Tokio's in the latest version.

PR Notes

Here's a preview of Tokio Console using this new instrumentation
(purple arrows are my addition).

tasks_screen blocking_task_detail_screen

In Tokio, the futures for tasks are stored on the stack unless they are
explicitly boxed (caveat in debug mode, see below). Having very large
futures can be problematic as it can cause a stack overflow.

This change adds the size of the future driving an async task or the
function driving a blocking task to the tracing instrumentation. This
will make this information immediately available in Tokio Console, and
will enable a new lint which will warn users if they have large futures
(just for async tasks).
hds added a commit to tokio-rs/console that referenced this pull request Oct 1, 2024
In Tokio, the futures for tasks are stored on the stack unless they are
explicitly boxed. Having very large futures can be problematic as it can
cause a stack overflow.

This change makes use of new instrumentation in Tokio
(tokio-rs/tokio#6881) which includes the size of the future which drives
a task. The size isn't given its own column (we're running out of
horizontal space) and appears in the additional fields column.

A new lint has been added for large futures. By default it will show a
warning when a task's future is greater than or equal to 2048 bytes.
This value was chosen as it is the threshold at which Tokio will box a
future in debug mode. The lint has been added to the list of default
lints.
@Darksonn Darksonn added A-tokio Area: The main tokio crate M-tracing Tracing support in Tokio labels Oct 1, 2024
@Darksonn
Copy link
Contributor

Darksonn commented Oct 1, 2024

We sometimes send large futures to the heap internally (#6826). How does this interact with that? Does it show the size of the future or just the box?

@hds
Copy link
Contributor Author

hds commented Oct 1, 2024

I developed this against a slightly older version of the code (I had the tokio 1.40.0 tag checked out) and hadn't seen this. Thanks for the heads up.

This feature would take the stack size of the future which is actually spawned - which means that in the case of auto-boxing, the box would be measured, not the future in the box. I believe that this is the right information, since ultimately we're checking for cases where a future might be too large and cause a stack overflow.

Of course, with this new behaviour, I'm not sure how valuable the tokio console lint is, since we will essentially only be catching values between the lint's "maximum reasonable future size" (which would default to 2048 and therefore never trigger in debug builds) and Tokio's new release box threshold of 16384. Anything higher than that will be auto-boxed and appear smaller.

Perhaps it would make sense to unify all the auto-boxing logic into the same place that we perform tracing and then we can record both the original future size and the final future size in the instrumentation.

@Darksonn
Copy link
Contributor

Darksonn commented Oct 1, 2024

The behavior has always existed on debug mode. The change I referenced just makes it apply in release mode too.

Either way, happy to change how the logic happens. But it's important that the auto-boxing happens without a lot of function calls above it, because each non-inlined function call increases the stack space used by the size of the future.

@hds
Copy link
Contributor Author

hds commented Oct 1, 2024

Yeah, I was aware of the behaviour in debug mode, but thought that this might be a nice runtime lint for release mode.

Right now I can't find a way to do this nicely without passing the original size down through the whole stack of function calls along side the future, which would be a bit messy (although we're already doing it with the name in some cases).

@hds
Copy link
Contributor Author

hds commented Oct 1, 2024

@Darksonn how would this approach work? I've generalized passing a name to the spawn functions with a SpawnMeta struct that can also contain the original future/function size.

(I'll fix the formatting issues if I'm on the right path, it needs some tracing instrumentation tests as well)

@Darksonn
Copy link
Contributor

Darksonn commented Oct 2, 2024

I think using a SpawnMeta to carry this information is a good idea.

@hds
Copy link
Contributor Author

hds commented Oct 2, 2024

Great, thanks, I'll finish off the PR and mark it as ready then.

Nice side effect is that SpawnMeta will be zero size when tokio_unstable and tracing are not enabled, which is a (small) improvement over always sending Option<&str>.

@hds hds marked this pull request as draft October 2, 2024 09:40
No point setting it if it's the same as the size.bytes field.
This is needed to get have Value implemented for Option<T>.

Also fix tracing instrumentation tests now that original_size.bytes
won't be recorded if it's the same as size.bytes.
@hds hds marked this pull request as ready for review October 2, 2024 16:19
@hds
Copy link
Contributor Author

hds commented Oct 2, 2024

@Darksonn I've cleaned it all up and updated the PR comment.

@Darksonn Darksonn mentioned this pull request Oct 4, 2024
@hds hds merged commit c3a9355 into master Oct 8, 2024
83 checks passed
@hds hds deleted the hds/task-size branch October 8, 2024 08:51
hds added a commit to tokio-rs/console that referenced this pull request Oct 8, 2024
In Tokio, the futures for tasks are stored on the stack unless they are
explicitly boxed. Having very large futures can be problematic as it can
cause a stack overflow.

This change makes use of new instrumentation in Tokio
(tokio-rs/tokio#6881) which includes the size of the future which drives
a task. The size isn't given its own column (we're running out of
horizontal space) and appears in the additional fields column. In the
case that a future was auto-boxed by Tokio, the original size of the
task will also be provided.

Two new lints have been added for large futures. The first will detect
auto-boxed futures and warn about them. The second will detect futures
which are large (over 1024 bytes by default), but have not been
auto-boxed by the runtime.

Since the new lints depend on the new instrumentation in Tokio, they
will only trigger if the instrumented application is running using
`tokio` 1.41.0 or later. The version is as yet, unreleased.

Both lints have been added to the default list.
hds added a commit to tokio-rs/console that referenced this pull request Oct 8, 2024
In Tokio, the futures for tasks are stored on the stack unless they are
explicitly boxed. Having very large futures can be problematic as it can
cause a stack overflow.

This change makes use of new instrumentation in Tokio
(tokio-rs/tokio#6881) which includes the size of the future which drives
a task. The size isn't given its own column (we're running out of
horizontal space) and appears in the additional fields column. In the
case that a future was auto-boxed by Tokio, the original size of the
task will also be provided.

Two new lints have been added for large futures. The first will detect
auto-boxed futures and warn about them. The second will detect futures
which are large (over 1024 bytes by default), but have not been
auto-boxed by the runtime.

Since the new lints depend on the new instrumentation in Tokio, they
will only trigger if the instrumented application is running using
`tokio` 1.41.0 or later. The version is as yet, unreleased.

Both lints have been added to the default list.
hds added a commit to tokio-rs/console that referenced this pull request Oct 8, 2024
In Tokio, the futures for tasks are stored on the stack unless they are
explicitly boxed. Having very large futures can be problematic as it can
cause a stack overflow.

This change makes use of new instrumentation in Tokio
(tokio-rs/tokio#6881) which includes the size of the future which drives
a task. The size isn't given its own column (we're running out of
horizontal space) and appears in the additional fields column. In the
case that a future was auto-boxed by Tokio, the original size of the
task will also be provided.

Two new lints have been added for large futures. The first will detect
auto-boxed futures and warn about them. The second will detect futures
which are large (over 1024 bytes by default), but have not been
auto-boxed by the runtime.

Since the new lints depend on the new instrumentation in Tokio, they
will only trigger if the instrumented application is running using
`tokio` 1.41.0 or later. The version is as yet, unreleased.

Both lints have been added to the default list.
hds added a commit to tokio-rs/console that referenced this pull request Oct 8, 2024
In Tokio, the futures for tasks are stored on the stack unless they are
explicitly boxed. Having very large futures can be problematic as it can
cause a stack overflow.

This change makes use of new instrumentation in Tokio
(tokio-rs/tokio#6881) which includes the size of the future which drives
a task. The size isn't given its own column (we're running out of
horizontal space) and appears in the additional fields column. In the
case that a future was auto-boxed by Tokio, the original size of the
task will also be provided.

Two new lints have been added for large futures. The first will detect
auto-boxed futures and warn about them. The second will detect futures
which are large (over 1024 bytes by default), but have not been
auto-boxed by the runtime.

Since the new lints depend on the new instrumentation in Tokio, they
will only trigger if the instrumented application is running using
`tokio` 1.41.0 or later. The version is as yet, unreleased.

Both lints have been added to the default list.
hds added a commit to tokio-rs/console that referenced this pull request Oct 9, 2024
In Tokio, the futures for tasks are stored on the stack unless they are
explicitly boxed. Having very large futures can be problematic as it can
cause a stack overflow.

This change makes use of new instrumentation in Tokio
(tokio-rs/tokio#6881) which includes the size of the future which drives
a task. The size isn't given its own column (we're running out of
horizontal space) and appears in the additional fields column. In the
case that a future was auto-boxed by Tokio, the original size of the
task will also be provided.

Two new lints have been added for large futures. The first will detect
auto-boxed futures and warn about them. The second will detect futures
which are large (over 1024 bytes by default), but have not been
auto-boxed by the runtime.

Since the new lints depend on the new instrumentation in Tokio, they
will only trigger if the instrumented application is running using
`tokio` 1.41.0 or later. The version is as yet, unreleased.

Both lints have been added to the default list.

Co-authored-by: Eliza Weisman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate M-tracing Tracing support in Tokio
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants