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

maitake: add task IDs #267

Closed
hawkw opened this issue Jul 26, 2022 · 1 comment · Fixed by #297
Closed

maitake: add task IDs #267

hawkw opened this issue Jul 26, 2022 · 1 comment · Fixed by #297
Labels
crate/maitake Related to the `maitake` crate good first issue Good for newcomers kind/enhancement New feature or request needs/design

Comments

@hawkw
Copy link
Owner

hawkw commented Jul 26, 2022

We should consider adding task IDs to maitake to identify a task relative to other currently running tasks. This can be useful for diagnostics (e.g. a JoinError can indicate the ID of the task that was cancelled, etc).

There are two potential approaches we could use for task IDs:

  • Use a global (or per-scheduler) counter of task IDs.
    • pros:
      • This means task IDs would be sequential, and will not collide unless the counter wraps.
      • Task ID uniqueness would be relative to all tasks, not just currently running ones.
    • cons:
      • We would have to actually store these IDs inside the task struct somewhere, making it a word bigger.
      • Accessing a task's ID would require dereferencing a TaskRef pointer.
      • Counters may wrap if you spawn a bunch of tasks. On 64-bit targets this will basically never happen, but it might be more of a concern on 32-bit targets. We may want to ensure that the task ID is always 64 bits, which would mean we would need to handle the fact that some 32-bit targets have 64-bit atomics and others do not. Falling back to a mutex on targets without 64-bit atomics would make accessing the task ID counter significantly more expensive.
  • Use the task's address on the heap as an ID.
    • pros:
      • This is cheap to implement, and doesn't require any counters that may wrap or be limited in size based on architecture.
      • Accessing a task ID from a TaskRef is just casting the pointer to an integer.
      • Address uniqueness is guaranteed by the allocator.
      • Existing maitake logs already include the task's address.
    • cons:
      • There's kind of one big one, which is that ID uniqueness is only relative to currently live tasks. The same address may be reused for a new task once a task completes. If the purpose of task IDs is just to identify tasks in logs, I don't know how big a problem this is, since we'll see in the logs when the task with a given ID is deallocated and when a new task is spawned...
@hawkw hawkw added kind/enhancement New feature or request good first issue Good for newcomers needs/design crate/maitake Related to the `maitake` crate labels Jul 26, 2022
@hawkw
Copy link
Owner Author

hawkw commented Jul 27, 2022

@jamesmunns would love your take on the proposed approaches here.

hawkw added a commit that referenced this issue Aug 11, 2022
This commit adds a `TaskId` type in `maitake::task`, and methods for
`Task`, `TaskRef`, and `JoinHandle` that return the ID of the referenced
task. Task IDs are implemented as an `AtomicUsize` counter that's
incremented for every new task. IDs start at 1, as ID 0 is reserved for
stub tasks.

Closes #267
hawkw added a commit that referenced this issue Aug 11, 2022
This commit adds a `TaskId` type in `maitake::task`, and methods for
`Task`, `TaskRef`, and `JoinHandle` that return the ID of the referenced
task. Task IDs are implemented as an `AtomicUsize` counter that's
incremented for every new task. IDs start at 1, as ID 0 is reserved for
stub tasks.

Closes #267
@hawkw hawkw closed this as completed in 7ccfce3 Aug 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate/maitake Related to the `maitake` crate good first issue Good for newcomers kind/enhancement New feature or request needs/design
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant