tokio: Add initial io driver metrics #4507

LucioFranco · 2022-02-16T17:33:21Z

Work in progress

This adds the initial IO driver metrics.

Todo

tests
docs

LucioFranco · 2022-02-16T17:34:01Z

tokio/src/io/driver/mod.rs

@@ -298,6 +304,17 @@ impl Handle {
    pub(super) fn inner(&self) -> Option<Arc<Inner>> {
        self.inner.upgrade()
    }
+
+    pub(crate) fn with_io_driver_metrics<F, R>(&self, f: F) -> Option<R>


Had to do this to avoid having to return an Arc and doing a ref count bump

Why not return the Arc? An upgrade() already does a ref count bump. Also, we probably should separately get rid of the weak ref inc.

At least leave a TODO, maybe open an issue to track getting rid of Weak?

LucioFranco · 2022-02-16T17:34:34Z

tokio/src/io/driver/mod.rs

@@ -197,6 +201,8 @@ impl Driver {
            return;
        }

+        self.inner.metrics.incr_ready_count();


Do we want to do this at the top of this method to capture invalid tokens as well? or do we want this to represent only valid wakeups?

tokio/src/runtime/metrics/runtime.rs

.gitignore

tokio/src/io/driver/mod.rs

tokio/src/runtime/metrics/io.rs

carllerche · 2022-02-22T19:07:23Z

tokio/src/io/driver/mod.rs

+        // &IoDriverMetrics instead of using a closure.
+        //
+        // Related issue: https://github.com/tokio-rs/tokio/issues/4509
+        pub(crate) fn with_io_driver_metrics<F, R>(&self, f: F) -> Option<R>


Fwiw, calling self.inner() does an arc bump anyway. I probably wouldn't bother w/ the closure, but it isn't a bit deal.

Yeah, plan to refactor with removal of weak from io driver handle.

carllerche · 2022-02-22T19:07:39Z

tokio/src/lib.rs

@@ -483,7 +483,7 @@ pub(crate) use self::doc::winapi;

 #[cfg(all(not(docsrs), windows, feature = "net"))]
 #[allow(unused)]
-pub(crate) use ::winapi;
+pub(crate) use winapi;


what is this change?

I think rustfmt did it? I didn't manually change it but I don't think it affects anything since this got updated in 2018 edition iirc

carllerche · 2022-02-22T19:11:51Z

tokio/src/runtime/metrics/io.rs

+
+#[derive(Default)]
+pub(crate) struct IoDriverMetrics {
+    pub(super) fd_count: AtomicU64,


Is it worth tracking registered & deregistered separately? Right now, if an FD is registered then immediately deregistered, this will show up as the counter not moving. What is the value in tracking outstanding FDs? If registered & deregistered ops are tracked separately, churn will be detectable.

Separately, do we want to track fd_count or punt that to tracing? I am not sure. Thoughts @hawkw

Wouldn't this catch things like leaked fds etc? I think there is value in tracking this internally to the driver in addition to what ever tracing can bring?

tracking register & deregister separately would be a super set. Total fds == registers - deregisters.

carllerche

The structure of the PR looks good. I left some comments inline.

tokio/src/runtime/metrics/runtime.rs

carllerche

lgtm thanks.

# 1.18.0 (April 27, 2022) This release adds a number of new APIs in `tokio::net`, `tokio::signal`, and `tokio::sync`. In addition, it adds new unstable APIs to `tokio::task` (`Id`s for uniquely identifying a task, and `AbortHandle` for remotely cancelling a task), as well as a number of bugfixes. ### Fixed - blocking: add missing `#[track_caller]` for `spawn_blocking` ([#4616]) - macros: fix `select` macro to process 64 branches ([#4519]) - net: fix `try_io` methods not calling Mio's `try_io` internally ([#4582]) - runtime: recover when OS fails to spawn a new thread ([#4485]) ### Added - macros: support setting a custom crate name for `#[tokio::main]` and `#[tokio::test]` ([#4613]) - net: add `UdpSocket::peer_addr` ([#4611]) - net: add `try_read_buf` method for named pipes ([#4626]) - signal: add `SignalKind` `Hash`/`Eq` impls and `c_int` conversion ([#4540]) - signal: add support for signals up to `SIGRTMAX` ([#4555]) - sync: add `watch::Sender::send_modify` method ([#4310]) - sync: add `broadcast::Receiver::len` method ([#4542]) - sync: add `watch::Receiver::same_channel` method ([#4581]) - sync: implement `Clone` for `RecvError` types ([#4560]) ### Changed - update `nix` to 0.24, limit features ([#4631]) - update `mio` to 0.8.1 ([#4582]) - macros: rename `tokio::select!`'s internal `util` module ([#4543]) - runtime: use `Vec::with_capacity` when building runtime ([#4553]) ### Documented - improve docs for `tokio_unstable` ([#4524]) - runtime: include more documentation for thread_pool/worker ([#4511]) - runtime: update `Handle::current`'s docs to mention `EnterGuard` ([#4567]) - time: clarify platform specific timer resolution ([#4474]) - signal: document that `Signal::recv` is cancel-safe ([#4634]) - sync: `UnboundedReceiver` close docs ([#4548]) ### Unstable The following changes only apply when building with `--cfg tokio_unstable`: - task: add `task::Id` type ([#4630]) - task: add `AbortHandle` type for cancelling tasks in a `JoinSet` ([#4530], [#4640]) - task: fix missing `doc(cfg(...))` attributes for `JoinSet` ([#4531]) - task: fix broken link in `AbortHandle` RustDoc ([#4545]) - metrics: add initial IO driver metrics ([#4507]) [#4616]: #4616 [#4519]: #4519 [#4582]: #4582 [#4485]: #4485 [#4613]: #4613 [#4611]: #4611 [#4626]: #4626 [#4540]: #4540 [#4555]: #4555 [#4310]: #4310 [#4542]: #4542 [#4581]: #4581 [#4560]: #4560 [#4631]: #4631 [#4582]: #4582 [#4543]: #4543 [#4553]: #4553 [#4524]: #4524 [#4511]: #4511 [#4567]: #4567 [#4474]: #4474 [#4634]: #4634 [#4548]: #4548 [#4630]: #4630 [#4530]: #4530 [#4640]: #4640 [#4531]: #4531 [#4545]: #4545 [#4507]: #4507

# 1.18.0 (April 27, 2022) This release adds a number of new APIs in `tokio::net`, `tokio::signal`, and `tokio::sync`. In addition, it adds new unstable APIs to `tokio::task` (`Id`s for uniquely identifying a task, and `AbortHandle` for remotely cancelling a task), as well as a number of bugfixes. ### Fixed - blocking: add missing `#[track_caller]` for `spawn_blocking` ([#4616]) - macros: fix `select` macro to process 64 branches ([#4519]) - net: fix `try_io` methods not calling Mio's `try_io` internally ([#4582]) - runtime: recover when OS fails to spawn a new thread ([#4485]) ### Added - macros: support setting a custom crate name for `#[tokio::main]` and `#[tokio::test]` ([#4613]) - net: add `UdpSocket::peer_addr` ([#4611]) - net: add `try_read_buf` method for named pipes ([#4626]) - signal: add `SignalKind` `Hash`/`Eq` impls and `c_int` conversion ([#4540]) - signal: add support for signals up to `SIGRTMAX` ([#4555]) - sync: add `watch::Sender::send_modify` method ([#4310]) - sync: add `broadcast::Receiver::len` method ([#4542]) - sync: add `watch::Receiver::same_channel` method ([#4581]) - sync: implement `Clone` for `RecvError` types ([#4560]) ### Changed - update `nix` to 0.24, limit features ([#4631]) - update `mio` to 0.8.1 ([#4582]) - macros: rename `tokio::select!`'s internal `util` module ([#4543]) - runtime: use `Vec::with_capacity` when building runtime ([#4553]) ### Documented - improve docs for `tokio_unstable` ([#4524]) - runtime: include more documentation for thread_pool/worker ([#4511]) - runtime: update `Handle::current`'s docs to mention `EnterGuard` ([#4567]) - time: clarify platform specific timer resolution ([#4474]) - signal: document that `Signal::recv` is cancel-safe ([#4634]) - sync: `UnboundedReceiver` close docs ([#4548]) ### Unstable The following changes only apply when building with `--cfg tokio_unstable`: - task: add `task::Id` type ([#4630]) - task: add `AbortHandle` type for cancelling tasks in a `JoinSet` ([#4530], [#4640]) - task: fix missing `doc(cfg(...))` attributes for `JoinSet` ([#4531]) - task: fix broken link in `AbortHandle` RustDoc ([#4545]) - metrics: add initial IO driver metrics ([#4507]) [#4616]: #4616 [#4519]: #4519 [#4582]: #4582 [#4485]: #4485 [#4613]: #4613 [#4611]: #4611 [#4626]: #4626 [#4540]: #4540 [#4555]: #4555 [#4310]: #4310 [#4542]: #4542 [#4581]: #4581 [#4560]: #4560 [#4631]: #4631 [#4582]: #4582 [#4543]: #4543 [#4553]: #4553 [#4524]: #4524 [#4511]: #4511 [#4567]: #4567 [#4474]: #4474 [#4634]: #4634 [#4548]: #4548 [#4630]: #4630 [#4530]: #4530 [#4640]: #4640 [#4531]: #4531 [#4545]: #4545 [#4507]: #4507 Signed-off-by: Eliza Weisman <eliza@buoyant.io>

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [tokio](https://tokio.rs) ([source](https://github.com/tokio-rs/tokio)) | dependencies | minor | `1.17.0` -> `1.18.0` | | [tokio](https://tokio.rs) ([source](https://github.com/tokio-rs/tokio)) | dev-dependencies | minor | `1.17.0` -> `1.18.0` | --- ### Release Notes <details> <summary>tokio-rs/tokio</summary> ### [`v1.18.0`](https://github.com/tokio-rs/tokio/releases/tokio-1.18.0) [Compare Source](tokio-rs/tokio@tokio-1.17.0...tokio-1.18.0) ##### 1.18.0 (April 27, 2022) This release adds a number of new APIs in `tokio::net`, `tokio::signal`, and `tokio::sync`. In addition, it adds new unstable APIs to `tokio::task` (`Id`s for uniquely identifying a task, and `AbortHandle` for remotely cancelling a task), as well as a number of bugfixes. ##### Fixed - blocking: add missing `#[track_caller]` for `spawn_blocking` ([#4616](tokio-rs/tokio#4616)) - macros: fix `select` macro to process 64 branches ([#4519](tokio-rs/tokio#4519)) - net: fix `try_io` methods not calling Mio's `try_io` internally ([#4582](tokio-rs/tokio#4582)) - runtime: recover when OS fails to spawn a new thread ([#4485](tokio-rs/tokio#4485)) ##### Added - macros: support setting a custom crate name for `#[tokio::main]` and `#[tokio::test]` ([#4613](tokio-rs/tokio#4613)) - net: add `UdpSocket::peer_addr` ([#4611](tokio-rs/tokio#4611)) - net: add `try_read_buf` method for named pipes ([#4626](tokio-rs/tokio#4626)) - signal: add `SignalKind` `Hash`/`Eq` impls and `c_int` conversion ([#4540](tokio-rs/tokio#4540)) - signal: add support for signals up to `SIGRTMAX` ([#4555](tokio-rs/tokio#4555)) - sync: add `watch::Sender::send_modify` method ([#4310](tokio-rs/tokio#4310)) - sync: add `broadcast::Receiver::len` method ([#4542](tokio-rs/tokio#4542)) - sync: add `watch::Receiver::same_channel` method ([#4581](tokio-rs/tokio#4581)) - sync: implement `Clone` for `RecvError` types ([#4560](tokio-rs/tokio#4560)) ##### Changed - update `mio` to 0.8.1 ([#4582](tokio-rs/tokio#4582)) - macros: rename `tokio::select!`'s internal `util` module ([#4543](tokio-rs/tokio#4543)) - runtime: use `Vec::with_capacity` when building runtime ([#4553](tokio-rs/tokio#4553)) ##### Documented - improve docs for `tokio_unstable` ([#4524](tokio-rs/tokio#4524)) - runtime: include more documentation for thread_pool/worker ([#4511](tokio-rs/tokio#4511)) - runtime: update `Handle::current`'s docs to mention `EnterGuard` ([#4567](tokio-rs/tokio#4567)) - time: clarify platform specific timer resolution ([#4474](tokio-rs/tokio#4474)) - signal: document that `Signal::recv` is cancel-safe ([#4634](tokio-rs/tokio#4634)) - sync: `UnboundedReceiver` close docs ([#4548](tokio-rs/tokio#4548)) ##### Unstable The following changes only apply when building with `--cfg tokio_unstable`: - task: add `task::Id` type ([#4630](tokio-rs/tokio#4630)) - task: add `AbortHandle` type for cancelling tasks in a `JoinSet` ([#4530](tokio-rs/tokio#4530)], \[[#4640](tokio-rs/tokio#4640)) - task: fix missing `doc(cfg(...))` attributes for `JoinSet` ([#4531](tokio-rs/tokio#4531)) - task: fix broken link in `AbortHandle` RustDoc ([#4545](tokio-rs/tokio#4545)) - metrics: add initial IO driver metrics ([#4507](tokio-rs/tokio#4507)) </details> --- ### Configuration 📅 **Schedule**: At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1327 Reviewed-by: crapStone <crapstone@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>

LucioFranco requested review from carllerche and Darksonn February 16, 2022 17:33

github-actions bot added the R-loom Run loom tests on this PR label Feb 16, 2022

LucioFranco commented Feb 16, 2022

View reviewed changes

tokio/src/runtime/metrics/runtime.rs Outdated Show resolved Hide resolved

Darksonn added A-tokio Area: The main tokio crate M-io Module: tokio/io M-metrics Module: tokio/runtime/metrics labels Feb 16, 2022

carllerche reviewed Feb 16, 2022

View reviewed changes

.gitignore Outdated Show resolved Hide resolved

carllerche reviewed Feb 16, 2022

View reviewed changes

tokio/src/io/driver/mod.rs Outdated Show resolved Hide resolved

carllerche reviewed Feb 16, 2022

View reviewed changes

tokio/src/runtime/metrics/io.rs Outdated Show resolved Hide resolved

tokio: Add initial io driver metrics

ffa5add

LucioFranco force-pushed the lucio/io-driver-metrics branch from b3d5df7 to ffa5add Compare February 17, 2022 20:23

LucioFranco marked this pull request as ready for review February 17, 2022 20:42

LucioFranco requested a review from carllerche February 17, 2022 20:42

Limit tests to run under macos/linux

d17dbab

carllerche reviewed Feb 22, 2022

View reviewed changes

LucioFranco added 2 commits February 22, 2022 19:26

Separate fd counters

8060072

fix dec fd count

525a991

LucioFranco requested a review from carllerche February 22, 2022 20:28

carllerche reviewed Feb 22, 2022

View reviewed changes

tokio/src/runtime/metrics/runtime.rs Outdated Show resolved Hide resolved

separate fd counters

b7a54f1

LucioFranco requested a review from carllerche February 24, 2022 14:33

carllerche approved these changes Feb 24, 2022

View reviewed changes

LucioFranco merged commit 769fb15 into master Feb 24, 2022

LucioFranco deleted the lucio/io-driver-metrics branch February 24, 2022 18:39

hawkw mentioned this pull request Apr 27, 2022

tokio: prepare to release v1.18.0 #4641

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tokio: Add initial io driver metrics #4507

tokio: Add initial io driver metrics #4507

LucioFranco commented Feb 16, 2022 •

edited

LucioFranco Feb 16, 2022

carllerche Feb 16, 2022

carllerche Feb 16, 2022

LucioFranco Feb 16, 2022

LucioFranco Feb 16, 2022

carllerche Feb 22, 2022

LucioFranco Feb 22, 2022

carllerche Feb 22, 2022

LucioFranco Feb 22, 2022

carllerche Feb 22, 2022

LucioFranco Feb 22, 2022

carllerche Feb 22, 2022

carllerche left a comment

carllerche left a comment

tokio: Add initial io driver metrics #4507

tokio: Add initial io driver metrics #4507

Conversation

LucioFranco commented Feb 16, 2022 • edited

Work in progress

Todo

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carllerche left a comment

Choose a reason for hiding this comment

carllerche left a comment

Choose a reason for hiding this comment

LucioFranco commented Feb 16, 2022 •

edited