other: slightly reduce the CPU time spent for draws (#918)

* other: group all dataset draws in a time chart

We used to draw each data set separately as a new canvas. Now, in one
canvas, we draw all datasets.

Note that this changes how dataset  lines are drawn - rather than
drawing one on top of another, it now draws kinda all at once. This
effect is *kinda* a bit better IMO, but it might also look a bit
more cluttered.

* other: optimize truncate_text

Flamegraphs showed that this area seems to be a bit heavy at times with
some inefficient use of iterators and collection. This change should
hopefully optimize this a bit by reducing some collections or
reallocations.

There can also be some further optimizations with less allocations from
callers.

* Reduce some redundant draws
This commit is contained in:
Clement Tsang 2022-11-29 03:53:58 -05:00 committed by GitHub
parent 913c9ed5c6
commit 9c3e60e74f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 118 additions and 72 deletions

View file

@ -41,6 +41,8 @@ doctest = true
doc = true doc = true
[profile.release] [profile.release]
# debug = true
# strip = false
debug = 0 debug = 0
strip = "symbols" strip = "symbols"
lto = true lto = true

View file

@ -32,6 +32,7 @@ pub async fn get_ram_data() -> crate::utils::error::Result<Option<MemHarvest>> {
let (mem_total_in_kib, mem_used_in_kib) = { let (mem_total_in_kib, mem_used_in_kib) = {
#[cfg(target_os = "linux")] #[cfg(target_os = "linux")]
{ {
// TODO: [OPT] is this efficient?
use smol::fs::read_to_string; use smol::fs::read_to_string;
let meminfo = read_to_string("/proc/meminfo").await?; let meminfo = read_to_string("/proc/meminfo").await?;

View file

@ -73,11 +73,9 @@ impl Default for CanvasColours {
Style::default().fg(Color::LightCyan), Style::default().fg(Color::LightCyan),
Style::default().fg(Color::LightGreen), Style::default().fg(Color::LightGreen),
Style::default().fg(Color::LightBlue), Style::default().fg(Color::LightBlue),
Style::default().fg(Color::LightRed),
Style::default().fg(Color::Cyan), Style::default().fg(Color::Cyan),
Style::default().fg(Color::Green), Style::default().fg(Color::Green),
Style::default().fg(Color::Blue), Style::default().fg(Color::Blue),
Style::default().fg(Color::Red),
], ],
border_style: Style::default().fg(text_colour), border_style: Style::default().fg(text_colour),
highlighted_border_style: Style::default().fg(HIGHLIGHT_COLOUR), highlighted_border_style: Style::default().fg(HIGHLIGHT_COLOUR),

View file

@ -4,6 +4,7 @@ use concat_string::concat_string;
use tui::{ use tui::{
backend::Backend, backend::Backend,
layout::{Constraint, Direction, Layout, Rect}, layout::{Constraint, Direction, Layout, Rect},
symbols::Marker,
terminal::Frame, terminal::Frame,
}; };
@ -209,8 +210,13 @@ impl Painter {
" CPU ".into() " CPU ".into()
}; };
let marker = if app_state.app_config_fields.use_dot {
Marker::Dot
} else {
Marker::Braille
};
TimeGraph { TimeGraph {
use_dot: app_state.app_config_fields.use_dot,
x_bounds, x_bounds,
hide_x_labels, hide_x_labels,
y_bounds: Y_BOUNDS, y_bounds: Y_BOUNDS,
@ -221,6 +227,7 @@ impl Painter {
is_expanded: app_state.is_expanded, is_expanded: app_state.is_expanded,
title_style: self.colours.widget_title_style, title_style: self.colours.widget_title_style,
legend_constraints: None, legend_constraints: None,
marker,
} }
.draw_time_graph(f, draw_loc, &points); .draw_time_graph(f, draw_loc, &points);
} }

View file

@ -3,6 +3,7 @@ use std::borrow::Cow;
use tui::{ use tui::{
backend::Backend, backend::Backend,
layout::{Constraint, Rect}, layout::{Constraint, Rect},
symbols::Marker,
terminal::Frame, terminal::Frame,
}; };
@ -104,8 +105,13 @@ impl Painter {
points points
}; };
let marker = if app_state.app_config_fields.use_dot {
Marker::Dot
} else {
Marker::Braille
};
TimeGraph { TimeGraph {
use_dot: app_state.app_config_fields.use_dot,
x_bounds, x_bounds,
hide_x_labels, hide_x_labels,
y_bounds: Y_BOUNDS, y_bounds: Y_BOUNDS,
@ -116,6 +122,7 @@ impl Painter {
is_expanded: app_state.is_expanded, is_expanded: app_state.is_expanded,
title_style: self.colours.widget_title_style, title_style: self.colours.widget_title_style,
legend_constraints: Some((Constraint::Ratio(3, 4), Constraint::Ratio(3, 4))), legend_constraints: Some((Constraint::Ratio(3, 4), Constraint::Ratio(3, 4))),
marker,
} }
.draw_time_graph(f, draw_loc, &points); .draw_time_graph(f, draw_loc, &points);
} }

View file

@ -1,6 +1,7 @@
use tui::{ use tui::{
backend::Backend, backend::Backend,
layout::{Constraint, Direction, Layout, Rect}, layout::{Constraint, Direction, Layout, Rect},
symbols::Marker,
terminal::Frame, terminal::Frame,
text::Text, text::Text,
widgets::{Block, Borders, Row, Table}, widgets::{Block, Borders, Row, Table},
@ -142,8 +143,13 @@ impl Painter {
] ]
}; };
let marker = if app_state.app_config_fields.use_dot {
Marker::Dot
} else {
Marker::Braille
};
TimeGraph { TimeGraph {
use_dot: app_state.app_config_fields.use_dot,
x_bounds, x_bounds,
hide_x_labels, hide_x_labels,
y_bounds, y_bounds,
@ -154,6 +160,7 @@ impl Painter {
is_expanded: app_state.is_expanded, is_expanded: app_state.is_expanded,
title_style: self.colours.widget_title_style, title_style: self.colours.widget_title_style,
legend_constraints: Some(legend_constraints), legend_constraints: Some(legend_constraints),
marker,
} }
.draw_time_graph(f, draw_loc, &points); .draw_time_graph(f, draw_loc, &points);
} }

