diff options
author | Clement Tsang <34804052+ClementTsang@users.noreply.github.com> | 2021-07-15 18:28:41 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-07-15 18:28:41 -0400 |
commit | 741054e84aa93d1343e19bbe754a01e6ca619c65 (patch) | |
tree | b5a6aac878a1d6e41174fbaff98e9c100b06fd85 /src | |
parent | b0199d4d1cbcb330e9080d4b94c94a51068fe269 (diff) |
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.
Diffstat (limited to 'src')
-rw-r--r-- | src/app/data_harvester/memory/heim.rs | 44 | ||||
-rw-r--r-- | src/data_conversion.rs | 41 |
2 files changed, 53 insertions, 32 deletions
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<Option<MemHarvest>> { let memory = heim::memory::memory().await?; - let mem_total_in_kb = memory.total().get::<heim::units::information::kibibyte>(); + 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::<heim::units::information::kilobyte>(), + memory.used().get::<heim::units::information::kibibyte>(), + ) + } + #[cfg(target_os = "macos")] + { + use heim::memory::os::macos::MemoryExt; + ( + memory.total().get::<heim::units::information::kibibyte>(), + memory.active().get::<heim::units::information::kibibyte>() + + memory.wire().get::<heim::units::information::kibibyte>(), + ) + } + #[cfg(target_os = "windows")] + { + let mem_total_in_kib = memory.total().get::<heim::units::information::kibibyte>(); + ( + mem_total_in_kib, + mem_total_in_kib + - memory + .available() + .get::<heim::units::information::kibibyte>(), + ) + } + }; Ok(Some(MemHarvest { - mem_total_in_kib: mem_total_in_kb, - mem_used_in_kib: mem_total_in_kb - - memory - .available() - .get::<heim::units::information::kibibyte>(), + 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 ) }, |