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.

Sign up for GitHub

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

Jump to bottom

fix(console): reduce decimal digits in UI #402

Merged
merged 5 commits into from
Mar 8, 2023
Merged

fix(console): reduce decimal digits in UI #402

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5995376
fix(console): reduce decimal digits in UI
hds Mar 2, 2023
d5a59ad
Update tokio-console/src/view/styles.rs
hds Mar 6, 2023
d6f934a
fix the double-allocation when formatting durations
hds Mar 7, 2023
8a382f3
convert time_units param width back to `Option<usize>`
hds Mar 7, 2023
4c62307
used width.saturating_sub(4) instead of my if-else mess
hds Mar 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 5 additions & 10 deletions tokio-console/src/view/async_ops.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::{
view::{
self, bold,
table::{self, TableList, TableListState},
DUR_LEN, DUR_PRECISION,
DUR_LEN, DUR_TABLE_PRECISION,
},
};

Expand Down Expand Up @@ -106,12 +106,7 @@ impl TableList<9> for AsyncOpsTable {
let mut polls_width = view::Width::new(Self::WIDTHS[7] as u16);

let dur_cell = |dur: std::time::Duration| -> Cell<'static> {
Cell::from(styles.time_units(format!(
"{:>width$.prec$?}",
dur,
width = DUR_LEN,
prec = DUR_PRECISION,
)))
Cell::from(styles.time_units(dur, DUR_TABLE_PRECISION, Some(DUR_LEN)))
};

