fully migrate network, and fix some log bugs while we're at it

This is something I never noticed, but the log of 0 is inf - so there
were gaps in the lines when using log scaling!

CHANGELOG.md

@@ -36,13 +36,17 @@ That said, these are more guidelines rather than hardset rules, though the proje
 - [#1593](https://github.com/ClementTsang/bottom/pull/1593): Fix using `"none"` for chart legend position in configs.
 - [#1594](https://github.com/ClementTsang/bottom/pull/1594): Fix incorrect default config definitions for chart legends.
 - [#1596](https://github.com/ClementTsang/bottom/pull/1596): Fix support for nilfs2 file system.
-- [#1660](https://github.com/ClementTsang/bottom/pull/1660): Handle terminal cleanup if the program is terminated due to an `Err` bubbling to the top.
+- [#1660](https://github.com/ClementTsang/bottom/pull/1660): Fix properly cleaning up the terminal if the program is terminated due to an `Err` bubbling to the top.
+- [#1663](https://github.com/ClementTsang/bottom/pull/1663): Fix network graphs using log scaling having broken lines when a point was 0.
 
 ### Changes
 
 - [#1559](https://github.com/ClementTsang/bottom/pull/1559): Rename `--enable_gpu` to `--disable_gpu`, and make GPU features enabled by default.
-- [#1570](https://github.com/ClementTsang/bottom/pull/1570): Consider `$XDG_CONFIG_HOME` on macOS when looking for a default config path in a
-  backwards-compatible fashion.
+- [#1570](https://github.com/ClementTsang/bottom/pull/1570): Consider `$XDG_CONFIG_HOME` on macOS when looking for a default config path in a backwards-compatible fashion.
+
+### Other
+
+- [#1663](https://github.com/ClementTsang/bottom/pull/1663): Rework how data is stored internally, reducing memory usage a bit.
 
 ## [0.10.2] - 2024-08-05
 

src/app.rs

@@ -26,7 +26,7 @@ use crate::{
     widgets::{ProcWidgetColumn, ProcWidgetMode},
 };
 
-#[derive(Debug, Clone, Eq, PartialEq, Default)]
+#[derive(Debug, Clone, Eq, PartialEq, Default, Copy)]
 pub enum AxisScaling {
     #[default]
     Log,

src/canvas/components/time_graph.rs

@@ -174,14 +174,15 @@ impl TimeGraph<'_> {
                 .hidden_legend_constraints(
                     self.legend_constraints
                         .unwrap_or(DEFAULT_LEGEND_CONSTRAINTS),
-                ).scaling(self.scaling),
+                )
+                .scaling(self.scaling),
             draw_loc,
         )
     }
 }
 
 /// Creates a new [`Dataset`].
-fn create_dataset<'a>(data: GraphData<'a>) -> Dataset<'a> {
+fn create_dataset(data: GraphData<'_>) -> Dataset<'_> {
     let GraphData {
         time,
         values,

src/canvas/components/time_graph/time_chart.rs

@@ -20,7 +20,10 @@ use tui::{
 };
 use unicode_width::UnicodeWidthStr;
 
-use crate::app::data::Values;
+use crate::{
+    app::data::Values,
+    utils::general::{saturating_log10, saturating_log2},
+};
 
 pub const DEFAULT_LEGEND_CONSTRAINTS: (Constraint, Constraint) =
     (Constraint::Ratio(1, 4), Constraint::Length(4));
@@ -42,7 +45,7 @@ pub enum AxisBound {
 }
 
 impl AxisBound {
-    fn to_bounds(&self) -> [f64; 2] {
+    fn get_bounds(&self) -> [f64; 2] {
         match self {
             AxisBound::Zero => [0.0, 0.0],
             AxisBound::Min(min) => [*min, 0.0],
@@ -388,10 +391,12 @@ pub(crate) enum ChartScaling {
 impl ChartScaling {
     /// Scale a value.
     pub(super) fn scale(&self, value: f64) -> f64 {
+        // Remember to do saturating log checks as otherwise 0.0 becomes inf, and you get
+        // gaps!
         match self {
             ChartScaling::Linear => value,
-            ChartScaling::Log10 => value.log10(),
-            ChartScaling::Log2 => value.log2(),
+            ChartScaling::Log10 => saturating_log10(value),
+            ChartScaling::Log2 => saturating_log2(value),
         }
     }
 }
@@ -816,8 +821,8 @@ impl Widget for TimeChart<'_> {
             }
         }
 
-        let x_bounds = self.x_axis.bounds.to_bounds();
-        let y_bounds = self.y_axis.bounds.to_bounds();
+        let x_bounds = self.x_axis.bounds.get_bounds();
+        let y_bounds = self.y_axis.bounds.get_bounds();
 
         Canvas::default()
             .background_color(self.style.bg.unwrap_or(Color::Reset))

src/canvas/components/time_graph/time_chart/points.rs

@@ -38,23 +38,21 @@ impl TimeChart<'_> {
             };
 
             let color = dataset.style.fg.unwrap_or(Color::Reset);
-            let left_edge = self.x_axis.bounds.to_bounds()[0];
+            let left_edge = self.x_axis.bounds.get_bounds()[0];
 
             // TODO: (points_rework_v1) Can we instead modify the range so it's based on the epoch rather than having to convert?
             // TODO: (points_rework_v1) Is this efficient? Or should I prune using take_while first?
             // TODO: (points_rework_v1) Should this be generic over dataset.graph_type?
-            for (mut curr, next) in values
+            for (curr, next) in values
                 .iter_along_base(times)
                 .rev()
                 .map(|(&time, &val)| {
                     let from_start: f64 =
                         (current_time.duration_since(time).as_millis() as f64).floor();
-                    (-from_start, val)
+                    (-from_start, self.scaling.scale(val))
                 })
                 .tuple_windows()
             {
-                curr.1 = self.scaling.scale(curr.1);
-
                 if curr.0 == left_edge {
                     // The current point hits the left edge. Draw just the current point and halt.
                     ctx.draw(&Points {

src/canvas/widgets/mem_graph.rs

@@ -101,7 +101,7 @@ impl Painter {
                     &mut points,
                     "RAM",
                     Some(&data.ram_harvest),
-                    &time,
+                    time,
                     &timeseries.ram,
                     self.styles.ram_style,
                 );
@@ -110,7 +110,7 @@ impl Painter {
                     &mut points,
                     "SWP",
                     data.swap_harvest.as_ref(),
-                    &time,
+                    time,
                     &timeseries.swap,
                     self.styles.swap_style,
                 );
@@ -121,7 +121,7 @@ impl Painter {
                         &mut points,
                         "CACHE", // TODO: Figure out how to line this up better
                         data.cache_harvest.as_ref(),
-                        &time,
+                        time,
                         &timeseries.cache_mem,
                         self.styles.cache_style,
                     );
@@ -133,7 +133,7 @@ impl Painter {
                         &mut points,
                         "ARC",
                         data.arc_harvest.as_ref(),
-                        &time,
+                        time,
                         &timeseries.arc_mem,
                         self.styles.arc_style,
                     );
@@ -161,7 +161,7 @@ impl Painter {
                                 &mut points,
                                 name, // TODO: REALLY figure out how to line this up better
                                 Some(harvest),
-                                &time,
+                                time,
                                 gpu_data,
                                 style,
                             );

src/canvas/widgets/network_basic.rs

@@ -41,10 +41,9 @@ impl Painter {
         }
 
         let use_binary_prefix = app_state.app_config_fields.network_use_binary_prefix;
-        let unit_type = app_state.app_config_fields.network_unit_type;
         let network_data = &(app_state.data_store.get_data().network_harvest);
-        let rx = get_unit_prefix(network_data.rx, unit_type, use_binary_prefix);
-        let tx = get_unit_prefix(network_data.tx, unit_type, use_binary_prefix);
+        let rx = get_unit_prefix(network_data.rx, use_binary_prefix);
+        let tx = get_unit_prefix(network_data.tx, use_binary_prefix);
         let total_rx = convert_bits(network_data.total_rx, use_binary_prefix);
         let total_tx = convert_bits(network_data.total_tx, use_binary_prefix);
 

src/canvas/widgets/network_graph.rs

@@ -1,3 +1,5 @@
+use std::time::Duration;
+
 use tui::{
     layout::{Constraint, Direction, Layout, Rect},
     symbols::Marker,
@@ -13,7 +15,10 @@ use crate::{
         drawing_utils::should_hide_x_label,
         Painter,
     },
-    utils::data_units::*,
+    utils::{
+        data_units::*,
+        general::{saturating_log10, saturating_log2},
+    },
 };
 
 impl Painter {
@@ -76,11 +81,13 @@ impl Painter {
                     // For now, just do it each time. Might want to cache this later though.
 
                     let mut biggest = 0.0;
+                    let first_time = *last_time
+                        - Duration::from_millis(network_widget_state.current_display_time);
 
                     for (_, &v) in rx_points
                         .iter_along_base(time)
                         .rev()
-                        .take_while(|(&time, _)| time >= *last_time)
+                        .take_while(|(&time, _)| time >= first_time)
                     {
                         if v > biggest {
                             biggest = v;
@@ -90,7 +97,7 @@ impl Painter {
                     for (_, &v) in tx_points
                         .iter_along_base(time)
                         .rev()
-                        .take_while(|(&time, _)| time >= *last_time)
+                        .take_while(|(&time, _)| time >= first_time)
                     {
                         if v > biggest {
                             biggest = v;
@@ -120,8 +127,8 @@ impl Painter {
                 DataUnit::Bit => "b/s",
             };
 
-            let rx = get_unit_prefix(network_latest_data.rx, unit_type, use_binary_prefix);
-            let tx = get_unit_prefix(network_latest_data.tx, unit_type, use_binary_prefix);
+            let rx = get_unit_prefix(network_latest_data.rx, use_binary_prefix);
+            let tx = get_unit_prefix(network_latest_data.tx, use_binary_prefix);
             let total_rx = convert_bits(network_latest_data.total_rx, use_binary_prefix);
             let total_tx = convert_bits(network_latest_data.total_tx, use_binary_prefix);
 
@@ -223,8 +230,8 @@ impl Painter {
             DataUnit::Bit => "b/s",
         };
 
-        let rx = get_unit_prefix(network_latest_data.rx, unit_type, use_binary_prefix);
-        let tx = get_unit_prefix(network_latest_data.tx, unit_type, use_binary_prefix);
+        let rx = get_unit_prefix(network_latest_data.rx, use_binary_prefix);
+        let tx = get_unit_prefix(network_latest_data.tx, use_binary_prefix);
 
         let rx_label = format!("{:.1}{}{}", rx.0, rx.1, unit);
         let tx_label = format!("{:.1}{}{}", tx.0, tx.1, unit);
@@ -265,7 +272,7 @@ impl Painter {
     }
 }
 
-/// Returns the required max data point and labels.
+/// Returns the required labels.
 ///
 /// TODO: This is _really_ ugly... also there might be a bug with certain heights and too many labels.
 /// We may need to take draw height into account, either here, or in the time graph itself.
@@ -297,7 +304,7 @@ fn adjust_network_data_point(max_entry: f64, config: &AppConfigFields) -> (f64,
     // Now just check the largest unit we correspond to... then proceed to build
     // some entries from there!
 
-    let scale_type = &config.network_scale_type;
+    let scale_type = config.network_scale_type;
     let use_binary_prefix = config.network_use_binary_prefix;
     let network_unit_type = config.network_unit_type;
 
@@ -308,6 +315,8 @@ fn adjust_network_data_point(max_entry: f64, config: &AppConfigFields) -> (f64,
 
     match scale_type {
         AxisScaling::Linear => {
+            let max_entry = max_entry * 1.5;
+
             let (k_limit, m_limit, g_limit, t_limit) = if use_binary_prefix {
                 (
                     KIBI_LIMIT_F64,
@@ -324,23 +333,22 @@ fn adjust_network_data_point(max_entry: f64, config: &AppConfigFields) -> (f64,
                 )
             };
 
-            let bumped_max_entry = max_entry * 1.5; // We use the bumped up version to calculate our unit type.
             let (max_value_scaled, unit_prefix, unit_type): (f64, &str, &str) =
-                if bumped_max_entry < k_limit {
+                if max_entry < k_limit {
                     (max_entry, "", unit_char)
-                } else if bumped_max_entry < m_limit {
+                } else if max_entry < m_limit {
                     (
                         max_entry / k_limit,
                         if use_binary_prefix { "Ki" } else { "K" },
                         unit_char,
                     )
-                } else if bumped_max_entry < g_limit {
+                } else if max_entry < g_limit {
                     (
                         max_entry / m_limit,
                         if use_binary_prefix { "Mi" } else { "M" },
                         unit_char,
                     )
-                } else if bumped_max_entry < t_limit {
+                } else if max_entry < t_limit {
                     (
                         max_entry / g_limit,
                         if use_binary_prefix { "Gi" } else { "G" },
@@ -357,7 +365,6 @@ fn adjust_network_data_point(max_entry: f64, config: &AppConfigFields) -> (f64,
             // Finally, build an acceptable range starting from there, using the given
             // height! Note we try to put more of a weight on the bottom section
             // vs. the top, since the top has less data.
-
             let base_unit = max_value_scaled;
             let labels: Vec<String> = vec![
                 format!("0{unit_prefix}{unit_type}"),
@@ -366,11 +373,13 @@ fn adjust_network_data_point(max_entry: f64, config: &AppConfigFields) -> (f64,
                 format!("{:.1}", base_unit * 1.5),
             ]
             .into_iter()
-            .map(|s| format!("{s:>5}")) // Pull 5 as the longest legend value is generally going to be 5 digits (if they somehow
-            // hit over 5 terabits per second)
+            .map(|s| {
+                // Pull 5 as the longest legend value is generally going to be 5 digits (if they somehow hit over 5 terabits per second)
+                format!("{s:>5}")
+            })
             .collect();
 
-            (bumped_max_entry, labels)
+            (max_entry, labels)
         }
         AxisScaling::Log => {
             let (m_limit, g_limit, t_limit) = if use_binary_prefix {
@@ -379,6 +388,14 @@ fn adjust_network_data_point(max_entry: f64, config: &AppConfigFields) -> (f64,
                 (LOG_MEGA_LIMIT, LOG_GIGA_LIMIT, LOG_TERA_LIMIT)
             };
 
+            // Remember to do saturating log checks as otherwise 0.0 becomes inf, and you get
+            // gaps!
+            let max_entry = if use_binary_prefix {
+                saturating_log2(max_entry)
+            } else {
+                saturating_log10(max_entry)
+            };
+
             fn get_zero(network_use_binary_prefix: bool, unit_char: &str) -> String {
                 format!(
                     "{}0{}",

src/utils/data_units.rs

@@ -75,11 +75,8 @@ pub fn convert_bits(bits: u64, base_two: bool) -> (f64, &'static str) {
 
 /// Return a tuple containing the value and a unit string to be used as a prefix.
 #[inline]
-pub fn get_unit_prefix(value: u64, unit: DataUnit, base_two: bool) -> (f64, &'static str) {
-    let float_value = match unit {
-        DataUnit::Byte => value / 8,
-        DataUnit::Bit => value,
-    } as f64;
+pub fn get_unit_prefix(value: u64, base_two: bool) -> (f64, &'static str) {
+    let float_value = value as f64;
 
     if base_two {
         match value {

src/utils/general.rs

@@ -62,6 +62,24 @@ macro_rules! clamp_num_impl {
 
 clamp_num_impl!(u8, u16, u32, u64, usize);
 
+/// Checked log2.
+pub fn saturating_log2(value: f64) -> f64 {
+    if value > 0.0 {
+        value.log2()
+    } else {
+        0.0
+    }
+}
+
+/// Checked log10.
+pub fn saturating_log10(value: f64) -> f64 {
+    if value > 0.0 {
+        value.log10()
+    } else {
+        0.0
+    }
+}
+
 #[cfg(test)]
 mod test {
     use super::*;