Skip to content

Commit

Permalink
bug: fix inaccuracy in memory usage/total on macOS and Linux (#545)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ClementTsang authored Jul 15, 2021
1 parent b0199d4 commit 741054e
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 33 deletions.
9 changes: 8 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
44 changes: 38 additions & 6 deletions src/app/data_harvester/memory/heim.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}))
}

Expand Down
41 changes: 15 additions & 26 deletions src/data_conversion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand All @@ -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(
Expand All @@ -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
)
},
Expand Down

0 comments on commit 741054e

Please sign in to comment.