From 7ee6da37766fdb2e36d9e9e11668f3fafe35463e Mon Sep 17 00:00:00 2001 From: Clement Tsang <34804052+ClementTsang@users.noreply.github.com> Date: Sun, 26 Mar 2023 01:53:43 -0400 Subject: 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 | 17 +-- src/app/data_harvester/memory.rs | 6 +- src/app/data_harvester/memory/arc.rs | 8 +- src/app/data_harvester/memory/gpu.rs | 10 +- src/app/data_harvester/memory/sysinfo.rs | 24 ++--- src/app/data_harvester/memory/windows.rs | 4 +- src/app/data_harvester/processes/freebsd.rs | 4 +- src/app/data_harvester/processes/linux.rs | 9 +- src/app/data_harvester/processes/macos_freebsd.rs | 6 +- src/app/data_harvester/processes/windows.rs | 6 +- src/bin/main.rs | 1 - src/data_conversion.rs | 122 ++++++++-------------- src/lib.rs | 1 - 13 files changed, 88 insertions(+), 130 deletions(-) diff --git a/src/app/data_harvester.rs b/src/app/data_harvester.rs index 4d8f7991..fce310e4 100644 --- a/src/app/data_harvester.rs +++ b/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, ) } } diff --git a/src/app/data_harvester/memory.rs b/src/app/data_harvester/memory.rs index 02c0fa9c..e8906284 100644 --- a/src/app/data_harvester/memory.rs +++ b/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_kib: u64, - pub use_percent: Option, + pub used_bytes: u64, + pub total_bytes: u64, + pub use_percent: Option, // TODO: Might be find to just make this an f64, and any consumer checks NaN. } diff --git a/src/app/data_harvester/memory/arc.rs b/src/app/data_harvester/memory/arc.rs index 3dc26ba8..9684c8be 100644 --- a/src/app/data_harvester/memory/arc.rs +++ b/src/app/data_harvester/memory/arc.rs @@ -37,7 +37,7 @@ pub(crate) fn get_arc_usage() -> Option { } } } - (mem_total / 1024, mem_arc / 1024) + (mem_total, mem_arc) } else { (0, 0) } @@ -50,7 +50,7 @@ pub(crate) fn get_arc_usage() -> Option { 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 { }; Some(MemHarvest { - total_kib: mem_total_in_kib, - used_kib: mem_used_in_kib, + total_bytes: mem_total_in_kib, + used_bytes: mem_used_in_kib, use_percent: if mem_total_in_kib == 0 { None } else { diff --git a/src/app/data_harvester/memory/gpu.rs b/src/app/data_harvester/memory/gpu.rs index 86c5a8b7..6fd66ba3 100644 --- a/src/app/data_harvester/memory/gpu.rs +++ b/src/app/data_harvester/memory/gpu.rs @@ -22,17 +22,15 @@ fn get_nvidia_mem_usage() -> Option> { 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, - used_kib: mem_used_in_kib, - use_percent: if mem_total_in_kib == 0 { + total_bytes: mem.total, + used_bytes: mem.used, + 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) }, }, )); diff --git a/src/app/data_harvester/memory/sysinfo.rs b/src/app/data_harvester/memory/sysinfo.rs index 4cb8bdb0..56844728 100644 --- a/src/app/data_harvester/memory/sysinfo.rs +++ b/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 { - let mem_used_in_kib = sys.used_memory() / 1024; - let mem_total_in_kib = sys.total_memory() / 1024; + let mem_used = sys.used_memory(); + let mem_total = sys.total_memory(); Some(MemHarvest { - total_kib: mem_total_in_kib, - used_kib: mem_used_in_kib, - use_percent: if mem_total_in_kib == 0 { + used_bytes: mem_used, + total_bytes: mem_total, + 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 { /// Returns SWAP usage. #[cfg(not(target_os = "windows"))] pub(crate) fn get_swap_usage(sys: &System) -> Option { - let mem_used_in_kib = sys.used_swap() / 1024; - let mem_total_in_kib = sys.total_swap() / 1024; + let mem_used = sys.used_swap(); + let mem_total = sys.total_swap(); Some(MemHarvest { - total_kib: mem_total_in_kib, - used_kib: mem_used_in_kib, - use_percent: if mem_total_in_kib == 0 { + used_bytes: mem_used, + total_bytes: mem_total, + 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) }, }) } diff --git a/src/app/data_harvester/memory/windows.rs b/src/app/data_harvester/memory/windows.rs index 5ead4b52..0b50e52b 100644 --- a/src/app/data_harvester/memory/windows.rs +++ b/src/app/data_harvester/memory/windows.rs @@ -21,8 +21,8 @@ pub(crate) fn get_swap_usage() -> Option { let swap_used = perf_info.PageSize.saturating_mul(perf_info.CommitTotal) as u64; Some(MemHarvest { - total_kib: swap_total / 1024, - used_kib: swap_used / 1024, + used_bytes: swap_used, + total_bytes: swap_total, use_percent: Some(swap_used as f64 / swap_total as f64 * 100.0), }) } else { diff --git a/src/app/data_harvester/processes/freebsd.rs b/src/app/data_harvester/processes/freebsd.rs index f724510b..715d2271 100644 --- a/src/app/data_harvester/processes/freebsd.rs +++ b/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> { super::macos_freebsd::get_process_data( sys, use_current_cpu_total, unnormalized_cpu, - mem_total, + total_memory, user_table, get_freebsd_process_cpu_usage, ) diff --git a/src/app/data_harvester/processes/linux.rs b/src/app/data_harvester/processes/linux.rs index e290f74b..4ada595f 100644 --- a/src/app/data_harvester/processes/linux.rs +++ b/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_kb as f64 / mem_total_kb as f64 * 100.0; + let mem_usage_percent = mem_usage_bytes as f64 / total_memory 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, - 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> { 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; diff --git a/src/app/data_harvester/processes/macos_freebsd.rs b/src/app/data_harvester/processes/macos_freebsd.rs index 6e0cc786..88076592 100644 --- a/src/app/data_harvester/processes/macos_freebsd.rs +++ b/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( - 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> where @@ -88,8 +88,8 @@ where }, name, command, - mem_usage_percent: if mem_total > 0 { - process_val.memory() as f64 * 100.0 / mem_total as f64 + mem_usage_percent: if total_memory > 0 { + process_val.memory() as f64 * 100.0 / total_memory as f64 } else { 0.0 }, diff --git a/src/app/data_harvester/processes/windows.rs b/src/app/data_harvester/processes/windows.rs index cbdcea5c..78f4d107 100644 --- a/src/app/data_harvester/processes/windows.rs +++ b/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> { let mut process_vector: Vec = 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 { - process_val.memory() as f64 * 100.0 / mem_total as f64 + mem_usage_percent: if total_memory > 0 { + process_val.memory() as f64 * 100.0 / total_memory as f64 } else { 0.0 }, diff --git a/src/bin/main.rs b/src/bin/main.rs index 697fc27f..0b3a9bd5 100644 --- a/src/bin/main.rs +++ b/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")] diff --git a/src/data_conversion.rs b/src/data_conversion.rs index a27663e3..56cd4d19 100644 --- a/src/data_conversion.rs +++ b/src/data_conversion.rs @@ -236,67 +236,65 @@ pub fn convert_swap_data_points(current_data: &DataCollection) -> Vec { 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( - current_data.memory_harvest.total_kib, + let (unit, denominator) = get_mem_binary_unit_and_denominator( + current_data.memory_harvest.total_bytes, ); format!( - " {:.1}{}/{:.1}{}", - current_data.memory_harvest.used_kib as f64 / denominator, - unit, - (current_data.memory_harvest.total_kib as f64 / denominator), - unit + " {:.1}{unit}/{:.1}{unit}", + current_data.memory_harvest.used_bytes as f64 / denominator, + (current_data.memory_harvest.total_bytes as f64 / denominator), ) }, )) } 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( - current_data.swap_harvest.total_kib, - ); + let (unit, denominator) = + get_mem_binary_unit_and_denominator(current_data.swap_harvest.total_bytes); format!( - " {:.1}{}/{:.1}{}", - current_data.swap_harvest.used_kib as f64 / denominator, - unit, - (current_data.swap_harvest.total_kib as f64 / denominator), - unit + " {:.1}{unit}/{:.1}{unit}", + current_data.swap_harvest.used_bytes as f64 / denominator, + (current_data.swap_harvest.total_bytes as f64 / denominator), ) }, )) @@ -550,24 +548,7 @@ pub fn convert_battery_harvest(current_data: &DataCollection) -> Vec 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.arc_harvest.total_kib > 0 { + if current_data.arc_harvest.total_bytes > 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}{}", - current_data.arc_harvest.used_kib as f64 / denominator, - unit, - (current_data.arc_harvest.total_kib as f64 / denominator), - unit + " {:.1}{unit}/{:.1}{unit}", + current_data.arc_harvest.used_bytes as f64 / denominator, + (current_data.arc_harvest.total_bytes as f64 / denominator), ) }, )) @@ -625,23 +604,6 @@ pub struct ConvertedGpuData { pub fn convert_gpu_data( current_data: &crate::app::data_farmer::DataCollection, ) -> Option> { - /// 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}{}", - gpu.1.used_kib as f64 / denominator, - unit, - (gpu.1.total_kib as f64 / denominator), - unit + " {:.1}{unit}/{:.1}{unit}", + gpu.1.used_bytes as f64 / denominator, + (gpu.1.total_bytes as f64 / denominator), ) }, } diff --git a/src/lib.rs b/src/lib.rs index 1c23f6d4..2c1689d8 100644 --- a/src/lib.rs +++ b/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")] -- cgit v1.2.3