View file

@ -221,7 +221,11 @@ where
.iter() .iter()
.zip(&self.state.calculated_widths) .zip(&self.state.calculated_widths)
.filter_map(|(column, &width)| { .filter_map(|(column, &width)| {
data_row.to_cell(column.inner(), width) if width > 0 {
data_row.to_cell(column.inner(), width)
} else {
None
}
}), }),
); );

View file

@ -22,9 +22,6 @@ pub struct GraphData<'a> {
} }
pub struct TimeGraph<'a> { pub struct TimeGraph<'a> {
/// Whether to use a dot marker over the default braille markers.
pub use_dot: bool,
/// The min and max x boundaries. Expects a f64 representing the time range in milliseconds. /// The min and max x boundaries. Expects a f64 representing the time range in milliseconds.
pub x_bounds: [u64; 2], pub x_bounds: [u64; 2],
@ -54,6 +51,10 @@ pub struct TimeGraph<'a> {
/// Any legend constraints. /// Any legend constraints.
pub legend_constraints: Option<(Constraint, Constraint)>, pub legend_constraints: Option<(Constraint, Constraint)>,
/// The marker type. Unlike tui-rs' native charts, we assume
/// only a single type of market.
pub marker: Marker,
} }
impl<'a> TimeGraph<'a> { impl<'a> TimeGraph<'a> {
@ -131,18 +132,7 @@ impl<'a> TimeGraph<'a> {
// This is some ugly manual loop unswitching. Maybe unnecessary. // This is some ugly manual loop unswitching. Maybe unnecessary.
// TODO: Optimize this step. Cut out unneeded points. // TODO: Optimize this step. Cut out unneeded points.
let data = if self.use_dot { let data = graph_data.iter().map(create_dataset).collect();
graph_data
.iter()
.map(|data| create_dataset(data, Marker::Dot))
.collect()
} else {
graph_data
.iter()
.map(|data| create_dataset(data, Marker::Braille))
.collect()
};
let block = Block::default() let block = Block::default()
.title(self.generate_title(draw_loc)) .title(self.generate_title(draw_loc))
.borders(Borders::ALL) .borders(Borders::ALL)
@ -164,7 +154,7 @@ impl<'a> TimeGraph<'a> {
} }
/// Creates a new [`Dataset`]. /// Creates a new [`Dataset`].
fn create_dataset<'a>(data: &'a GraphData<'a>, marker: Marker) -> Dataset<'a> { fn create_dataset<'a>(data: &'a GraphData<'a>) -> Dataset<'a> {
let GraphData { let GraphData {
points, points,
style, style,
@ -174,8 +164,7 @@ fn create_dataset<'a>(data: &'a GraphData<'a>, marker: Marker) -> Dataset<'a> {
let dataset = Dataset::default() let dataset = Dataset::default()
.style(*style) .style(*style)
.data(points) .data(points)
.graph_type(GraphType::Line) .graph_type(GraphType::Line);
.marker(marker);
if let Some(name) = name { if let Some(name) = name {
dataset.name(name.as_ref()) dataset.name(name.as_ref())
@ -191,6 +180,7 @@ mod test {
use tui::{ use tui::{
layout::Rect, layout::Rect,
style::{Color, Style}, style::{Color, Style},
symbols::Marker,
text::{Span, Spans}, text::{Span, Spans},
}; };
@ -206,7 +196,6 @@ mod test {
fn create_time_graph() -> TimeGraph<'static> { fn create_time_graph() -> TimeGraph<'static> {
TimeGraph { TimeGraph {
title: " Network ".into(), title: " Network ".into(),
use_dot: true,
x_bounds: [0, 15000], x_bounds: [0, 15000],
hide_x_labels: false, hide_x_labels: false,
y_bounds: [0.0, 100.5], y_bounds: [0.0, 100.5],
@ -216,6 +205,7 @@ mod test {
is_expanded: false, is_expanded: false,
title_style: Style::default().fg(Color::Cyan), title_style: Style::default().fg(Color::Cyan),
legend_constraints: None, legend_constraints: None,
marker: Marker::Braille,
} }
} }

View file

@ -4,7 +4,7 @@ use tui::{
buffer::Buffer, buffer::Buffer,
layout::{Constraint, Rect}, layout::{Constraint, Rect},
style::{Color, Style}, style::{Color, Style},
symbols, symbols::{self, Marker},
text::{Span, Spans}, text::{Span, Spans},
widgets::{ widgets::{
canvas::{Canvas, Line, Points}, canvas::{Canvas, Line, Points},
@ -75,8 +75,6 @@ pub struct Dataset<'a> {
name: Cow<'a, str>, name: Cow<'a, str>,
/// A reference to the actual data /// A reference to the actual data
data: &'a [Point], data: &'a [Point],
/// Symbol used for each points of this dataset
marker: symbols::Marker,
/// Determines graph type used for drawing points /// Determines graph type used for drawing points
graph_type: GraphType, graph_type: GraphType,
/// Style used to plot this dataset /// Style used to plot this dataset
@ -88,7 +86,6 @@ impl<'a> Default for Dataset<'a> {
Dataset { Dataset {
name: Cow::from(""), name: Cow::from(""),
data: &[], data: &[],
marker: symbols::Marker::Dot,
graph_type: GraphType::Scatter, graph_type: GraphType::Scatter,
style: Style::default(), style: Style::default(),
} }
@ -110,11 +107,6 @@ impl<'a> Dataset<'a> {
self self
} }
pub fn marker(mut self, marker: symbols::Marker) -> Dataset<'a> {
self.marker = marker;
self
}
pub fn graph_type(mut self, graph_type: GraphType) -> Dataset<'a> { pub fn graph_type(mut self, graph_type: GraphType) -> Dataset<'a> {
self.graph_type = graph_type; self.graph_type = graph_type;
self self
@ -173,10 +165,12 @@ pub struct TimeChart<'a> {
legend_style: Style, legend_style: Style,
/// Constraints used to determine whether the legend should be shown or not /// Constraints used to determine whether the legend should be shown or not
hidden_legend_constraints: (Constraint, Constraint), hidden_legend_constraints: (Constraint, Constraint),
/// The marker type.
marker: Marker,
} }
pub const DEFAULT_LEGEND_CONSTRAINTS: (Constraint, Constraint) = pub const DEFAULT_LEGEND_CONSTRAINTS: (Constraint, Constraint) =
(Constraint::Ratio(1, 4), Constraint::Ratio(1, 4)); (Constraint::Ratio(1, 4), Constraint::Length(4));
#[allow(dead_code)] #[allow(dead_code)]
impl<'a> TimeChart<'a> { impl<'a> TimeChart<'a> {
@ -192,6 +186,7 @@ impl<'a> TimeChart<'a> {
legend_style: Default::default(), legend_style: Default::default(),
datasets, datasets,
hidden_legend_constraints: DEFAULT_LEGEND_CONSTRAINTS, hidden_legend_constraints: DEFAULT_LEGEND_CONSTRAINTS,
marker: Marker::Braille,
} }
} }
@ -220,6 +215,11 @@ impl<'a> TimeChart<'a> {
self self
} }
pub fn marker(mut self, marker: Marker) -> TimeChart<'a> {
self.marker = marker;
self
}
/// Set the constraints used to determine whether the legend should be shown or not. /// Set the constraints used to determine whether the legend should be shown or not.
pub fn hidden_legend_constraints( pub fn hidden_legend_constraints(
mut self, constraints: (Constraint, Constraint), mut self, constraints: (Constraint, Constraint),
@ -422,14 +422,14 @@ impl<'a> Widget for TimeChart<'a> {
} }
} }
// Probably better to move the dataset inside. Canvas::default()
for dataset in &self.datasets { .background_color(self.style.bg.unwrap_or(Color::Reset))
Canvas::default() .x_bounds(self.x_axis.bounds)
.background_color(self.style.bg.unwrap_or(Color::Reset)) .y_bounds(self.y_axis.bounds)
.x_bounds(self.x_axis.bounds) .paint(|ctx| {
.y_bounds(self.y_axis.bounds) for dataset in &self.datasets {
.marker(dataset.marker) let color = dataset.style.fg.unwrap_or(Color::Reset);
.paint(|ctx| {
let start_bound = self.x_axis.bounds[0]; let start_bound = self.x_axis.bounds[0];
let end_bound = self.x_axis.bounds[1]; let end_bound = self.x_axis.bounds[1];
@ -438,11 +438,6 @@ impl<'a> Widget for TimeChart<'a> {
let data_slice = &dataset.data[start_index..end_index]; let data_slice = &dataset.data[start_index..end_index];
ctx.draw(&Points {
coords: data_slice,
color: dataset.style.fg.unwrap_or(Color::Reset),
});
if let Some(interpolate_start) = interpolate_start { if let Some(interpolate_start) = interpolate_start {
if let (Some(older_point), Some(newer_point)) = ( if let (Some(older_point), Some(newer_point)) = (
dataset.data.get(interpolate_start), dataset.data.get(interpolate_start),
@ -453,18 +448,18 @@ impl<'a> Widget for TimeChart<'a> {
interpolate_point(older_point, newer_point, self.x_axis.bounds[0]), interpolate_point(older_point, newer_point, self.x_axis.bounds[0]),
); );
ctx.draw(&Points {
coords: &[interpolated_point],
color: dataset.style.fg.unwrap_or(Color::Reset),
});
if let GraphType::Line = dataset.graph_type { if let GraphType::Line = dataset.graph_type {
ctx.draw(&Line { ctx.draw(&Line {
x1: interpolated_point.0, x1: interpolated_point.0,
y1: interpolated_point.1, y1: interpolated_point.1,
x2: newer_point.0, x2: newer_point.0,
y2: newer_point.1, y2: newer_point.1,
color: dataset.style.fg.unwrap_or(Color::Reset), color,
});
} else {
ctx.draw(&Points {
coords: &[interpolated_point],
color,
}); });
} }
} }
@ -477,9 +472,14 @@ impl<'a> Widget for TimeChart<'a> {
y1: data[0].1, y1: data[0].1,
x2: data[1].0, x2: data[1].0,
y2: data[1].1, y2: data[1].1,
color: dataset.style.fg.unwrap_or(Color::Reset), color,
}); });
} }
} else {
ctx.draw(&Points {
coords: data_slice,
color,
});
} }
if let Some(interpolate_end) = interpolate_end { if let Some(interpolate_end) = interpolate_end {
@ -492,25 +492,25 @@ impl<'a> Widget for TimeChart<'a> {
interpolate_point(older_point, newer_point, self.x_axis.bounds[1]), interpolate_point(older_point, newer_point, self.x_axis.bounds[1]),
); );
ctx.draw(&Points {
coords: &[interpolated_point],
color: dataset.style.fg.unwrap_or(Color::Reset),
});
if let GraphType::Line = dataset.graph_type { if let GraphType::Line = dataset.graph_type {
ctx.draw(&Line { ctx.draw(&Line {
x1: older_point.0, x1: older_point.0,
y1: older_point.1, y1: older_point.1,
x2: interpolated_point.0, x2: interpolated_point.0,
y2: interpolated_point.1, y2: interpolated_point.1,
color: dataset.style.fg.unwrap_or(Color::Reset), color,
});
} else {
ctx.draw(&Points {
coords: &[interpolated_point],
color,
}); });
} }
} }
} }
}) }
.render(graph_area, buf); })
} .render(graph_area, buf);
if let Some(legend_area) = layout.legend_area { if let Some(legend_area) = layout.legend_area {
buf.set_style(legend_area, original_style); buf.set_style(legend_area, original_style);

View file

@ -522,6 +522,8 @@ pub fn create_collection_thread(
} }
} }
} }
// TODO: [OPT] this feels like it might not be totally optimal. Hm.
futures::executor::block_on(data_state.update_data()); futures::executor::block_on(data_state.update_data());
// Yet another check to bail if needed... // Yet another check to bail if needed...

