7921: runtime: add worker_index() getter#70
Conversation
Add `tokio::runtime::worker_id()` which returns the 0-based index of the current runtime worker thread. The index matches the worker indices used by `RuntimeMetrics` (e.g. `worker_total_busy_duration`), making it possible to correlate per-worker metrics with specific tasks. Returns `None` from blocking threads, the `block_on` caller thread (on the multi-thread runtime), and non-Tokio threads. For the current-thread runtime, always returns `Some(0)`.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the Tokio runtime by providing a new API to identify the specific worker thread executing a task. This feature is particularly useful for advanced debugging, logging, or metrics collection, allowing developers to gain deeper insights into how their asynchronous tasks are distributed and processed across the runtime's thread pool. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
WalkthroughThis pull request introduces a new ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new tokio::runtime::worker_index() function to get the index of the current worker thread. The implementation is well-structured, correctly plumbed through the runtime and scheduler layers, and is exposed under the tokio_unstable flag. The documentation is comprehensive, and the accompanying tests are thorough. No security vulnerabilities were found. I have one minor suggestion to improve code conciseness.
|
|
||
| #[cfg(tokio_unstable)] | ||
| pub(crate) fn worker_index() -> Option<usize> { | ||
| with_scheduler(|ctx| ctx.and_then(|c| c.worker_index())) |
There was a problem hiding this comment.
The closure |c| c.worker_index() can be simplified by passing the method as a function pointer directly to and_then. This is more idiomatic and concise.
| with_scheduler(|ctx| ctx.and_then(|c| c.worker_index())) | |
| with_scheduler(|ctx| ctx.and_then(scheduler::Context::worker_index)) |
There was a problem hiding this comment.
value:good-but-wont-fix; category:bug; feedback: The Gemini AI reviewer is correct! The function pointer may be used instead of the closure. But this is a matter of taste and the Tokio project already uses many closures in such use cases, so this one will stay for consistency
🤖 Augment PR SummarySummary: This PR introduces a new unstable helper for identifying which runtime worker thread is executing the current task. Changes:
Technical Notes: The API is wired through 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tokio/tests/rt_worker_index.rs (1)
71-94: Optional: add ablock_in_placebehavior test.Given the API’s context-sensitive behavior, a dedicated
block_in_placeassertion would further harden regressions around runtime-entry transitions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tokio/tests/rt_worker_index.rs` around lines 71 - 94, Add a test that asserts runtime::worker_index reports a worker thread when code runs inside tokio::task::block_in_place: create a new Runtime (Runtime::new()), use rt.block_on to run an async block that calls tokio::task::block_in_place(|| runtime::worker_index()), await and unwrap the result, then assert_eq!(index, Some(_)) (or assert!(index.is_some())) to validate block_in_place transitions to a worker context; put this alongside the existing worker_index tests and reference runtime::worker_index and tokio::task::block_in_place in the new test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tokio/tests/rt_worker_index.rs`:
- Around line 71-94: Add a test that asserts runtime::worker_index reports a
worker thread when code runs inside tokio::task::block_in_place: create a new
Runtime (Runtime::new()), use rt.block_on to run an async block that calls
tokio::task::block_in_place(|| runtime::worker_index()), await and unwrap the
result, then assert_eq!(index, Some(_)) (or assert!(index.is_some())) to
validate block_in_place transitions to a worker context; put this alongside the
existing worker_index tests and reference runtime::worker_index and
tokio::task::block_in_place in the new test.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
tokio/src/runtime/context.rstokio/src/runtime/mod.rstokio/src/runtime/scheduler/mod.rstokio/src/runtime/scheduler/multi_thread/worker.rstokio/tests/rt_worker_index.rs
PR Review: runtime: add worker_index() getterOverall this is a well-motivated addition. The implementation is clean, the layering through the scheduler context is correct, and the test suite covers the important cases. A few things worth discussing before merging: CorrectnessContext::MultiThread always wraps in Some (scheduler/mod.rs:301) The Documentation gap: worker index is not stable across .await pointsThe docs describe what the function returns, but do not warn that the index can change between suspension points. In the multi-thread scheduler, a task can be stolen and resume on a different worker. Users correlating per-worker metrics with task activity need to know they should capture the index at a specific point, not assume it remains constant for the task lifetime. This is the most likely source of misuse and should be called out explicitly in the doc comment. Test file is over-gated on target_has_atomic = "64"The Consider splitting the file or using per-test cfg attributes so the metrics-specific test carries that gate while the rest run on all supported platforms. Missing test: LocalRuntime
Minor: doctest does not demonstrate the primary use caseThe doc example shows the raw call but not the correlation with RuntimeMetrics that motivates the whole feature. A brief second example (even as a no_run snippet) showing the intended pattern of capturing the index and reading Nit: cfg_unstable! vs inline cfg(tokio_unstable)The internal helpers in context.rs, scheduler/mod.rs, and worker.rs use raw Summary
The stability caveat in the docs is the most important change to address before merging; everything else is incremental improvement. |
value:good-to-have; category:documentation; feedback: The Claude AI reviewer is correct! Await points are the place where the Async runtime may suspend a task and later resume it on a different worked thread. Since this new method is all about worker thread index it is important to mention this in its documentation. |
value:good-to-have; category:bug; feedback: The Claude AI reviewer is correct! Most of the test cases do not need the metrics registry, so they could be executed without this requirement being fulfilled. It would be good to run more tests on more platforms to catch any bugs as early as possible. |
value:good-to-have; category:bug; feedback: The Claude AI reviewer is correct! Adding a test case for LocalRuntime will document it explicitly how this new method should behave on this kind of a runtime. |
2 participants
7921: To review by AI