feat(console): add large future lints #587
Conversation
|
I'll keep this in draft until the Tokio PR is merged, as this feature is useless without it. |
hds
force-pushed
the
hds/large-future-lint
branch
3 times, most recently
from
991b06e
to
92894ad
Compare
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
force-pushed
the
hds/large-future-lint
branch
from
92894ad
to
d5af179
Compare
hds
changed the title
|
The dependent change in Tokio tokio-rs/tokio#6881 has been merged, so this is good to go. |
There was a problem hiding this comment.
This is really cool --- I love seeing new lints I hadn't thought of being added to the console!
I had some very small suggestions about the wording of the lint text, and noticed some stuff was missing from the docs. Other than that, the implementation looks great!
Co-authored-by: Eliza Weisman <eliza@elizas.website>
There was a problem hiding this comment.
Oh, one last thing: since this change depends on new Tokio instrumentation, can we add something stating which version of Tokio these lints will be available in to the list of Tokio versions here: https://github.com/tokio-rs/console/tree/main/console-subscriber#required-tokio-versions
Also, I assume you've tested that the console still works fine with prior tokio versions that are missing the task-size fields? It seems like it ought to, but it's probably worth making sure...
It was causing a stack overflow when run on tokio 1.40.0.
|
@hawkw Good point about the Tokio version! I had tested an earlier version against the current version of Tokio, but it was good that I tested it again, after modifying the large blocking task I had accidentally made it so large that in the Tokio 1.40.0 (and lower) it was causing a stack overflow (just in the example application, but all the same) because that blocking task wasn't being auto-boxed. Fixed that now and confirmed that everything works fine in the current version of Tokio, just that there is no size information and so the lints won't ever trigger. |
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
tokio1.41.0 or later. The version is as yet, unreleased.Both lints have been added to the default list.
PR Notes
Here's a screenshot with the new lints triggered on tasks from the
appexample.