let rows = {
Expand Down Expand Up @@ -222,9 +217,9 @@ impl TableList<9> for AsyncOpsTable {
parent_width.constraint(),
task_width.constraint(),
source_width.constraint(),
layout::Constraint::Length(DUR_LEN as u16),
layout::Constraint::Length(DUR_LEN as u16),
layout::Constraint::Length(DUR_LEN as u16),
layout::Constraint::Length(DUR_LEN.get() as u16),
layout::Constraint::Length(DUR_LEN.get() as u16),
layout::Constraint::Length(DUR_LEN.get() as u16),
polls_width.constraint(),
attributes_width,
];
Expand Down
7 changes: 5 additions & 2 deletions tokio-console/src/view/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::view::{resources::ResourcesTable, table::TableListState, tasks::TasksTable};
use crate::{input, state::State};
use std::num::NonZeroUsize;
use std::{borrow::Cow, cmp};
use tui::{
layout,
Expand All @@ -18,11 +19,13 @@ mod tasks;
pub(crate) use self::styles::{Palette, Styles};
pub(crate) use self::table::SortBy;

const DUR_LEN: usize = 10;
// SAFETY: 6 is non-zero.
const DUR_LEN: NonZeroUsize = unsafe { NonZeroUsize::new_unchecked(6) };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't love that this is unsafe. note that NonZeroUsize::new is const fn in our MSRV, so I believe this could just be

Suggested change
const DUR_LEN: NonZeroUsize = unsafe { NonZeroUsize::new_unchecked(6) };
const DUR_LEN: NonZeroUsize = NonZeroUsize::new(6).expect("6 is non-zero");

alternatively, we could just get rid of the NonZeroUsize, which i would also be fine about.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The unwrap wasn't const in our MSRV I believe.

I changed this back to Option<usize>.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I get the following when trying that code:

error: `std::option::Option::<T>::expect` is not yet stable as a const fn
  --> tokio-console/src/view/mod.rs:22:31
   |
22 | const DUR_LEN: NonZeroUsize = NonZeroUsize::new(6).expect("6 is non-zero");
   |

So it seems like while NonZeroUsize::new() is stable, we would also need const Option::unwrap(), which isn't yet stable.

Another thing I've learnt today.

// This data is only updated every second, so it doesn't make a ton of
// sense to have a lot of precision in timestamps (and this makes sure
// there's room for the unit!)
const DUR_PRECISION: usize = 4;
const DUR_LIST_PRECISION: usize = 2;
const DUR_TABLE_PRECISION: usize = 0;
const TABLE_HIGHLIGHT_SYMBOL: &str = ">> ";

pub struct View {
Expand Down
13 changes: 6 additions & 7 deletions tokio-console/src/view/resources.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::{
view::{
self, bold,
table::{self, TableList, TableListState},
DUR_LEN, DUR_PRECISION,
DUR_LEN, DUR_TABLE_PRECISION,
},
};

Expand Down Expand Up @@ -104,12 +104,11 @@ impl TableList<9> for ResourcesTable {
))),
Cell::from(parent_width.update_str(resource.parent_id()).to_owned()),
Cell::from(kind_width.update_str(resource.kind()).to_owned()),
Cell::from(styles.time_units(format!(
"{:>width$.prec$?}",
Cell::from(styles.time_units(
resource.total(now),
width = DUR_LEN,
prec = DUR_PRECISION,
))),
DUR_TABLE_PRECISION,
Some(DUR_LEN),
)),
Cell::from(target_width.update_str(resource.target()).to_owned()),
Cell::from(type_width.update_str(resource.concrete_type()).to_owned()),
Cell::from(resource.type_visibility().render(styles)),
Expand Down Expand Up @@ -187,7 +186,7 @@ impl TableList<9> for ResourcesTable {
id_width.constraint(),
parent_width.constraint(),
kind_width.constraint(),
layout::Constraint::Length(DUR_LEN as u16),
layout::Constraint::Length(DUR_LEN.get() as u16),
target_width.constraint(),
type_width.constraint(),
layout::Constraint::Length(viz_len),
Expand Down
145 changes: 120 additions & 25 deletions tokio-console/src/view/styles.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::config;
use serde::{Deserialize, Serialize};
use std::{borrow::Cow, str::FromStr};
use std::{num::NonZeroUsize, str::FromStr, time::Duration};
use tui::{
style::{Color, Modifier, Style},
text::Span,
Expand Down Expand Up @@ -32,11 +32,41 @@ pub enum Palette {
All,
}

/// Represents formatted time spans.
///
/// Distinguishing between different units allows appropriate colouring.
enum FormattedDuration {
hawkw marked this conversation as resolved.
Show resolved Hide resolved
/// Days (and no minor unit), e.g. `102d`
Days(String),
/// Days with hours, e.g. `12d03h`
DaysHours(String),
/// Hours with minutes, e.g. `14h32m`
HoursMinutes(String),
/// Minutes with seconds, e.g. `43m02s`
MinutesSeconds(String),
/// The `time::Duration` debug string which uses units ranging from
/// picoseconds (`ps`) to seconds (`s`). May contain decimal digits
/// (e.g. `628.76ms`) or not (e.g. `32ns`)
Debug(String),
}

impl FormattedDuration {
fn into_inner(self) -> String {
match self {
Self::Days(inner) => inner,
Self::DaysHours(inner) => inner,
Self::HoursMinutes(inner) => inner,
Self::MinutesSeconds(inner) => inner,
Self::Debug(inner) => inner,
}
}
}

fn fg_style(color: Color) -> Style {
Style::default().fg(color)
}

// === impl Config ===
// === impl Styles ===

impl Styles {
pub fn from_config(config: config::ViewOptions) -> Self {
Expand Down Expand Up @@ -126,39 +156,104 @@ impl Styles {
}
}

pub fn time_units<'a>(&self, text: impl Into<Cow<'a, str>>) -> Span<'a> {
let mut text = text.into();
if !self.toggles.color_durations() {
return Span::raw(text);
}
/// Creates a span with a formatted duration inside.
///
/// The formatted duration will be colored depending on the palette
/// defined for this `Styles` object.
///
/// If the `width` parameter is `None` then no padding will be
/// added. Otherwise the text in the span will be left-padded to
/// the specified width (right aligned).
pub fn time_units<'a>(
&self,
dur: Duration,
prec: usize,
width: Option<NonZeroUsize>,
) -> Span<'a> {
let formatted = self.duration_text(dur, width.map_or(0, |w| w.get()), prec);

if !self.utf8 {
if let Some(mu_offset) = text.find("µs") {
text.to_mut().replace_range(mu_offset.., "us");
}
if !self.toggles.color_durations() {
return Span::raw(formatted.into_inner());
}

let style = match self.palette {
Palette::NoColors => return Span::raw(text),
Palette::Ansi8 | Palette::Ansi16 => match text.as_ref() {
s if s.ends_with("ps") => fg_style(Color::Blue),
s if s.ends_with("ns") => fg_style(Color::Green),
s if s.ends_with("µs") || s.ends_with("us") => fg_style(Color::Yellow),
s if s.ends_with("ms") => fg_style(Color::Red),
s if s.ends_with('s') => fg_style(Color::Magenta),
Palette::NoColors => return Span::raw(formatted.into_inner()),
Palette::Ansi8 | Palette::Ansi16 => match &formatted {
FormattedDuration::Days(_) => fg_style(Color::Blue),
FormattedDuration::DaysHours(_) => fg_style(Color::Blue),
FormattedDuration::HoursMinutes(_) => fg_style(Color::Cyan),
FormattedDuration::MinutesSeconds(_) => fg_style(Color::Green),
FormattedDuration::Debug(s) if s.ends_with("ps") => fg_style(Color::Gray),
FormattedDuration::Debug(s) if s.ends_with("ns") => fg_style(Color::Gray),
FormattedDuration::Debug(s) if s.ends_with("µs") || s.ends_with("us") => {
fg_style(Color::Magenta)
}
FormattedDuration::Debug(s) if s.ends_with("ms") => fg_style(Color::Red),
FormattedDuration::Debug(s) if s.ends_with('s') => fg_style(Color::Yellow),
_ => Style::default(),
},
Palette::Ansi256 | Palette::All => match text.as_ref() {
s if s.ends_with("ps") => fg_style(Color::Indexed(40)), // green 3
s if s.ends_with("ns") => fg_style(Color::Indexed(41)), // spring green 3
s if s.ends_with("µs") || s.ends_with("us") => fg_style(Color::Indexed(42)), // spring green 2
s if s.ends_with("ms") => fg_style(Color::Indexed(43)), // cyan 3
s if s.ends_with('s') => fg_style(Color::Indexed(44)), // dark turquoise,
Palette::Ansi256 | Palette::All => match &formatted {
FormattedDuration::Days(_) => fg_style(Color::Indexed(33)), // dodger blue 1
FormattedDuration::DaysHours(_) => fg_style(Color::Indexed(33)), // dodger blue 1
FormattedDuration::HoursMinutes(_) => fg_style(Color::Indexed(39)), // deep sky blue 1
FormattedDuration::MinutesSeconds(_) => fg_style(Color::Indexed(45)), // turquoise 2
FormattedDuration::Debug(s) if s.ends_with("ps") => fg_style(Color::Indexed(40)), // green 3
FormattedDuration::Debug(s) if s.ends_with("ns") => fg_style(Color::Indexed(41)), // spring green 3
FormattedDuration::Debug(s) if s.ends_with("µs") || s.ends_with("us") => {
fg_style(Color::Indexed(42))
} // spring green 2
FormattedDuration::Debug(s) if s.ends_with("ms") => fg_style(Color::Indexed(43)), // cyan 3
FormattedDuration::Debug(s) if s.ends_with('s') => fg_style(Color::Indexed(44)), // dark turquoise,
_ => Style::default(),
},
};

Span::styled(text, style)
Span::styled(formatted.into_inner(), style)
}

fn duration_text(&self, dur: Duration, width: usize, prec: usize) -> FormattedDuration {
let secs = dur.as_secs();

if secs >= 60 * 60 * 24 * 100 {
let days = secs / (60 * 60 * 24);
FormattedDuration::Days(format!("{days:>width$}d", days = days, width = width))
} else if secs >= 60 * 60 * 24 {
let hours = secs / (60 * 60);
FormattedDuration::DaysHours(format!(
"{days:>leading_width$}d{hours:02.0}h",
days = hours / 24,
hours = hours % 24,
// Subtract the known 4 characters that trail the days value.
leading_width = if width > 4 { width - 4 } else { 0 }
hds marked this conversation as resolved.
Show resolved Hide resolved
))
} else if secs >= 60 * 60 {
let mins = secs / 60;
FormattedDuration::HoursMinutes(format!(
"{hours:>leading_width$}h{minutes:02.0}m",
hours = mins / 60,
minutes = mins % 60,
// Subtract the known 4 characters that trail the hours value.
leading_width = if width > 4 { width - 4 } else { 0 }
hds marked this conversation as resolved.
Show resolved Hide resolved
))
} else if secs >= 60 {
FormattedDuration::MinutesSeconds(format!(
"{minutes:>leading_width$}m{seconds:02.0}s",
minutes = secs / 60,
seconds = secs % 60,
// Subtract the known 4 characters that trail the minutes value.
leading_width = if width > 4 { width - 4 } else { 0 },
hds marked this conversation as resolved.
Show resolved Hide resolved
))
} else {
let mut text = format!("{:>width$.prec$?}", dur, width = width, prec = prec);

if !self.utf8 {
if let Some(mu_offset) = text.find("µs") {
text.replace_range(mu_offset.., "us");
}
}

FormattedDuration::Debug(text)
}
}

pub fn terminated(&self) -> Style {
Expand Down
31 changes: 15 additions & 16 deletions tokio-console/src/view/task.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use crate::{
view::{
self, bold,
mini_histogram::{HistogramMetadata, MiniHistogram},
DUR_LIST_PRECISION,
},
};
use std::{
Expand Down Expand Up @@ -329,29 +330,27 @@ impl Details {
fn make_percentiles_widget(&self, styles: &view::Styles) -> Text<'static> {
let mut text = Text::default();
let histogram = self.poll_times_histogram();
let percentiles =
histogram
.iter()
.flat_map(|&DurationHistogram { ref histogram, .. }| {
let pairs = [10f64, 25f64, 50f64, 75f64, 90f64, 95f64, 99f64]
.iter()
.map(move |i| (*i, histogram.value_at_percentile(*i)));
pairs.map(|pair| {
Spans::from(vec![
bold(format!("p{:>2}: ", pair.0)),
dur(styles, Duration::from_nanos(pair.1)),
])
})
});
let percentiles = histogram
.iter()
.flat_map(|&DurationHistogram { histogram, .. }| {
let pairs = [10f64, 25f64, 50f64, 75f64, 90f64, 95f64, 99f64]
.iter()
.map(move |i| (*i, histogram.value_at_percentile(*i)));
pairs.map(|pair| {
Spans::from(vec![
bold(format!("p{:>2}: ", pair.0)),
dur(styles, Duration::from_nanos(pair.1)),
])
})
});
text.extend(percentiles);
text
}
}

fn dur(styles: &view::Styles, dur: std::time::Duration) -> Span<'static> {
const DUR_PRECISION: usize = 4;
// TODO(eliza): can we not have to use `format!` to make a string here? is
// there a way to just give TUI a `fmt::Debug` implementation, or does it
// have to be given a string in order to do layout stuff?
styles.time_units(format!("{:.prec$?}", dur, prec = DUR_PRECISION))
styles.time_units(dur, DUR_LIST_PRECISION, None)
}
21 changes: 8 additions & 13 deletions tokio-console/src/view/tasks.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::{
view::{
self, bold,
table::{self, TableList, TableListState},
DUR_LEN, DUR_PRECISION,
DUR_LEN, DUR_TABLE_PRECISION,
},
};
use tui::{
Expand Down Expand Up @@ -68,12 +68,7 @@ impl TableList<11> for TasksTable {
.sort(now, &mut table_list_state.sorted_items);

let dur_cell = |dur: std::time::Duration| -> Cell<'static> {
Cell::from(styles.time_units(format!(
"{:>width$.prec$?}",
dur,
width = DUR_LEN,
prec = DUR_PRECISION,
)))
Cell::from(styles.time_units(dur, DUR_TABLE_PRECISION, Some(DUR_LEN)))
};

// Start out wide enough to display the column headers...
Expand Down Expand Up @@ -193,9 +188,9 @@ impl TableList<11> for TasksTable {
let fixed_col_width = id_width.chars()
+ STATE_LEN
+ name_width.chars()
+ DUR_LEN as u16
+ DUR_LEN as u16
+ DUR_LEN as u16
+ DUR_LEN.get() as u16
+ DUR_LEN.get() as u16
+ DUR_LEN.get() as u16
+ POLLS_LEN as u16
+ target_width.chars();
*/
Expand Down Expand Up @@ -254,9 +249,9 @@ impl TableList<11> for TasksTable {
id_width.constraint(),
layout::Constraint::Length(state_len),
name_width.constraint(),
layout::Constraint::Length(DUR_LEN as u16),
layout::Constraint::Length(DUR_LEN as u16),
layout::Constraint::Length(DUR_LEN as u16),
layout::Constraint::Length(DUR_LEN.get() as u16),
layout::Constraint::Length(DUR_LEN.get() as u16),
layout::Constraint::Length(DUR_LEN.get() as u16),
polls_width.constraint(),
target_width.constraint(),
location_width.constraint(),
Expand Down