From 741054e84aa93d1343e19bbe754a01e6ca619c65 Mon Sep 17 00:00:00 2001 From: Clement Tsang <34804052+ClementTsang@users.noreply.github.com> Date: Thu, 15 Jul 2021 18:28:41 -0400 Subject: bug: fix inaccuracy in memory usage/total on macOS and Linux (#545) Fixes the accuracy of the memory widget for Linux and macOS, and uses binary prefixes instead to be more accurate. Regarding the first part, it turns out that the way I was calculating memory usage was *slightly* incorrect for a few reasons: - Regarding macOS, it seems like the way I was determining usage (`usage = total - available`) is not the most accurate. The better way of doing this is apparently `usage = wire + active`, where `wire` is memory always marked to stay in RAM, and `active` is memory currently in RAM. This seems to be much closer to other applications now. - Regarding Linux, this was somewhat due to two issues - one was that I should have used heim's own built-in function call to get how much memory was *used*, and the other is that when heim reads info from `meminfo`, it reads it in *kilobytes* - however, the values are actually in *kibibytes*. As such, to get the value in kibibytes, you want to actually take it in kilobytes. While I've filed an issue for the library, for now, I'll just manually bandaid over this. See https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/6/html/deployment_guide/s2-proc-meminfo for more info. Both changes take more advantage of platform-specific methods, and as such, the change unfortunately adds some ugly platform-specific code blocks. Side note, Windows Task Manager apparently (?) uses binary prefixes for the values behind the scenes, but displays decimal prefixes. As such, now that we've switched to binary prefixes, it'll "seem" like the two aren't matching anymore since the units won't match, despite the values matching. --- CHANGELOG.md | 9 ++++++- src/app/data_harvester/memory/heim.rs | 44 ++++++++++++++++++++++++++++++----- src/data_conversion.rs | 41 ++++++++++++-------------------- 3 files changed, 61 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c7d61176..e0ac5aa4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,14 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [0.6.3]/[0.7.0] - Unreleased +## [0.6.4]/[0.7.0] - Unreleased + +## [0.6.3] - Unreleased + +## Bug Fixes + +- [#542](https://github.com/ClementTsang/bottom/pull/542): Fixes missing config options in the default generated config file. +- [#545](https://github.com/ClementTsang/bottom/pull/545): Fixes inaccurate memory usage/totals in macOS and Linux. ## [0.6.2] - 2021-06-26 diff --git a/src/app/data_harvester/memory/heim.rs b/src/app/data_harvester/memory/heim.rs index 5319b1b3..6e455510 100644 --- a/src/app/data_harvester/memory/heim.rs +++ b/src/app/data_harvester/memory/heim.rs @@ -33,14 +33,46 @@ pub async fn get_mem_data( pub async fn get_ram_data() -> crate::utils::error::Result> { let memory = heim::memory::memory().await?; - let mem_total_in_kb = memory.total().get::(); + let (mem_total_in_kib, mem_used_in_kib) = { + #[cfg(target_os = "linux")] + { + // For Linux, the "kilobyte" value in the .total call is actually kibibytes - see + // https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/6/html/deployment_guide/s2-proc-meminfo + // + // Heim parses this as kilobytes (https://github.com/heim-rs/heim/blob/master/heim-memory/src/sys/linux/memory.rs#L82) + // even though it probably shouldn't... + + use heim::memory::os::linux::MemoryExt; + ( + memory.total().get::(), + memory.used().get::(), + ) + } + #[cfg(target_os = "macos")] + { + use heim::memory::os::macos::MemoryExt; + ( + memory.total().get::(), + memory.active().get::() + + memory.wire().get::(), + ) + } + #[cfg(target_os = "windows")] + { + let mem_total_in_kib = memory.total().get::(); + ( + mem_total_in_kib, + mem_total_in_kib + - memory + .available() + .get::(), + ) + } + }; Ok(Some(MemHarvest { - mem_total_in_kib: mem_total_in_kb, - mem_used_in_kib: mem_total_in_kb - - memory - .available() - .get::(), + mem_total_in_kib, + mem_used_in_kib, })) } diff --git a/src/data_conversion.rs b/src/data_conversion.rs index 1efc6db4..6548e0b6 100644 --- a/src/data_conversion.rs +++ b/src/data_conversion.rs @@ -300,22 +300,19 @@ pub fn convert_mem_labels( current_data: &data_farmer::DataCollection, ) -> (Option<(String, String)>, Option<(String, String)>) { /// Returns the unit type and denominator for given total amount of memory in kibibytes. - /// - /// Yes, this function is a bit of a lie. But people seem to generally expect, say, GiB when what they actually - /// wanted calculated was GiB. fn return_unit_and_denominator_for_mem_kib(mem_total_kib: u64) -> (&'static str, f64) { if mem_total_kib < 1024 { // Stay with KiB - ("KB", 1.0) - } else if mem_total_kib < 1_048_576 { + ("KiB", 1.0) + } else if mem_total_kib < MEBI_LIMIT { // Use MiB - ("MB", 1024.0) - } else if mem_total_kib < 1_073_741_824 { + ("MiB", KIBI_LIMIT_F64) + } else if mem_total_kib < GIBI_LIMIT { // Use GiB - ("GB", 1_048_576.0) + ("GiB", MEBI_LIMIT_F64) } else { // Use TiB - ("TB", 1_073_741_824.0) + ("TiB", GIBI_LIMIT_F64) } } @@ -324,13 +321,9 @@ pub fn convert_mem_labels( Some(( format!( "{:3.0}%", - match current_data.memory_harvest.mem_total_in_kib { - 0 => 0.0, - _ => - current_data.memory_harvest.mem_used_in_kib as f64 - / current_data.memory_harvest.mem_total_in_kib as f64 - * 100.0, - } + current_data.memory_harvest.mem_used_in_kib as f64 + / current_data.memory_harvest.mem_total_in_kib as f64 + * 100.0 ), { let (unit, denominator) = return_unit_and_denominator_for_mem_kib( @@ -353,24 +346,20 @@ pub fn convert_mem_labels( Some(( format!( "{:3.0}%", - match current_data.swap_harvest.mem_total_in_kib { - 0 => 0.0, - _ => - current_data.swap_harvest.mem_used_in_kib as f64 - / current_data.swap_harvest.mem_total_in_kib as f64 - * 100.0, - } + current_data.swap_harvest.mem_used_in_kib as f64 + / current_data.swap_harvest.mem_total_in_kib as f64 + * 100.0 ), { - let (unit, numerator) = return_unit_and_denominator_for_mem_kib( + let (unit, denominator) = return_unit_and_denominator_for_mem_kib( current_data.swap_harvest.mem_total_in_kib, ); format!( " {:.1}{}/{:.1}{}", - current_data.swap_harvest.mem_used_in_kib as f64 / numerator, + current_data.swap_harvest.mem_used_in_kib as f64 / denominator, unit, - (current_data.swap_harvest.mem_total_in_kib as f64 / numerator), + (current_data.swap_harvest.mem_total_in_kib as f64 / denominator), unit ) }, -- cgit v1.2.3