View file

@ -1,7 +1,6 @@
use std::cmp::Ordering; use std::cmp::Ordering;
use concat_string::concat_string; use tui::text::{Span, Spans, Text};
use tui::text::Text;
use unicode_segmentation::UnicodeSegmentation; use unicode_segmentation::UnicodeSegmentation;
pub const KILO_LIMIT: u64 = 1000; pub const KILO_LIMIT: u64 = 1000;
@ -99,14 +98,37 @@ pub fn get_decimal_prefix(quantity: u64, unit: &str) -> (f64, String) {
/// Truncates text if it is too long, and adds an ellipsis at the end if needed. /// Truncates text if it is too long, and adds an ellipsis at the end if needed.
pub fn truncate_text<'a, U: Into<usize>>(content: &str, width: U) -> Text<'a> { pub fn truncate_text<'a, U: Into<usize>>(content: &str, width: U) -> Text<'a> {
let width = width.into(); let width = width.into();
let graphemes: Vec<&str> = UnicodeSegmentation::graphemes(content, true).collect(); let mut graphemes = UnicodeSegmentation::graphemes(content, true);
let grapheme_len = {
let (_, upper) = graphemes.size_hint();
match upper {
Some(upper) => upper,
None => graphemes.clone().count(), // Don't think this ever fires.
}
};
if graphemes.len() > width && width > 0 { let text = if grapheme_len > width {
// Truncate with ellipsis let mut text = String::with_capacity(width);
let first_n = graphemes[..(width - 1)].concat(); // Truncate with ellipsis.
Text::raw(concat_string!(first_n, ""))
// Use a hack to reduce the size to size `width`. Think of it like removing
// The last `grapheme_len - width` graphemes, which reduces the length to
// `width` long.
//
// This is a way to get around the currently experimental`advance_back_by`.
graphemes.nth_back(grapheme_len - width);
text.push_str(graphemes.as_str());
text.push('…');
text
} else { } else {
Text::raw(content.to_string()) content.to_string()
};
// TODO: [OPT] maybe add interning here?
Text {
lines: vec![Spans(vec![Span::raw(text)])],
} }
} }
@ -156,4 +178,9 @@ mod test {
y.sort_by(|a, b| sort_partial_fn(true)(a, b)); y.sort_by(|a, b| sort_partial_fn(true)(a, b));
assert_eq!(y, vec![16.15, 15.0, 1.0, -1.0, -100.0, -100.0, -100.1]); assert_eq!(y, vec![16.15, 15.0, 1.0, -1.0, -100.0, -100.0, -100.1]);
} }
#[test]
fn test_truncation() {
// TODO: Add tests for `truncate_text`
}
} }

View file

@ -221,6 +221,7 @@ impl ProcWidgetData {
impl DataToCell<ProcColumn> for ProcWidgetData { impl DataToCell<ProcColumn> for ProcWidgetData {
fn to_cell<'a>(&'a self, column: &ProcColumn, calculated_width: u16) -> Option<Text<'a>> { fn to_cell<'a>(&'a self, column: &ProcColumn, calculated_width: u16) -> Option<Text<'a>> {
// TODO: Optimize the string allocations here...
Some(truncate_text( Some(truncate_text(
&match column { &match column {
ProcColumn::CpuPercent => { ProcColumn::CpuPercent => {