rt: add optional per-task user data exposed to hooks #7688
Conversation
In tokio-rs#7197 and tokio-rs#7306, improved capabilities of task hooks were discussed and an initial implementation provided. However, that involved quite wide-reaching changes, modifying every spawn site and introducing a global map to provide the full inheritance capabilities originally proposed. This is the first part of a more basic version where we only use the existing hooks and provide the capabilities for consumers to be able to implement more complex relationships if needed, just adding an optional user data ref to the task header. The additional data is 2*usize, and is not enough to result in the struct requiring more than one cache line. A user is now able to use their own global map to build inheritance capabilities if needed, and this would be made simpler by also exposing the current task user data to the on_task_spawn hook, which a followup will look to do.
github-actions
bot
added
R-loom-current-thread
…ast helpers It's nicer syntactically to be able to directly pass whatever type and have tokio deal with how the user data is stored, but it makes on_task_spawn have a horrible wrapper function, so just take that out to keep things simpler and nicer for now.
|
|
||
| /// Custom user defined metadata for this task for use in hooks. | ||
| #[cfg(tokio_unstable)] | ||
| pub(super) user_data: UserData, |
There was a problem hiding this comment.
I believe the task header is supposed to fit in 64 bytes. Is that still the case? I'm thinking we might need to put this in the trailer.
There was a problem hiding this comment.
Yep, it's still the case:
$ cargo clean; RUSTFLAGS="--cfg tokio_unstable" cargo +nightly rustc --lib --all-features -- -Zprint-type-sizes | grep '`runtime::task::core::Header`'
...
print-type-size type: `runtime::task::core::Header`: 56 bytes, alignment: 8 bytes
and reviewing the full output confirming that's with the additional user_data, with its expected 16 bytes:
print-type-size type: `runtime::task::core::Header`: 56 bytes, alignment: 8 bytes
print-type-size field `.state`: 8 bytes
print-type-size field `.queue_next`: 8 bytes
print-type-size field `.vtable`: 8 bytes
print-type-size field `.owner_id`: 8 bytes
print-type-size field `.tracing_id`: 8 bytes
print-type-size field `.user_data`: 16 bytes
|
Yes, it's useful with an overview of the public API being added. |
|
|
||
| /// Spawns a future with user data onto the thread pool | ||
| #[cfg(tokio_unstable)] | ||
| pub(crate) fn spawn_with_user_data<F>( |
There was a problem hiding this comment.
I think we need to make this pub, not pub(crate) unless I've missed something? Same for all the other things people need to access like Userdata.
There was a problem hiding this comment.
I need to add an example of this with the docs, I suspect, but the task::Builder interface is how to use this:
| } | ||
|
|
||
| /// Assigns user data to the task which will be spawned. | ||
| pub fn data(self, data: &'static dyn std::any::Any) -> Self { |
There was a problem hiding this comment.
nit: this method consumes self, while fn name(&self) above does not. I think name() should be changed too.
| #[cfg_attr(docsrs, doc(cfg(all(tokio_unstable, feature = "tracing"))))] | ||
| pub struct Builder<'a> { | ||
| name: Option<&'a str>, | ||
| user_data: UserData, |
There was a problem hiding this comment.
| user_data: UserData, | |
| #[cfg(tokio_unstable)] | |
| user_data: UserData, |
There was a problem hiding this comment.
I thought the entire builder interface is locked behind tokio_unstable?
There was a problem hiding this comment.
Why ?
Because of https://github.com/tokio-rs/tokio/pull/7688/files/e3030a4933e9ca2280ab5d667cd4857654f1ff12#diff-c85538dc30f8755f5f2d5ec2ca9eb8c1fda8a8fb3686e114e67e16030ef98f6bR2 ? It is a condition only for the following use.
| /// used throughout the task spawning system when the `tokio_unstable` feature | ||
| /// is enabled. | ||
| #[cfg(tokio_unstable)] | ||
| pub(crate) type UserData = Option<&'static dyn Any>; |
There was a problem hiding this comment.
Should there be local and multi-thread versions of this type depending on the runtime in use ?
I.e. for multi-threaded runtime should this be pub(crate) type UserData = Option<&'static (dyn Any + Send + Sync)>; ?
5 participants
In #7197 and #7306, improved capabilities of task hooks were discussed and an initial implementation provided. However, that involved quite wide-reaching changes, modifying every spawn site and introducing a global map to provide the full inheritance capabilities originally proposed.
This is the first part of a more basic version where we only use the existing hooks and provide the capabilities for consumers to be able to implement more complex relationships if needed, just adding an optional user data ref to the task header.
The additional data is 2*usize, and is not enough to result in the struct requiring more than one cache line.
A user is now able to use their own global map to build inheritance capabilities if needed, and this would be made simpler by also exposing the current task user data to the on_task_spawn hook, which a followup will look to do.
Motivation
See #7306. The intent is to allow persistence of context for across evaluations of task hooks, which is potentially also accessible by the task itself.
For example you may want to just know how many poll()s a task took, metrics of each poll duration (or between each poll duration), or to set some flag to track what a task was about to do (e.g. read some bytes from a socket) to identify what a task has become blocked on.
Solution
By adding a fat pointer for task metadata, consumers are able to attack a reference to data of an arbitrary type for a task, which we then expose in hooks. This metadata can be configured at task spawn time, also allowing consumers to maintain some shared reference between the hooks and the task's future itself to maintain knowledge of what is going on.