From 8da6db78c388ea5d8ef3ec28dc7823463587ef72 Mon Sep 17 00:00:00 2001 From: Jeff Lai Date: Wed, 21 Jun 2023 20:05:31 -0400 Subject: [PATCH 01/15] feat(console): add warning for tasks that never yield --- console-subscriber/examples/busy_loop.rs | 31 ++++++++++++ tokio-console/src/main.rs | 1 + tokio-console/src/state/tasks.rs | 4 ++ tokio-console/src/warnings.rs | 61 +++++++++++++++++++++++- 4 files changed, 95 insertions(+), 2 deletions(-) create mode 100644 console-subscriber/examples/busy_loop.rs diff --git a/console-subscriber/examples/busy_loop.rs b/console-subscriber/examples/busy_loop.rs new file mode 100644 index 000000000..a72b037f5 --- /dev/null +++ b/console-subscriber/examples/busy_loop.rs @@ -0,0 +1,31 @@ +use std::sync::atomic::Ordering; +use std::sync::{atomic::AtomicBool, Arc}; +use std::time::Duration; +use tokio::task; +use tokio::time::sleep; + +#[tokio::main] +async fn main() -> Result<(), Box> { + console_subscriber::init(); + let stop = Arc::new(AtomicBool::new(false)); + + let t = task::Builder::default() + .name("busy-loop") + .spawn({ + let stop = Arc::clone(&stop); + async move { + loop { + if stop.load(Ordering::Acquire) { + break; + } + } + } + }) + .unwrap(); + + sleep(Duration::from_secs(300)).await; + stop.store(true, Ordering::Release); + t.await?; + + Ok(()) +} diff --git a/tokio-console/src/main.rs b/tokio-console/src/main.rs index 4fc6630bb..8545e30b9 100644 --- a/tokio-console/src/main.rs +++ b/tokio-console/src/main.rs @@ -65,6 +65,7 @@ async fn main() -> color_eyre::Result<()> { .with_task_linters(vec![ warnings::Linter::new(warnings::SelfWakePercent::default()), warnings::Linter::new(warnings::LostWaker), + warnings::Linter::new(warnings::NeverYielded::default()), ]) .with_retain_for(retain_for); let mut input = input::EventStream::new(); diff --git a/tokio-console/src/state/tasks.rs b/tokio-console/src/state/tasks.rs index bf9c8a2c0..7507b57b7 100644 --- a/tokio-console/src/state/tasks.rs +++ b/tokio-console/src/state/tasks.rs @@ -368,6 +368,10 @@ impl Task { .unwrap_or_default() } + pub(crate) fn yielded(&self) -> bool { + self.total_polls() > 1 + } + /// Returns the total number of times the task has been polled. pub(crate) fn total_polls(&self) -> u64 { self.stats.polls diff --git a/tokio-console/src/warnings.rs b/tokio-console/src/warnings.rs index 818b140bc..013e2c006 100644 --- a/tokio-console/src/warnings.rs +++ b/tokio-console/src/warnings.rs @@ -1,5 +1,9 @@ -use crate::state::tasks::Task; -use std::{fmt::Debug, rc::Rc}; +use crate::state::tasks::{Task, TaskState}; +use std::{ + fmt::Debug, + rc::Rc, + time::{Duration, SystemTime}, +}; /// A warning for a particular type of monitored entity (e.g. task or resource). /// @@ -150,3 +154,56 @@ impl Warn for LostWaker { "This task has lost its waker, and will never be woken again.".into() } } + +/// Warning for if a task has never yielded +#[derive(Clone, Debug)] +pub(crate) struct NeverYielded { + min_duration: Duration, + description: String, +} + +impl NeverYielded { + pub(crate) const DEFAULT_DURATION: Duration = Duration::from_millis(10); + pub(crate) fn new(min_duration: Duration) -> Self { + Self { + min_duration, + description: format!( + "tasks have never yielded (threshold {}ms)", + min_duration.as_millis() + ), + } + } +} + +impl Default for NeverYielded { + fn default() -> Self { + Self::new(Self::DEFAULT_DURATION) + } +} + +impl Warn for NeverYielded { + fn summary(&self) -> &str { + self.description.as_str() + } + + fn check(&self, task: &Task) -> bool { + // Don't fire warning for tasks that are waiting to run + if task.state() != TaskState::Running { + return false; + } + + if task.yielded() { + return false; + } + + // Avoid short-lived task false positives + task.busy(SystemTime::now()) >= self.min_duration + } + + fn format(&self, task: &Task) -> String { + format!( + "This task has never yielded ({:?})", + task.busy(SystemTime::now()), + ) + } +} From 21996cbba4c7f76a8d274feefe4054617e414bf8 Mon Sep 17 00:00:00 2001 From: jefftt Date: Wed, 12 Jul 2023 09:33:14 -0400 Subject: [PATCH 02/15] Update tokio-console/src/warnings.rs set DEFAULT_DURATION to 1 second Co-authored-by: Hayden Stainsby --- tokio-console/src/warnings.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tokio-console/src/warnings.rs b/tokio-console/src/warnings.rs index 013e2c006..96e990882 100644 --- a/tokio-console/src/warnings.rs +++ b/tokio-console/src/warnings.rs @@ -163,7 +163,7 @@ pub(crate) struct NeverYielded { } impl NeverYielded { - pub(crate) const DEFAULT_DURATION: Duration = Duration::from_millis(10); + pub(crate) const DEFAULT_DURATION: Duration = Duration::from_secs(1); pub(crate) fn new(min_duration: Duration) -> Self { Self { min_duration, From 03d4160d8b410995c79dac182b2d23f35844c785 Mon Sep 17 00:00:00 2001 From: Jeff Lai Date: Wed, 12 Jul 2023 09:35:45 -0400 Subject: [PATCH 03/15] Inline Task::yielded() --- tokio-console/src/state/tasks.rs | 4 ---- tokio-console/src/warnings.rs | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/tokio-console/src/state/tasks.rs b/tokio-console/src/state/tasks.rs index 7507b57b7..bf9c8a2c0 100644 --- a/tokio-console/src/state/tasks.rs +++ b/tokio-console/src/state/tasks.rs @@ -368,10 +368,6 @@ impl Task { .unwrap_or_default() } - pub(crate) fn yielded(&self) -> bool { - self.total_polls() > 1 - } - /// Returns the total number of times the task has been polled. pub(crate) fn total_polls(&self) -> u64 { self.stats.polls diff --git a/tokio-console/src/warnings.rs b/tokio-console/src/warnings.rs index 96e990882..fd049cd67 100644 --- a/tokio-console/src/warnings.rs +++ b/tokio-console/src/warnings.rs @@ -192,7 +192,7 @@ impl Warn for NeverYielded { return false; } - if task.yielded() { + if task.total_polls() > 1 { return false; } From e7cf5b56043f9ac87c853b5170946d4d541f93cb Mon Sep 17 00:00:00 2001 From: Jeff Lai Date: Wed, 12 Jul 2023 09:43:39 -0400 Subject: [PATCH 04/15] Remove examples/busy_loop.rs, update examples/app.rs --- console-subscriber/examples/app.rs | 21 ++++++++++++++++ console-subscriber/examples/busy_loop.rs | 31 ------------------------ 2 files changed, 21 insertions(+), 31 deletions(-) delete mode 100644 console-subscriber/examples/busy_loop.rs diff --git a/console-subscriber/examples/app.rs b/console-subscriber/examples/app.rs index f1247ed62..1bdf02227 100644 --- a/console-subscriber/examples/app.rs +++ b/console-subscriber/examples/app.rs @@ -11,6 +11,7 @@ OPTIONS: blocks Includes a (misbehaving) blocking task burn Includes a (misbehaving) task that spins CPU with self-wakes coma Includes a (misbehaving) task that forgets to register a waker + noyield Includes a (misbehaving) task that spawns tasks that never yield "#; #[tokio::main] @@ -38,6 +39,12 @@ async fn main() -> Result<(), Box> { .spawn(burn(1, 10)) .unwrap(); } + "noyield" => { + tokio::task::Builder::new() + .name("noyield") + .spawn(no_yield(20)) + .unwrap(); + } "help" | "-h" => { eprintln!("{}", HELP); return Ok(()); @@ -114,3 +121,17 @@ async fn burn(min: u64, max: u64) { } } } + +#[tracing::instrument] +async fn no_yield(seconds: u64) { + loop { + let handle = tokio::task::Builder::new() + .name("greedy") + .spawn(async move { + std::thread::sleep(Duration::from_secs(seconds)); + }) + .expect("Couldn't spawn greedy task"); + + _ = handle.await; + } +} diff --git a/console-subscriber/examples/busy_loop.rs b/console-subscriber/examples/busy_loop.rs deleted file mode 100644 index a72b037f5..000000000 --- a/console-subscriber/examples/busy_loop.rs +++ /dev/null @@ -1,31 +0,0 @@ -use std::sync::atomic::Ordering; -use std::sync::{atomic::AtomicBool, Arc}; -use std::time::Duration; -use tokio::task; -use tokio::time::sleep; - -#[tokio::main] -async fn main() -> Result<(), Box> { - console_subscriber::init(); - let stop = Arc::new(AtomicBool::new(false)); - - let t = task::Builder::default() - .name("busy-loop") - .spawn({ - let stop = Arc::clone(&stop); - async move { - loop { - if stop.load(Ordering::Acquire) { - break; - } - } - } - }) - .unwrap(); - - sleep(Duration::from_secs(300)).await; - stop.store(true, Ordering::Release); - t.await?; - - Ok(()) -} From e8c9c5224aa78de1a50b2e017ad0c71ba6e18ceb Mon Sep 17 00:00:00 2001 From: Jeff Lai Date: Sat, 12 Aug 2023 10:18:09 -0400 Subject: [PATCH 05/15] Lint every task on each update --- tokio-console/src/state/tasks.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tokio-console/src/state/tasks.rs b/tokio-console/src/state/tasks.rs index bf9c8a2c0..a4aa98a7c 100644 --- a/tokio-console/src/state/tasks.rs +++ b/tokio-console/src/state/tasks.rs @@ -224,7 +224,10 @@ impl TasksState { for (stats, mut task) in self.tasks.updated(stats_update) { tracing::trace!(?task, ?stats, "processing stats update for"); task.stats = stats.into(); - task.lint(linters); + } + + for (_, task) in self.tasks.iter() { + task.borrow_mut().lint(linters); } self.dropped_events += update.dropped_events; From 7fed234be3b269960f1675de2bdb47bb880173dc Mon Sep 17 00:00:00 2001 From: Jeff Lai Date: Sat, 12 Aug 2023 10:21:58 -0400 Subject: [PATCH 06/15] Fix doc formatting --- console-subscriber/examples/app.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/console-subscriber/examples/app.rs b/console-subscriber/examples/app.rs index 1bdf02227..7a1b50c11 100644 --- a/console-subscriber/examples/app.rs +++ b/console-subscriber/examples/app.rs @@ -11,7 +11,7 @@ OPTIONS: blocks Includes a (misbehaving) blocking task burn Includes a (misbehaving) task that spins CPU with self-wakes coma Includes a (misbehaving) task that forgets to register a waker - noyield Includes a (misbehaving) task that spawns tasks that never yield + noyield Includes a (misbehaving) task that spawns tasks that never yield "#; #[tokio::main] From aaf0415574fa94666e0331bde32ed4e1e589b3d6 Mon Sep 17 00:00:00 2001 From: Jeff Lai Date: Tue, 15 Aug 2023 08:42:45 -0400 Subject: [PATCH 07/15] Remove dupe lint on insert --- tokio-console/src/state/tasks.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tokio-console/src/state/tasks.rs b/tokio-console/src/state/tasks.rs index a4aa98a7c..98480806c 100644 --- a/tokio-console/src/state/tasks.rs +++ b/tokio-console/src/state/tasks.rs @@ -204,7 +204,7 @@ impl TasksState { (None, None) => "".to_owned(), }); - let mut task = Task { + let task = Task { name, id, task_id, @@ -217,7 +217,6 @@ impl TasksState { warnings: Vec::new(), location, }; - task.lint(linters); Some((id, task)) }); From 847089413e664689126f1cea0112505080bd08e8 Mon Sep 17 00:00:00 2001 From: Jeff Lai Date: Sat, 2 Sep 2023 09:40:00 -0400 Subject: [PATCH 08/15] Conditionally updates non-updated tasks --- Cargo.lock | 1 + tokio-console/src/state/tasks.rs | 47 +++++++++++++++----- tokio-console/src/warnings.rs | 73 ++++++++++++++++++++++---------- 3 files changed, 89 insertions(+), 32 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 27d4c3088..9e73cde34 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -257,6 +257,7 @@ dependencies = [ name = "console-api" version = "0.5.0" dependencies = [ + "futures-core", "prost", "prost-build", "prost-types", diff --git a/tokio-console/src/state/tasks.rs b/tokio-console/src/state/tasks.rs index 98480806c..ef939c426 100644 --- a/tokio-console/src/state/tasks.rs +++ b/tokio-console/src/state/tasks.rs @@ -9,13 +9,13 @@ use crate::{ }, util::Percentage, view, - warnings::Linter, + warnings::{Lint, Linter}, }; use console_api as proto; use ratatui::{style::Color, text::Span}; use std::{ cell::RefCell, - collections::HashMap, + collections::{HashMap, HashSet}, convert::{TryFrom, TryInto}, rc::{Rc, Weak}, time::{Duration, SystemTime}, @@ -24,6 +24,7 @@ use std::{ #[derive(Default, Debug)] pub(crate) struct TasksState { tasks: Store, + pending_lint: HashSet>, pub(crate) linters: Vec>, dropped_events: u64, } @@ -204,7 +205,7 @@ impl TasksState { (None, None) => "".to_owned(), }); - let task = Task { + let mut task = Task { name, id, task_id, @@ -217,17 +218,37 @@ impl TasksState { warnings: Vec::new(), location, }; + let recheck = task.lint(linters); + if recheck { + self.pending_lint.insert(task.id); + } Some((id, task)) }); + let mut checked = HashSet::new(); for (stats, mut task) in self.tasks.updated(stats_update) { tracing::trace!(?task, ?stats, "processing stats update for"); task.stats = stats.into(); + let recheck = task.lint(linters); + if !recheck { + self.pending_lint.remove(&task.id); + } + checked.insert(task.id); } - for (_, task) in self.tasks.iter() { - task.borrow_mut().lint(linters); - } + self.pending_lint.retain(|id| { + if checked.contains(id) { + true + } else { + let recheck = self + .tasks + .get(*id) + .expect("task pending lint is not in task store") + .borrow_mut() + .lint(linters); + recheck + } + }); self.dropped_events += update.dropped_events; } @@ -432,15 +453,21 @@ impl Task { &self.warnings[..] } - fn lint(&mut self, linters: &[Linter]) { + fn lint(&mut self, linters: &[Linter]) -> bool { self.warnings.clear(); + let mut recheck = false; for lint in linters { tracing::debug!(?lint, task = ?self, "checking..."); - if let Some(warning) = lint.check(self) { - tracing::info!(?warning, task = ?self, "found a warning!"); - self.warnings.push(warning) + match lint.check(self) { + Lint::Warning(warning) => { + tracing::info!(?warning, task = ?self, "found a warning!"); + self.warnings.push(warning); + } + Lint::Ok => {} + Lint::Recheck => recheck = true, } } + recheck } pub(crate) fn location(&self) -> &str { diff --git a/tokio-console/src/warnings.rs b/tokio-console/src/warnings.rs index fd049cd67..87bdada5c 100644 --- a/tokio-console/src/warnings.rs +++ b/tokio-console/src/warnings.rs @@ -11,8 +11,8 @@ use std::{ /// generating a warning message describing it. The [`Linter`] type wraps an /// instance of this trait to track active instances of the warning. pub trait Warn: Debug { - /// Returns `true` if the warning applies to `val`. - fn check(&self, val: &T) -> bool; + /// Returns if the warning applies to `val`. + fn check(&self, val: &T) -> Warning; /// Formats a description of the warning detected for a *specific* `val`. /// @@ -50,6 +50,31 @@ pub trait Warn: Debug { fn summary(&self) -> &str; } +/// A result for a linter check +pub(crate) enum Lint { + /// No warning applies to the entity + Ok, + + /// The cloned instance of `Self` should be held by the entity that + /// generated the warning, so that it can be formatted. Holding the clone of + /// `Self` will increment the warning count for that entity. + Warning(Linter), + + /// The lint should be rechecked as the conditions to allow for checking are + /// not satisfied yet + Recheck, +} + +/// A result for a warning check +pub enum Warning { + /// Set `true` if the warning applies or `false` otherwise + Check(bool), + + /// The warning should be rechecked as the conditions to allow for checking + /// are not satisfied yet + Recheck, +} + #[derive(Debug)] pub(crate) struct Linter(Rc>); @@ -61,17 +86,12 @@ impl Linter { Self(Rc::new(warning)) } - /// Checks if the warning applies to a particular entity, returning a clone - /// of `Self` if it does. - /// - /// The cloned instance of `Self` should be held by the entity that - /// generated the warning, so that it can be formatted. Holding the clone of - /// `Self` will increment the warning count for that entity. - pub(crate) fn check(&self, val: &T) -> Option { - if self.0.check(val) { - Some(Self(self.0.clone())) - } else { - None + /// Checks if the warning applies to a particular entity + pub(crate) fn check(&self, val: &T) -> Lint { + match self.0.check(val) { + Warning::Check(false) => Lint::Ok, + Warning::Check(true) => Lint::Warning(Self(self.0.clone())), + Warning::Recheck => Lint::Recheck, } } @@ -82,7 +102,7 @@ impl Linter { pub(crate) fn format(&self, val: &T) -> String { debug_assert!( - self.0.check(val), + matches!(self.0.check(val), Warning::Check(true)), "tried to format a warning for a {} that did not have that warning!", std::any::type_name::() ); @@ -124,9 +144,9 @@ impl Warn for SelfWakePercent { self.description.as_str() } - fn check(&self, task: &Task) -> bool { + fn check(&self, task: &Task) -> Warning { let self_wakes = task.self_wake_percent(); - self_wakes > self.min_percent + Warning::Check(self_wakes > self.min_percent) } fn format(&self, task: &Task) -> String { @@ -146,8 +166,13 @@ impl Warn for LostWaker { "tasks have lost their waker" } - fn check(&self, task: &Task) -> bool { - !task.is_completed() && task.waker_count() == 0 && !task.is_running() && !task.is_awakened() + fn check(&self, task: &Task) -> Warning { + Warning::Check( + !task.is_completed() + && task.waker_count() == 0 + && !task.is_running() + && !task.is_awakened(), + ) } fn format(&self, _: &Task) -> String { @@ -186,18 +211,22 @@ impl Warn for NeverYielded { self.description.as_str() } - fn check(&self, task: &Task) -> bool { + fn check(&self, task: &Task) -> Warning { // Don't fire warning for tasks that are waiting to run if task.state() != TaskState::Running { - return false; + return Warning::Check(false); } if task.total_polls() > 1 { - return false; + return Warning::Check(false); } // Avoid short-lived task false positives - task.busy(SystemTime::now()) >= self.min_duration + if task.busy(SystemTime::now()) >= self.min_duration { + Warning::Check(true) + } else { + Warning::Recheck + } } fn format(&self, task: &Task) -> String { From 9a9b56e12b550d03a835b9343cd9c420d431cab1 Mon Sep 17 00:00:00 2001 From: Jeff Lai Date: Tue, 12 Sep 2023 10:27:09 -0400 Subject: [PATCH 09/15] Addressed code review comments --- tokio-console/src/state/tasks.rs | 51 +++++++++++-------- tokio-console/src/warnings.rs | 86 ++++++++++++++++++-------------- 2 files changed, 78 insertions(+), 59 deletions(-) diff --git a/tokio-console/src/state/tasks.rs b/tokio-console/src/state/tasks.rs index ef939c426..9822696cf 100644 --- a/tokio-console/src/state/tasks.rs +++ b/tokio-console/src/state/tasks.rs @@ -146,6 +146,9 @@ impl TasksState { let mut stats_update = update.stats_update; let linters = &self.linters; + // Gathers the tasks that need to be linted again on the next update cycle + let mut next_pending_lint = HashSet::new(); + self.tasks .insert_with(visibility, update.new_tasks, |ids, mut task| { if task.id.is_none() { @@ -218,37 +221,34 @@ impl TasksState { warnings: Vec::new(), location, }; - let recheck = task.lint(linters); - if recheck { - self.pending_lint.insert(task.id); + if matches!(task.lint(linters), TaskLintResult::RequiresRecheck) { + next_pending_lint.insert(task.id); } Some((id, task)) }); - let mut checked = HashSet::new(); for (stats, mut task) in self.tasks.updated(stats_update) { tracing::trace!(?task, ?stats, "processing stats update for"); task.stats = stats.into(); - let recheck = task.lint(linters); - if !recheck { + if matches!(task.lint(linters), TaskLintResult::RequiresRecheck) { + next_pending_lint.insert(task.id); + } else { + // Avoid linting this task again this cycle self.pending_lint.remove(&task.id); } - checked.insert(task.id); } - self.pending_lint.retain(|id| { - if checked.contains(id) { - true - } else { - let recheck = self - .tasks - .get(*id) - .expect("task pending lint is not in task store") - .borrow_mut() - .lint(linters); - recheck + for id in &self.pending_lint { + if let Some(task) = self.tasks.get(*id) { + if matches!( + task.borrow_mut().lint(linters), + TaskLintResult::RequiresRecheck + ) { + next_pending_lint.insert(*id); + } } - }); + } + self.pending_lint = next_pending_lint; self.dropped_events += update.dropped_events; } @@ -453,7 +453,7 @@ impl Task { &self.warnings[..] } - fn lint(&mut self, linters: &[Linter]) -> bool { + fn lint(&mut self, linters: &[Linter]) -> TaskLintResult { self.warnings.clear(); let mut recheck = false; for lint in linters { @@ -467,7 +467,11 @@ impl Task { Lint::Recheck => recheck = true, } } - recheck + if recheck { + TaskLintResult::RequiresRecheck + } else { + TaskLintResult::Linted + } } pub(crate) fn location(&self) -> &str { @@ -475,6 +479,11 @@ impl Task { } } +enum TaskLintResult { + Linted, + RequiresRecheck, +} + impl From for TaskStats { fn from(pb: proto::tasks::Stats) -> Self { let created_at = pb diff --git a/tokio-console/src/warnings.rs b/tokio-console/src/warnings.rs index 87bdada5c..745b27625 100644 --- a/tokio-console/src/warnings.rs +++ b/tokio-console/src/warnings.rs @@ -50,31 +50,6 @@ pub trait Warn: Debug { fn summary(&self) -> &str; } -/// A result for a linter check -pub(crate) enum Lint { - /// No warning applies to the entity - Ok, - - /// The cloned instance of `Self` should be held by the entity that - /// generated the warning, so that it can be formatted. Holding the clone of - /// `Self` will increment the warning count for that entity. - Warning(Linter), - - /// The lint should be rechecked as the conditions to allow for checking are - /// not satisfied yet - Recheck, -} - -/// A result for a warning check -pub enum Warning { - /// Set `true` if the warning applies or `false` otherwise - Check(bool), - - /// The warning should be rechecked as the conditions to allow for checking - /// are not satisfied yet - Recheck, -} - #[derive(Debug)] pub(crate) struct Linter(Rc>); @@ -89,8 +64,8 @@ impl Linter { /// Checks if the warning applies to a particular entity pub(crate) fn check(&self, val: &T) -> Lint { match self.0.check(val) { - Warning::Check(false) => Lint::Ok, - Warning::Check(true) => Lint::Warning(Self(self.0.clone())), + Warning::Ok => Lint::Ok, + Warning::Warn => Lint::Warning(Self(self.0.clone())), Warning::Recheck => Lint::Recheck, } } @@ -102,7 +77,7 @@ impl Linter { pub(crate) fn format(&self, val: &T) -> String { debug_assert!( - matches!(self.0.check(val), Warning::Check(true)), + matches!(self.0.check(val), Warning::Ok), "tried to format a warning for a {} that did not have that warning!", std::any::type_name::() ); @@ -114,6 +89,34 @@ impl Linter { } } +/// A result for a linter check +pub(crate) enum Lint { + /// No warning applies to the entity + Ok, + + /// The cloned instance of `Self` should be held by the entity that + /// generated the warning, so that it can be formatted. Holding the clone of + /// `Self` will increment the warning count for that entity. + Warning(Linter), + + /// The lint should be rechecked as the conditions to allow for checking are + /// not satisfied yet + Recheck, +} + +/// A result for a warning check +pub enum Warning { + /// No warning for this entity. + Ok, + + /// A warning has been detected for this entity. + Warn, + + /// The warning should be rechecked as the conditions to allow for checking + /// are not satisfied yet + Recheck, +} + #[derive(Clone, Debug)] pub(crate) struct SelfWakePercent { min_percent: u64, @@ -146,7 +149,11 @@ impl Warn for SelfWakePercent { fn check(&self, task: &Task) -> Warning { let self_wakes = task.self_wake_percent(); - Warning::Check(self_wakes > self.min_percent) + if self_wakes > self.min_percent { + Warning::Warn + } else { + Warning::Ok + } } fn format(&self, task: &Task) -> String { @@ -167,12 +174,15 @@ impl Warn for LostWaker { } fn check(&self, task: &Task) -> Warning { - Warning::Check( - !task.is_completed() - && task.waker_count() == 0 - && !task.is_running() - && !task.is_awakened(), - ) + if !task.is_completed() + && task.waker_count() == 0 + && !task.is_running() + && !task.is_awakened() + { + Warning::Warn + } else { + Warning::Ok + } } fn format(&self, _: &Task) -> String { @@ -214,16 +224,16 @@ impl Warn for NeverYielded { fn check(&self, task: &Task) -> Warning { // Don't fire warning for tasks that are waiting to run if task.state() != TaskState::Running { - return Warning::Check(false); + return Warning::Ok; } if task.total_polls() > 1 { - return Warning::Check(false); + return Warning::Ok; } // Avoid short-lived task false positives if task.busy(SystemTime::now()) >= self.min_duration { - Warning::Check(true) + Warning::Warn } else { Warning::Recheck } From 4e3616b9cd2ba3b2a3b490382287fa8c875c5c18 Mon Sep 17 00:00:00 2001 From: jefftt Date: Thu, 14 Sep 2023 08:28:47 -0400 Subject: [PATCH 10/15] Update tokio-console/src/state/tasks.rs Co-authored-by: Hayden Stainsby --- tokio-console/src/state/tasks.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tokio-console/src/state/tasks.rs b/tokio-console/src/state/tasks.rs index 9822696cf..affb59e2c 100644 --- a/tokio-console/src/state/tasks.rs +++ b/tokio-console/src/state/tasks.rs @@ -240,10 +240,7 @@ impl TasksState { for id in &self.pending_lint { if let Some(task) = self.tasks.get(*id) { - if matches!( - task.borrow_mut().lint(linters), - TaskLintResult::RequiresRecheck - ) { + if let TaskLintResult::RequiresRecheck = task.borrow_mut().lint(linters) { next_pending_lint.insert(*id); } } From 06b02343558dd92d6da5d1324a43be8b8067cab0 Mon Sep 17 00:00:00 2001 From: jefftt Date: Thu, 14 Sep 2023 08:29:22 -0400 Subject: [PATCH 11/15] Update tokio-console/src/state/tasks.rs Co-authored-by: Hayden Stainsby --- tokio-console/src/state/tasks.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tokio-console/src/state/tasks.rs b/tokio-console/src/state/tasks.rs index affb59e2c..f9ac8befa 100644 --- a/tokio-console/src/state/tasks.rs +++ b/tokio-console/src/state/tasks.rs @@ -221,7 +221,7 @@ impl TasksState { warnings: Vec::new(), location, }; - if matches!(task.lint(linters), TaskLintResult::RequiresRecheck) { + if let TaskLintResult::RequiresRecheck = task.lint(linters) { next_pending_lint.insert(task.id); } Some((id, task)) From 9a9b80e61d7e4db458fd7b9221d7b0166a7ed290 Mon Sep 17 00:00:00 2001 From: jefftt Date: Thu, 14 Sep 2023 08:29:45 -0400 Subject: [PATCH 12/15] Update tokio-console/src/warnings.rs Co-authored-by: Hayden Stainsby --- tokio-console/src/warnings.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tokio-console/src/warnings.rs b/tokio-console/src/warnings.rs index 745b27625..34ca6bd42 100644 --- a/tokio-console/src/warnings.rs +++ b/tokio-console/src/warnings.rs @@ -77,7 +77,7 @@ impl Linter { pub(crate) fn format(&self, val: &T) -> String { debug_assert!( - matches!(self.0.check(val), Warning::Ok), + matches!(self.0.check(val), Warning::Warn), "tried to format a warning for a {} that did not have that warning!", std::any::type_name::() ); From 1fe3ae691d77840cd51ae6c78b4b8b2756597600 Mon Sep 17 00:00:00 2001 From: jefftt Date: Thu, 14 Sep 2023 08:30:07 -0400 Subject: [PATCH 13/15] Update tokio-console/src/state/tasks.rs Co-authored-by: Hayden Stainsby --- tokio-console/src/state/tasks.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tokio-console/src/state/tasks.rs b/tokio-console/src/state/tasks.rs index f9ac8befa..abf2e56dc 100644 --- a/tokio-console/src/state/tasks.rs +++ b/tokio-console/src/state/tasks.rs @@ -230,11 +230,10 @@ impl TasksState { for (stats, mut task) in self.tasks.updated(stats_update) { tracing::trace!(?task, ?stats, "processing stats update for"); task.stats = stats.into(); - if matches!(task.lint(linters), TaskLintResult::RequiresRecheck) { - next_pending_lint.insert(task.id); - } else { + match task.lint(linters) { + TaskLintResult::RequiresRecheck => next_pending_lint.insert(task.id), // Avoid linting this task again this cycle - self.pending_lint.remove(&task.id); + _ => self.pending_lint.remove(&task.id), } } From 17ec390e16919ae70d614d8b5bd65813ffc12bdb Mon Sep 17 00:00:00 2001 From: Jeff Lai Date: Thu, 14 Sep 2023 08:30:39 -0400 Subject: [PATCH 14/15] Move Warning back --- tokio-console/src/warnings.rs | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/tokio-console/src/warnings.rs b/tokio-console/src/warnings.rs index 34ca6bd42..59d43f99f 100644 --- a/tokio-console/src/warnings.rs +++ b/tokio-console/src/warnings.rs @@ -50,6 +50,19 @@ pub trait Warn: Debug { fn summary(&self) -> &str; } +/// A result for a warning check +pub enum Warning { + /// No warning for this entity. + Ok, + + /// A warning has been detected for this entity. + Warn, + + /// The warning should be rechecked as the conditions to allow for checking + /// are not satisfied yet + Recheck, +} + #[derive(Debug)] pub(crate) struct Linter(Rc>); @@ -104,19 +117,6 @@ pub(crate) enum Lint { Recheck, } -/// A result for a warning check -pub enum Warning { - /// No warning for this entity. - Ok, - - /// A warning has been detected for this entity. - Warn, - - /// The warning should be rechecked as the conditions to allow for checking - /// are not satisfied yet - Recheck, -} - #[derive(Clone, Debug)] pub(crate) struct SelfWakePercent { min_percent: u64, From ca39241aed3f957a45052fc52b66ba389d921461 Mon Sep 17 00:00:00 2001 From: Jeff Lai Date: Thu, 14 Sep 2023 08:32:15 -0400 Subject: [PATCH 15/15] Fix compiliation --- tokio-console/src/state/tasks.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tokio-console/src/state/tasks.rs b/tokio-console/src/state/tasks.rs index abf2e56dc..97aa4f35b 100644 --- a/tokio-console/src/state/tasks.rs +++ b/tokio-console/src/state/tasks.rs @@ -234,7 +234,7 @@ impl TasksState { TaskLintResult::RequiresRecheck => next_pending_lint.insert(task.id), // Avoid linting this task again this cycle _ => self.pending_lint.remove(&task.id), - } + }; } for id in &self.pending_lint {