From de34605fff39ec9a0656766da6bf724c8062010f Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Wed, 8 Sep 2021 09:47:44 -0700 Subject: [PATCH] feat(console): add self-wake percentage and warning (#113) Depends on #112 This branch builds on #112 by calculating the *percentage* of a task's wakeups that were self-wakes, and displaying them in the tasks view. It also adds the beginnings of a rudimentary "Warnings" system, and displays warnings for any tasks who have woken themselves for more than 50% of their wakeups. There is a new `Warn` trait for generating warnings that can be used to add new warnings in the future. The warnings functionality can almost certainly be expanded --- for example, it would be nice to be able to quickly jump to the details view for a task that has warnings. In the future, I could imagine generating web documentation with details about a particular warning and how to fix it, like [clippy has][1], and linking to them from the console. But, this is a start, at least! [1]: https://rust-lang.github.io/rust-clippy/v0.0.174/ ![image](https://user-images.githubusercontent.com/2796466/132146062-9599ba12-cb17-48f8-b15c-4ba91947e282.png) ![image](https://user-images.githubusercontent.com/2796466/132146081-6dac231c-e929-4f93-b6ef-ac16f1dd166d.png) Signed-off-by: Eliza Weisman --- console/src/main.rs | 2 + console/src/tasks.rs | 7 ++- console/src/util.rs | 28 ++++++++++ console/src/view/task.rs | 27 ++++++---- console/src/view/tasks.rs | 106 +++++++++++++++++++++++++++++--------- console/src/warnings.rs | 37 +++++++++++++ 6 files changed, 171 insertions(+), 36 deletions(-) create mode 100644 console/src/util.rs create mode 100644 console/src/warnings.rs diff --git a/console/src/main.rs b/console/src/main.rs index 0c973051..162ab3d0 100644 --- a/console/src/main.rs +++ b/console/src/main.rs @@ -20,7 +20,9 @@ mod conn; mod input; mod tasks; mod term; +mod util; mod view; +mod warnings; #[tokio::main] async fn main() -> color_eyre::Result<()> { diff --git a/console/src/tasks.rs b/console/src/tasks.rs index 226c86c9..3fbf5474 100644 --- a/console/src/tasks.rs +++ b/console/src/tasks.rs @@ -1,4 +1,4 @@ -use crate::view; +use crate::{util::Percentage, view}; use console_api as proto; use hdrhistogram::Histogram; use std::{ @@ -379,6 +379,11 @@ impl Task { self.stats.self_wakes } + /// Returns the percentage of this task's total wakeups that were self-wakes. + pub(crate) fn self_wake_percent(&self) -> u64 { + self.self_wakes().percent_of(self.wakes()) + } + fn update(&mut self) { let completed = self.stats.total.is_some() && self.completed_for == 0; if completed { diff --git a/console/src/util.rs b/console/src/util.rs new file mode 100644 index 00000000..994cd7ea --- /dev/null +++ b/console/src/util.rs @@ -0,0 +1,28 @@ +pub(crate) trait Percentage { + // Using an extension trait for this is maybe a bit excessive, but making it + // a method has the nice advantage of making it *really* obvious which is + // the total and which is the amount. + fn percent_of(self, total: Self) -> Self; +} + +impl Percentage for usize { + fn percent_of(self, total: Self) -> Self { + percentage(total as f64, self as f64) as Self + } +} + +impl Percentage for u64 { + fn percent_of(self, total: Self) -> Self { + percentage(total as f64, self as f64) as Self + } +} + +pub(crate) fn percentage(total: f64, amount: f64) -> f64 { + debug_assert!( + total >= amount, + "assertion failed: total >= amount; total={}, amount={}", + total, + amount + ); + (amount / total) * 100.0 +} diff --git a/console/src/view/task.rs b/console/src/view/task.rs index d16c30dd..ac102073 100644 --- a/console/src/view/task.rs +++ b/console/src/view/task.rs @@ -133,34 +133,40 @@ impl TaskView { dur(styles, task.idle(now)), ])); - let wakers = Spans::from(vec![ + let mut waker_stats = vec![Spans::from(vec![ bold("Current wakers: "), Span::from(format!("{} (", task.waker_count())), bold("clones: "), Span::from(format!("{}, ", task.waker_clones())), bold("drops: "), Span::from(format!("{})", task.waker_drops())), - ]); + ])]; let mut wakeups = vec![ bold("Woken: "), Span::from(format!("{} times", task.wakes())), ]; - if task.self_wakes() > 0 { - wakeups.push(Span::raw(", ")); - wakeups.push(bold("Self Wakes: ")); - wakeups.push(Span::from(format!("{} times", task.self_wakes()))); - } - // If the task has been woken, add the time since wake to its stats as well. if let Some(since) = task.since_wake(now) { + wakeups.reserve(3); wakeups.push(Span::raw(", ")); wakeups.push(bold("last woken:")); wakeups.push(Span::from(format!(" {:?} ago", since))); } - let wakeups = Spans::from(wakeups); + waker_stats.push(Spans::from(wakeups)); + + if task.self_wakes() > 0 { + waker_stats.push(Spans::from(vec![ + bold("Self Wakes: "), + Span::from(format!( + "{} times ({}%)", + task.self_wakes(), + task.self_wake_percent() + )), + ])); + } let mut fields = Text::default(); fields.extend(task.formatted_fields().iter().cloned().map(Spans::from)); @@ -190,8 +196,7 @@ impl TaskView { } let task_widget = Paragraph::new(metrics).block(styles.border_block().title("Task")); - let wakers_widget = - Paragraph::new(vec![wakers, wakeups]).block(styles.border_block().title("Waker")); + let wakers_widget = Paragraph::new(waker_stats).block(styles.border_block().title("Waker")); let fields_widget = Paragraph::new(fields).block(styles.border_block().title("Fields")); let percentiles_widget = Paragraph::new( details diff --git a/console/src/view/tasks.rs b/console/src/view/tasks.rs index abd77326..7674b71a 100644 --- a/console/src/view/tasks.rs +++ b/console/src/view/tasks.rs @@ -1,7 +1,8 @@ use crate::{ input, - tasks::{self, TaskRef, TaskState}, + tasks::{self, Task, TaskRef, TaskState}, view::{self, bold}, + warnings, }; use std::convert::TryFrom; use tui::{ @@ -10,8 +11,12 @@ use tui::{ text::{self, Span, Spans}, widgets::{Cell, Paragraph, Row, Table, TableState}, }; -#[derive(Clone, Debug, Default)] + +#[derive(Debug)] pub(crate) struct List { + /// A list of linters (implementing the [`warnings::Warn`] trait) used to generate + /// warnings. + linters: Vec>>, sorted_tasks: Vec, sort_by: tasks::SortBy, table_state: TableState, @@ -76,27 +81,6 @@ impl List { area: layout::Rect, state: &mut tasks::State, ) { - let chunks = layout::Layout::default() - .direction(layout::Direction::Vertical) - .margin(0) - .constraints( - [ - layout::Constraint::Length(1), - layout::Constraint::Min(area.height - 1), - ] - .as_ref(), - ) - .split(area); - let controls_area = chunks[0]; - let tasks_area = chunks[1]; - - let now = if let Some(now) = state.last_updated_at() { - now - } else { - // If we have never gotten an update yet, skip... - return; - }; - const STATE_LEN: u16 = List::HEADER[1].len() as u16; const DUR_LEN: usize = 10; // This data is only updated every second, so it doesn't make a ton of @@ -105,6 +89,13 @@ impl List { const DUR_PRECISION: usize = 4; const POLLS_LEN: usize = 5; + let now = if let Some(now) = state.last_updated_at() { + now + } else { + // If we have never gotten an update yet, skip... + return; + }; + self.sorted_tasks.extend(state.take_new_tasks()); self.sort_by.sort(now, &mut self.sorted_tasks); @@ -123,17 +114,36 @@ impl List { let mut target_width = view::Width::new(Self::HEADER[7].len() as u16); let mut num_idle = 0; let mut num_running = 0; + let mut warnings = Vec::new(); let rows = { let id_width = &mut id_width; let target_width = &mut target_width; let name_width = &mut name_width; let num_running = &mut num_running; let num_idle = &mut num_idle; + let warnings = &mut warnings; + + let linters = &self.linters; self.sorted_tasks.iter().filter_map(move |task| { let task = task.upgrade()?; let task = task.borrow(); let state = task.state(); - + warnings.extend(linters.iter().filter_map(|warning| { + let warning = warning.check(&*task)?; + let task = if let Some(name) = task.name() { + Span::from(format!("Task '{}' (ID {}) ", name, task.id())) + } else { + Span::from(format!("Task ID {} ", task.id())) + }; + Some(Spans::from(vec![ + Span::styled( + styles.if_utf8("\u{26A0} ", "/!\\ "), + styles.fg(Color::LightYellow), + ), + task, + Span::from(warning), + ])) + })); // Count task states match state { TaskState::Running => *num_running += 1, @@ -215,6 +225,33 @@ impl List { + POLLS_LEN as u16 + target_width.chars(); */ + let layout = layout::Layout::default() + .direction(layout::Direction::Vertical) + .margin(0); + let (controls_area, tasks_area, warnings_area) = if warnings.is_empty() { + let chunks = layout + .constraints( + [ + layout::Constraint::Length(1), + layout::Constraint::Min(area.height - 1), + ] + .as_ref(), + ) + .split(area); + (chunks[0], chunks[1], None) + } else { + let chunks = layout + .constraints( + [ + layout::Constraint::Length(1), + layout::Constraint::Length(warnings.len() as u16 + 2), + layout::Constraint::Min(area.height - 1), + ] + .as_ref(), + ) + .split(area); + (chunks[0], chunks[2], Some(chunks[1])) + }; // Fill all remaining characters in the frame with the task's fields. // @@ -260,6 +297,14 @@ impl List { frame.render_widget(Paragraph::new(controls), controls_area); + if let Some(area) = warnings_area { + let block = styles.border_block().title(Spans::from(vec![ + bold("Warnings"), + Span::from(format!(" ({})", warnings.len())), + ])); + frame.render_widget(Paragraph::new(warnings).block(block), area); + } + self.sorted_tasks.retain(|t| t.upgrade().is_some()); } @@ -317,3 +362,16 @@ impl List { .unwrap_or_default() } } + +impl Default for List { + fn default() -> Self { + Self { + linters: vec![Box::new(warnings::SelfWakePercent::default())], + sorted_tasks: Vec::new(), + sort_by: tasks::SortBy::default(), + table_state: TableState::default(), + selected_column: 0, + sort_descending: false, + } + } +} diff --git a/console/src/warnings.rs b/console/src/warnings.rs new file mode 100644 index 00000000..972b80ca --- /dev/null +++ b/console/src/warnings.rs @@ -0,0 +1,37 @@ +use crate::tasks::Task; + +pub trait Warn: std::fmt::Debug { + fn check(&self, val: &T) -> Option; +} + +#[derive(Clone, Debug)] +pub(crate) struct SelfWakePercent { + min_percent: u64, +} + +impl SelfWakePercent { + pub(crate) const DEFAULT_PERCENT: u64 = 50; + pub(crate) fn new(min_percent: u64) -> Self { + Self { min_percent } + } +} + +impl Default for SelfWakePercent { + fn default() -> Self { + Self::new(Self::DEFAULT_PERCENT) + } +} + +impl Warn for SelfWakePercent { + fn check(&self, task: &Task) -> Option { + let self_wakes = task.self_wake_percent(); + if self_wakes > self.min_percent { + return Some(format!( + "has woken itself for more than {}% of its total wakeups ({}%)", + self.min_percent, self_wakes + )); + } + + None + } +}