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 IDs #4630

Merged
merged 14 commits into from Apr 25, 2022
Merged

task: add task IDs #4630

merged 14 commits into from Apr 25, 2022

Commits on Apr 22, 2022

  1. task: add task IDs

    ## Motivation
    
    PR #4538 adds a prototype implementation of a `JoinMap` API in
    `tokio::task`. In [this comment][1] on that PR, @carllerche pointed out
    that a much simpler `JoinMap` type could be implemented outside of
    `tokio` (either in `tokio-util` or in user code) if we just modified
    `JoinSet` to return a task ID type when spawning new tasks, and when
    tasks complete. This seems like a better approach for the following
    reasons:
    
    * A `JoinMap`-like type need not become a permanent part of `tokio`'s
      stable API
    * Task IDs seem like something that could be generally useful outside of
      a `JoinMap` implementation
    
    ## Solution
    
    This branch adds a `tokio::task::Id` type that uniquely identifies a
    task relative to all currently spawned tasks. The ID is internally
    represented as a `NonZeroUsize` that's based on the address of the
    task's header. I thought that it was better to use addresses to generate
    IDs than assigning sequential IDs to tasks, because a sequential ID
    would mean adding an additional usize field to the task data
    somewhere, making it a word bigger. I've added methods to `JoinHandle`
    and `AbortHandle` for returning a task's ID.
    
    In addition, I modified `JoinSet` to add a `join_with_id` method that
    behaves identically to `join_one` but also returns an ID. This can be
    used to implement a `JoinMap` type.
    
    Note that because `join_with_id` must return a task ID regardless of
    whether the task completes successfully or returns a `JoinError` (in
    order to ensure that dead tasks are always cleaned up from a map), it
    inverts the ordering of the `Option` and `Result` returned by `join_one`
    --- which we've bikeshedded about a bit [here][2]. This makes the
    method's return type inconsistent with the existing `join_one` method,
    which feels not great. As I see it, there are three possible solutions
    to this:
    
    * change the existing `join_one` method to also swap the `Option` and
      `Result` nesting for consistency.
    * change `join_with_id` to return `Result<Option<(Id, T)>, (Id,
      JoinError)>>`, which feels gross...
    * add a task ID to `JoinError` as well.
    
    [1]: #4538 (comment)
    [2]: #4335 (comment)
    hawkw committed Apr 22, 2022
    Copy the full SHA
    1f43cb3 View commit details
    Browse the repository at this point in the history
  2. add task IDs to join errors

    Signed-off-by: Eliza Weisman <eliza@buoyant.io>
    hawkw committed Apr 22, 2022
    Copy the full SHA
    e77bacc View commit details
    Browse the repository at this point in the history
  3. assign IDs sequentially instead of by addr

    Signed-off-by: Eliza Weisman <eliza@buoyant.io>
    hawkw committed Apr 22, 2022
    Copy the full SHA
    ece2698 View commit details
    Browse the repository at this point in the history
  4. add task IDs to tracing spans

    This commit adds the runtime generated task IDs to the task `tracing`
    spans, when tracing is enabled. Unfortunately, this means the IDs have
    to be generated much higher up the callstack so they can be added to the
    tracing spans, and then passed down to the actual task machinery. But, I
    think this is worth it to be able to record that data.
    
    Signed-off-by: Eliza Weisman <eliza@buoyant.io>
    hawkw committed Apr 22, 2022
    Copy the full SHA
    47a26c5 View commit details
    Browse the repository at this point in the history
  5. make IDs always be 64-bit

    Signed-off-by: Eliza Weisman <eliza@buoyant.io>
    hawkw committed Apr 22, 2022
    Copy the full SHA
    09254f9 View commit details
    Browse the repository at this point in the history
  6. naming review feedback

    hawkw committed Apr 22, 2022
    Copy the full SHA
    8ead959 View commit details
    Browse the repository at this point in the history
  7. fix tests

    hawkw committed Apr 22, 2022
    Copy the full SHA
    1fd964e View commit details
    Browse the repository at this point in the history
  8. fix miri tests also

    Signed-off-by: Eliza Weisman <eliza@buoyant.io>
    hawkw committed Apr 22, 2022
    Copy the full SHA
    dfb38c4 View commit details
    Browse the repository at this point in the history
  9. block_on also wants an ID

    Signed-off-by: Eliza Weisman <eliza@buoyant.io>
    hawkw committed Apr 22, 2022
    Copy the full SHA
    4412f1d View commit details
    Browse the repository at this point in the history
  10. fixup

    Signed-off-by: Eliza Weisman <eliza@buoyant.io>
    hawkw committed Apr 22, 2022
    Copy the full SHA
    558a744 View commit details
    Browse the repository at this point in the history
  11. rustfmt

    Signed-off-by: Eliza Weisman <eliza@buoyant.io>
    hawkw committed Apr 22, 2022
    Copy the full SHA
    9463376 View commit details
    Browse the repository at this point in the history
  12. ugh somehow missed that

    Signed-off-by: Eliza Weisman <eliza@buoyant.io>
    hawkw committed Apr 22, 2022
    Copy the full SHA
    7567ecc View commit details
    Browse the repository at this point in the history
  13. bleh fix 32-bit code

    Signed-off-by: Eliza Weisman <eliza@buoyant.io>
    hawkw committed Apr 22, 2022
    Copy the full SHA
    ca15b2f View commit details
    Browse the repository at this point in the history

Commits on Apr 25, 2022

  1. remove duplicate attrs

    Signed-off-by: Eliza Weisman <eliza@buoyant.io>
    hawkw committed Apr 25, 2022
    Copy the full SHA
    9a57cd4 View commit details
    Browse the repository at this point in the history