refactor: unify on using bytes for the memory unit when harvesting (#1077)

* refactor: unify on using bytes for the memory unit when harvesting

* some ordering stuff that doesn't mean much

* some comments

* more fixes

* refactor: rename

* comments v2

* some more cleanup

* remove uninlined_format_args allow

src/app/data_harvester.rs

@@ -100,7 +100,6 @@ impl Data {
 pub struct DataCollector {
     pub data: Data,
     sys: System,
-    mem_total_kb: u64,
     temperature_type: TemperatureType,
     use_current_cpu_total: bool,
     unnormalized_cpu: bool,
@@ -138,7 +137,6 @@ impl DataCollector {
             prev_idle: 0_f64,
             #[cfg(target_os = "linux")]
             prev_non_idle: 0_f64,
-            mem_total_kb: 0,
             temperature_type: TemperatureType::Celsius,
             use_current_cpu_total: false,
             unnormalized_cpu: false,
@@ -222,7 +220,6 @@ impl DataCollector {
 
         if self.widgets_to_harvest.use_mem || self.widgets_to_harvest.use_proc {
             self.sys.refresh_memory();
-            self.mem_total_kb = self.sys.total_memory() / 1024; // FIXME: This is sorta not really correct atm due to units, fix in future PR.
         }
 
         if self.widgets_to_harvest.use_net {
@@ -258,12 +255,12 @@ impl DataCollector {
         let current_instant = Instant::now();
 
         self.update_cpu_usage();
-        self.update_temps();
         self.update_memory_usage();
         self.update_processes(
             #[cfg(target_os = "linux")]
             current_instant,
         );
+        self.update_temps();
         self.update_network_usage(current_instant);
 
         #[cfg(feature = "battery")]
@@ -307,6 +304,12 @@ impl DataCollector {
     fn update_processes(&mut self, #[cfg(target_os = "linux")] current_instant: Instant) {
         if self.widgets_to_harvest.use_proc {
             if let Ok(mut process_list) = {
+                let total_memory = if let Some(memory) = &self.data.memory {
+                    memory.total_bytes
+                } else {
+                    self.sys.total_memory()
+                };
+
                 #[cfg(target_os = "linux")]
                 {
                     use self::processes::{PrevProc, ProcHarvestOptions};
@@ -331,7 +334,7 @@ impl DataCollector {
                         &mut self.pid_mapping,
                         proc_harvest_options,
                         time_diff,
-                        self.mem_total_kb,
+                        total_memory,
                         &mut self.user_table,
                     )
                 }
@@ -343,7 +346,7 @@ impl DataCollector {
                             &self.sys,
                             self.use_current_cpu_total,
                             self.unnormalized_cpu,
-                            self.mem_total_kb * 1024,
+                            total_memory,
                             &mut self.user_table,
                         )
                     }
@@ -353,7 +356,7 @@ impl DataCollector {
                             &self.sys,
                             self.use_current_cpu_total,
                             self.unnormalized_cpu,
-                            self.mem_total_kb * 1024,
+                            total_memory,
                         )
                     }
                 }

src/app/data_harvester/memory.rs

@@ -21,7 +21,7 @@ pub mod arc;
 
 #[derive(Debug, Clone, Default)]
 pub struct MemHarvest {
-    pub total_kib: u64,
+    pub used_bytes: u64,
-    pub used_kib: u64,
+    pub total_bytes: u64,
-    pub use_percent: Option<f64>,
+    pub use_percent: Option<f64>, // TODO: Might be find to just make this an f64, and any consumer checks NaN.
 }

src/app/data_harvester/memory/arc.rs

@@ -37,7 +37,7 @@ pub(crate) fn get_arc_usage() -> Option<MemHarvest> {
                             }
                         }
                     }
-                    (mem_total / 1024, mem_arc / 1024)
+                    (mem_total, mem_arc)
                 } else {
                     (0, 0)
                 }
@@ -50,7 +50,7 @@ pub(crate) fn get_arc_usage() -> Option<MemHarvest> {
                     if let (Ok(sysctl::CtlValue::U64(arc)), Ok(sysctl::CtlValue::Ulong(mem))) =
                         (mem_arc_value.value(), mem_sys_value.value())
                     {
-                        (mem / 1024, arc / 1024)
+                        (mem, arc)
                     } else {
                         (0, 0)
                     }
@@ -64,8 +64,8 @@ pub(crate) fn get_arc_usage() -> Option<MemHarvest> {
     };
 
     Some(MemHarvest {
-        total_kib: mem_total_in_kib,
+        total_bytes: mem_total_in_kib,
-        used_kib: mem_used_in_kib,
+        used_bytes: mem_used_in_kib,
         use_percent: if mem_total_in_kib == 0 {
             None
         } else {

src/app/data_harvester/memory/gpu.rs

@@ -22,17 +22,15 @@ fn get_nvidia_mem_usage() -> Option<Vec<(String, MemHarvest)>> {
                 if let Ok(device) = nvml.device_by_index(i) {
                     if let (Ok(name), Ok(mem)) = (device.name(), device.memory_info()) {
                         // add device memory in bytes
-                        let mem_total_in_kib = mem.total / 1024;
-                        let mem_used_in_kib = mem.used / 1024;
                         results.push((
                             name,
                             MemHarvest {
-                                total_kib: mem_total_in_kib,
+                                total_bytes: mem.total,
-                                used_kib: mem_used_in_kib,
+                                used_bytes: mem.used,
-                                use_percent: if mem_total_in_kib == 0 {
+                                use_percent: if mem.total == 0 {
                                     None
                                 } else {
-                                    Some(mem_used_in_kib as f64 / mem_total_in_kib as f64 * 100.0)
+                                    Some(mem.used as f64 / mem.total as f64 * 100.0)
                                 },
                             },
                         ));

src/app/data_harvester/memory/sysinfo.rs

@@ -6,16 +6,16 @@ use crate::data_harvester::memory::MemHarvest;
 
 /// Returns RAM usage.
 pub(crate) fn get_ram_usage(sys: &System) -> Option<MemHarvest> {
-    let mem_used_in_kib = sys.used_memory() / 1024;
+    let mem_used = sys.used_memory();
-    let mem_total_in_kib = sys.total_memory() / 1024;
+    let mem_total = sys.total_memory();
 
     Some(MemHarvest {
-        total_kib: mem_total_in_kib,
+        used_bytes: mem_used,
-        used_kib: mem_used_in_kib,
+        total_bytes: mem_total,
-        use_percent: if mem_total_in_kib == 0 {
+        use_percent: if mem_total == 0 {
             None
         } else {
-            Some(mem_used_in_kib as f64 / mem_total_in_kib as f64 * 100.0)
+            Some(mem_used as f64 / mem_total as f64 * 100.0)
         },
     })
 }
@@ -23,16 +23,16 @@ pub(crate) fn get_ram_usage(sys: &System) -> Option<MemHarvest> {
 /// Returns SWAP usage.
 #[cfg(not(target_os = "windows"))]
 pub(crate) fn get_swap_usage(sys: &System) -> Option<MemHarvest> {
-    let mem_used_in_kib = sys.used_swap() / 1024;
+    let mem_used = sys.used_swap();
-    let mem_total_in_kib = sys.total_swap() / 1024;
+    let mem_total = sys.total_swap();
 
     Some(MemHarvest {
-        total_kib: mem_total_in_kib,
+        used_bytes: mem_used,
-        used_kib: mem_used_in_kib,
+        total_bytes: mem_total,
-        use_percent: if mem_total_in_kib == 0 {
+        use_percent: if mem_total == 0 {
             None
         } else {
-            Some(mem_used_in_kib as f64 / mem_total_in_kib as f64 * 100.0)
+            Some(mem_used as f64 / mem_total as f64 * 100.0)
         },
     })
 }

src/app/data_harvester/memory/windows.rs

@@ -21,8 +21,8 @@ pub(crate) fn get_swap_usage() -> Option<MemHarvest> {
             let swap_used = perf_info.PageSize.saturating_mul(perf_info.CommitTotal) as u64;
 
             Some(MemHarvest {
-                total_kib: swap_total / 1024,
+                used_bytes: swap_used,
-                used_kib: swap_used / 1024,
+                total_bytes: swap_total,
                 use_percent: Some(swap_used as f64 / swap_total as f64 * 100.0),
             })
         } else {

src/app/data_harvester/processes/freebsd.rs

@@ -25,14 +25,14 @@ struct ProcessRow {
 }
 
 pub fn get_process_data(
-    sys: &System, use_current_cpu_total: bool, unnormalized_cpu: bool, mem_total: u64,
+    sys: &System, use_current_cpu_total: bool, unnormalized_cpu: bool, total_memory: u64,
     user_table: &mut UserTable,
 ) -> crate::utils::error::Result<Vec<ProcessHarvest>> {
     super::macos_freebsd::get_process_data(
         sys,
         use_current_cpu_total,
         unnormalized_cpu,
-        mem_total,
+        total_memory,
         user_table,
         get_freebsd_process_cpu_usage,
     )

src/app/data_harvester/processes/linux.rs

@@ -116,7 +116,7 @@ fn get_linux_cpu_usage(
 
 fn read_proc(
     prev_proc: &PrevProcDetails, process: &Process, cpu_usage: f64, cpu_fraction: f64,
-    use_current_cpu_total: bool, time_difference_in_secs: u64, mem_total_kb: u64,
+    use_current_cpu_total: bool, time_difference_in_secs: u64, total_memory: u64,
     user_table: &mut UserTable,
 ) -> error::Result<(ProcessHarvest, u64)> {
     let stat = process.stat()?;
@@ -164,8 +164,7 @@ fn read_proc(
     );
     let parent_pid = Some(stat.ppid);
     let mem_usage_bytes = stat.rss_bytes();
-    let mem_usage_kb = mem_usage_bytes / 1024;
+    let mem_usage_percent = mem_usage_bytes as f64 / total_memory as f64 * 100.0;
-    let mem_usage_percent = mem_usage_kb as f64 / mem_total_kb as f64 * 100.0;
 
     // This can fail if permission is denied!
     let (total_read_bytes, total_write_bytes, read_bytes_per_sec, write_bytes_per_sec) =
@@ -233,7 +232,7 @@ pub(crate) struct ProcHarvestOptions {
 
 pub(crate) fn get_process_data(
     sys: &System, prev_proc: PrevProc<'_>, pid_mapping: &mut FxHashMap<Pid, PrevProcDetails>,
-    proc_harvest_options: ProcHarvestOptions, time_difference_in_secs: u64, mem_total_kb: u64,
+    proc_harvest_options: ProcHarvestOptions, time_difference_in_secs: u64, total_memory: u64,
     user_table: &mut UserTable,
 ) -> crate::utils::error::Result<Vec<ProcessHarvest>> {
     let ProcHarvestOptions {
@@ -280,7 +279,7 @@ pub(crate) fn get_process_data(
                             cpu_fraction,
                             use_current_cpu_total,
                             time_difference_in_secs,
-                            mem_total_kb,
+                            total_memory,
                             user_table,
                         ) {
                             prev_proc_details.cpu_time = new_process_times;

src/app/data_harvester/processes/macos_freebsd.rs

@@ -9,7 +9,7 @@ use super::ProcessHarvest;
 use crate::{data_harvester::processes::UserTable, utils::error::Result, Pid};
 
 pub fn get_process_data<F>(
-    sys: &System, use_current_cpu_total: bool, unnormalized_cpu: bool, mem_total: u64,
+    sys: &System, use_current_cpu_total: bool, unnormalized_cpu: bool, total_memory: u64,
     user_table: &mut UserTable, backup_cpu_proc_usage: F,
 ) -> Result<Vec<ProcessHarvest>>
 where
@@ -88,8 +88,8 @@ where
             },
             name,
             command,
-            mem_usage_percent: if mem_total > 0 {
+            mem_usage_percent: if total_memory > 0 {
-                process_val.memory() as f64 * 100.0 / mem_total as f64
+                process_val.memory() as f64 * 100.0 / total_memory as f64
             } else {
                 0.0
             },

src/app/data_harvester/processes/windows.rs

@@ -5,7 +5,7 @@ use sysinfo::{CpuExt, PidExt, ProcessExt, System, SystemExt, UserExt};
 use super::ProcessHarvest;
 
 pub fn get_process_data(
-    sys: &System, use_current_cpu_total: bool, unnormalized_cpu: bool, mem_total: u64,
+    sys: &System, use_current_cpu_total: bool, unnormalized_cpu: bool, total_memory: u64,
 ) -> crate::utils::error::Result<Vec<ProcessHarvest>> {
     let mut process_vector: Vec<ProcessHarvest> = Vec::new();
     let process_hashmap = sys.processes();
@@ -63,8 +63,8 @@ pub fn get_process_data(
             parent_pid: process_val.parent().map(|p| p.as_u32() as _),
             name,
             command,
-            mem_usage_percent: if mem_total > 0 {
+            mem_usage_percent: if total_memory > 0 {
-                process_val.memory() as f64 * 100.0 / mem_total as f64
+                process_val.memory() as f64 * 100.0 / total_memory as f64
             } else {
                 0.0
             },

src/bin/main.rs

@@ -1,5 +1,4 @@
 #![warn(rust_2018_idioms)]
-#![allow(clippy::uninlined_format_args)]
 #![deny(clippy::missing_safety_doc)]
 #[allow(unused_imports)]
 #[cfg(feature = "log")]

src/data_conversion.rs

@@ -236,67 +236,65 @@ pub fn convert_swap_data_points(current_data: &DataCollection) -> Vec<Point> {
     result
 }
 
+/// Returns the most appropriate binary prefix unit type (e.g. kibibyte) and denominator for the given amount of bytes.
+///
+/// The expected usage is to divide out the given value with the returned denominator in order to be able to use it
+/// with the returned binary unit (e.g. divide 3000 bytes by 1024 to have a value in KiB).
+fn get_mem_binary_unit_and_denominator(bytes: u64) -> (&'static str, f64) {
+    if bytes < KIBI_LIMIT {
+        // Stick with bytes if under a kibibyte.
+        ("B", 1.0)
+    } else if bytes < MEBI_LIMIT {
+        ("KiB", KIBI_LIMIT_F64)
+    } else if bytes < GIBI_LIMIT {
+        ("MiB", MEBI_LIMIT_F64)
+    } else if bytes < TEBI_LIMIT {
+        ("GiB", GIBI_LIMIT_F64)
+    } else {
+        // Otherwise just use tebibytes, which is probably safe for most use cases.
+        ("TiB", TEBI_LIMIT_F64)
+    }
+}
+
 pub fn convert_mem_labels(
     current_data: &DataCollection,
 ) -> (Option<(String, String)>, Option<(String, String)>) {
-    /// Returns the unit type and denominator for given total amount of memory in kibibytes.
-    fn return_unit_and_denominator_for_mem_kib(mem_total_kib: u64) -> (&'static str, f64) {
-        if mem_total_kib < 1024 {
-            // Stay with KiB
-            ("KiB", 1.0)
-        } else if mem_total_kib < MEBI_LIMIT {
-            // Use MiB
-            ("MiB", KIBI_LIMIT_F64)
-        } else if mem_total_kib < GIBI_LIMIT {
-            // Use GiB
-            ("GiB", MEBI_LIMIT_F64)
-        } else {
-            // Use TiB
-            ("TiB", GIBI_LIMIT_F64)
-        }
-    }
-
     (
-        if current_data.memory_harvest.total_kib > 0 {
+        if current_data.memory_harvest.total_bytes > 0 {
             Some((
                 format!(
                     "{:3.0}%",
                     current_data.memory_harvest.use_percent.unwrap_or(0.0)
                 ),
                 {
-                    let (unit, denominator) = return_unit_and_denominator_for_mem_kib(
+                    let (unit, denominator) = get_mem_binary_unit_and_denominator(
-                        current_data.memory_harvest.total_kib,
+                        current_data.memory_harvest.total_bytes,
                     );
 
                     format!(
-                        "   {:.1}{}/{:.1}{}",
+                        "   {:.1}{unit}/{:.1}{unit}",
-                        current_data.memory_harvest.used_kib as f64 / denominator,
+                        current_data.memory_harvest.used_bytes as f64 / denominator,
-                        unit,
+                        (current_data.memory_harvest.total_bytes as f64 / denominator),
-                        (current_data.memory_harvest.total_kib as f64 / denominator),
-                        unit
                     )
                 },
             ))
         } else {
             None
         },
-        if current_data.swap_harvest.total_kib > 0 {
+        if current_data.swap_harvest.total_bytes > 0 {
             Some((
                 format!(
                     "{:3.0}%",
                     current_data.swap_harvest.use_percent.unwrap_or(0.0)
                 ),
                 {
-                    let (unit, denominator) = return_unit_and_denominator_for_mem_kib(
+                    let (unit, denominator) =
-                        current_data.swap_harvest.total_kib,
+                        get_mem_binary_unit_and_denominator(current_data.swap_harvest.total_bytes);
-                    );
 
                     format!(
-                        "   {:.1}{}/{:.1}{}",
+                        "   {:.1}{unit}/{:.1}{unit}",
-                        current_data.swap_harvest.used_kib as f64 / denominator,
+                        current_data.swap_harvest.used_bytes as f64 / denominator,
-                        unit,
+                        (current_data.swap_harvest.total_bytes as f64 / denominator),
-                        (current_data.swap_harvest.total_kib as f64 / denominator),
-                        unit
                     )
                 },
             ))
@@ -550,24 +548,7 @@ pub fn convert_battery_harvest(current_data: &DataCollection) -> Vec<ConvertedBa
 pub fn convert_arc_labels(
     current_data: &crate::app::data_farmer::DataCollection,
 ) -> Option<(String, String)> {
-    /// Returns the unit type and denominator for given total amount of memory in kibibytes.
+    if current_data.arc_harvest.total_bytes > 0 {
-    fn return_unit_and_denominator_for_mem_kib(mem_total_kib: u64) -> (&'static str, f64) {
-        if mem_total_kib < 1024 {
-            // Stay with KiB
-            ("KiB", 1.0)
-        } else if mem_total_kib < MEBI_LIMIT {
-            // Use MiB
-            ("MiB", KIBI_LIMIT_F64)
-        } else if mem_total_kib < GIBI_LIMIT {
-            // Use GiB
-            ("GiB", MEBI_LIMIT_F64)
-        } else {
-            // Use TiB
-            ("TiB", GIBI_LIMIT_F64)
-        }
-    }
-
-    if current_data.arc_harvest.total_kib > 0 {
         Some((
             format!(
                 "{:3.0}%",
@@ -575,14 +556,12 @@ pub fn convert_arc_labels(
             ),
             {
                 let (unit, denominator) =
-                    return_unit_and_denominator_for_mem_kib(current_data.arc_harvest.total_kib);
+                    get_mem_binary_unit_and_denominator(current_data.arc_harvest.total_bytes);
 
                 format!(
-                    "   {:.1}{}/{:.1}{}",
+                    "   {:.1}{unit}/{:.1}{unit}",
-                    current_data.arc_harvest.used_kib as f64 / denominator,
+                    current_data.arc_harvest.used_bytes as f64 / denominator,
-                    unit,
+                    (current_data.arc_harvest.total_bytes as f64 / denominator),
-                    (current_data.arc_harvest.total_kib as f64 / denominator),
-                    unit
                 )
             },
         ))
@@ -625,23 +604,6 @@ pub struct ConvertedGpuData {
 pub fn convert_gpu_data(
     current_data: &crate::app::data_farmer::DataCollection,
 ) -> Option<Vec<ConvertedGpuData>> {
-    /// Returns the unit type and denominator for given total amount of memory in kibibytes.
-    fn return_unit_and_denominator_for_mem_kib(mem_total_kib: u64) -> (&'static str, f64) {
-        if mem_total_kib < 1024 {
-            // Stay with KiB
-            ("KiB", 1.0)
-        } else if mem_total_kib < MEBI_LIMIT {
-            // Use MiB
-            ("MiB", KIBI_LIMIT_F64)
-        } else if mem_total_kib < GIBI_LIMIT {
-            // Use GiB
-            ("GiB", MEBI_LIMIT_F64)
-        } else {
-            // Use TiB
-            ("TiB", GIBI_LIMIT_F64)
-        }
-    }
-
     let current_time = current_data.current_instant;
 
     // convert points
@@ -682,14 +644,12 @@ pub fn convert_gpu_data(
                 mem_percent: format!("{:3.0}%", gpu.1.use_percent.unwrap_or(0.0)),
                 mem_total: {
                     let (unit, denominator) =
-                        return_unit_and_denominator_for_mem_kib(gpu.1.total_kib);
+                        get_mem_binary_unit_and_denominator(gpu.1.total_bytes);
 
                     format!(
-                        "   {:.1}{}/{:.1}{}",
+                        "   {:.1}{unit}/{:.1}{unit}",
-                        gpu.1.used_kib as f64 / denominator,
+                        gpu.1.used_bytes as f64 / denominator,
-                        unit,
+                        (gpu.1.total_bytes as f64 / denominator),
-                        (gpu.1.total_kib as f64 / denominator),
-                        unit
                     )
                 },
             }

src/lib.rs

@@ -6,7 +6,6 @@
 //! bottom, refer to [here](https://clementtsang.github.io/bottom/stable/).
 
 #![warn(rust_2018_idioms)]
-#![allow(clippy::uninlined_format_args)]
 #![deny(clippy::missing_safety_doc)]
 #[allow(unused_imports)]
 #[cfg(feature = "log")]