other: change how we calculate swap usage in Windows (#1769)

* bump sysinfo

* other: change how we calculate swap usage in Windows

* update changelog

* update comments

* add test?

* adjust test

CHANGELOG.md

@@ -52,6 +52,7 @@ That said, these are more guidelines rather than hardset rules, though the proje
 - [#1570](https://github.com/ClementTsang/bottom/pull/1570): Consider `$XDG_CONFIG_HOME` on macOS when looking for a default config path in a backwards-compatible fashion.
 - [#1686](https://github.com/ClementTsang/bottom/pull/1686): Allow hyphenated arguments to work as well (e.g. `--autohide-time`).
 - [#1701](https://github.com/ClementTsang/bottom/pull/1701): Redesign process kill dialog.
+- [#1769](https://github.com/ClementTsang/bottom/pull/1769): Change how we calculate swap usage in Windows.
 
 ### Other
 

Cargo.lock

@@ -1511,9 +1511,9 @@ dependencies = [
 
 [[package]]
 name = "sysinfo"
-version = "0.35.2"
+version = "0.36.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "3c3ffa3e4ff2b324a57f7aeb3c349656c7b127c3c189520251a648102a92496e"
+checksum = "252800745060e7b9ffb7b2badbd8b31cfa4aa2e61af879d0a3bf2a317c20217d"
 dependencies = [
  "libc",
  "memchr",

Cargo.toml

@@ -96,7 +96,7 @@ nvml-wrapper = { version = "0.11.0", optional = true, features = [
 regex = "1.11.1"
 serde = { version = "1.0.219", features = ["derive"] }
 starship-battery = { version = "0.10.2", optional = true }
-sysinfo = "=0.35.2"
+sysinfo = "=0.36.1"
 timeless = "0.0.14-alpha"
 toml_edit = { version = "0.22.27", features = ["serde"] }
 tui = { version = "0.29.0", package = "ratatui", features = [

docs/content/usage/widgets/memory.md

@@ -31,7 +31,9 @@ Note that key bindings are generally case-sensitive.
 | ------------ | -------------------------------------------------------------- |
 | ++"Scroll"++ | Scrolling up or down zooms in or out of the graph respectively |
 
-## Calculations
+## How are memory values determined?
+
+### Linux
 
 Memory usage is calculated using the following formula based on values from `/proc/meminfo` (based on [htop's implementation](https://github.com/htop-dev/htop/blob/976c6123f41492aaf613b9d172eef1842fb7b0a3/linux/LinuxProcessList.c#L1584)):
 
@@ -40,3 +42,10 @@ MemTotal - MemFree - Buffers - (Cached + SReclaimable - Shmem)
 ```
 
 You can find more info on `/proc/meminfo` and its fields [here](https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/6/html/deployment_guide/s2-proc-meminfo).
+
+### Windows
+
+In Windows, we calculate swap by querying `Get-Counter "\Paging File(*)\% Usage"`. This
+is also what some libraries like [psutil](https://github.com/giampaolo/psutil/blob/master/psutil/arch/windows/mem.c) use. However, note there are also a few other valid methods of
+representing "swap" in Windows (e.g. using `GetPerformanceInfo`), which all slightly don't
+match.

src/collection/memory.rs

@@ -2,18 +2,18 @@
 
 use std::num::NonZeroU64;
 
-#[cfg(not(target_os = "windows"))]
-pub(crate) use self::sysinfo::get_cache_usage;
-pub(crate) use self::sysinfo::{get_ram_usage, get_swap_usage};
+pub(crate) use self::sysinfo::get_ram_usage;
 
 pub mod sysinfo;
 
-// cfg_if::cfg_if! {
-//     if #[cfg(target_os = "windows")] {
-//         mod windows;
-//         pub(crate) use self::windows::get_committed_usage;
-//     }
-// }
+cfg_if::cfg_if! {
+    if #[cfg(target_os = "windows")] {
+        mod windows;
+        pub(crate) use self::windows::get_swap_usage;
+    } else {
+        pub(crate) use self::sysinfo::{get_cache_usage, get_swap_usage};
+    }
+}
 
 #[cfg(feature = "zfs")]
 pub mod arc;

src/collection/memory/sysinfo.rs

@@ -20,6 +20,7 @@ pub(crate) fn get_ram_usage(sys: &System) -> Option<MemData> {
 }
 
 /// Returns SWAP usage.
+#[cfg(not(target_os = "windows"))]
 pub(crate) fn get_swap_usage(sys: &System) -> Option<MemData> {
     get_usage(sys.used_swap(), sys.total_swap())
 }

src/collection/memory/windows.rs

@@ -1,32 +1,96 @@
-use std::mem::{size_of, zeroed};
+use std::{mem::zeroed, num::NonZeroU64};
 
-use windows::Win32::System::ProcessStatus::{GetPerformanceInfo, PERFORMANCE_INFORMATION};
+use sysinfo::System;
+use windows::{
+    Win32::{
+        Foundation::ERROR_SUCCESS,
+        System::Performance::{
+            PDH_FMT_COUNTERVALUE, PDH_FMT_DOUBLE, PDH_HCOUNTER, PDH_HQUERY, PdhAddEnglishCounterW,
+            PdhCloseQuery, PdhCollectQueryData, PdhGetFormattedCounterValue, PdhOpenQueryW,
+            PdhRemoveCounter,
+        },
+    },
+    core::w,
+};
 
-use crate::collection::memory::MemHarvest;
+use crate::collection::memory::MemData;
 
-const PERFORMANCE_INFORMATION_SIZE: u32 = size_of::<PERFORMANCE_INFORMATION>() as _;
-
-/// Get the committed memory usage.
+/// Get swap memory usage on Windows. This does it by using checking Windows' performance counters.
+/// This is based on the technique done by psutil [here](https://github.com/giampaolo/psutil/pull/2160).
 ///
-/// Code based on [sysinfo's](https://github.com/GuillaumeGomez/sysinfo/blob/6f8178495adcf3ca4696a9ec548586cf6a621bc8/src/windows/system.rs#L169).
-pub(crate) fn get_committed_usage() -> Option<MemHarvest> {
-    // SAFETY: The safety invariant is that we only touch what's in `perf_info` if it succeeds, and that
-    // the bindings are "safe" to use with how we call them.
+/// Also see:
+/// - <https://github.com/GuillaumeGomez/sysinfo/blob/master/src/windows/system.rs>
+/// - <https://learn.microsoft.com/en-us/windows/win32/api/psapi/ns-psapi-performance_information>
+/// - <https://en.wikipedia.org/wiki/Commit_charge>.
+/// - <https://github.com/giampaolo/psutil/issues/2431>
+/// - <https://github.com/oshi/oshi/issues/1175>
+/// - <https://github.com/oshi/oshi/issues/1182>
+pub(crate) fn get_swap_usage(sys: &System) -> Option<MemData> {
+    let total_bytes = NonZeroU64::new(sys.total_swap())?;
+
+    // See https://kennykerr.ca/rust-getting-started/string-tutorial.html
+    let query = w!("\\Paging File(_Total)\\% Usage");
+
+    // SAFETY: Hits a few Windows APIs; this should be safe as we check each step, and
+    // we clean up at the end.
     unsafe {
-        let mut perf_info: PERFORMANCE_INFORMATION = zeroed();
-        if GetPerformanceInfo(&mut perf_info, PERFORMANCE_INFORMATION_SIZE).is_ok() {
-            let page_size = perf_info.PageSize;
+        let mut query_handle: PDH_HQUERY = zeroed();
+        let mut counter_handle: PDH_HCOUNTER = zeroed();
+        let mut counter_value: PDH_FMT_COUNTERVALUE = zeroed();
 
-            let committed_total = page_size.saturating_mul(perf_info.CommitLimit) as u64;
-            let committed_used = page_size.saturating_mul(perf_info.CommitTotal) as u64;
+        if PdhOpenQueryW(None, 0, &mut query_handle) != ERROR_SUCCESS.0 {
+            return None;
+        }
 
-            Some(MemHarvest {
-                used_bytes: committed_used,
-                total_bytes: committed_total,
-                use_percent: Some(committed_used as f64 / committed_total as f64 * 100.0),
-            })
+        if PdhAddEnglishCounterW(query_handle, query, 0, &mut counter_handle) != ERROR_SUCCESS.0 {
+            return None;
+        }
+
+        // May fail if swap is disabled.
+        if PdhCollectQueryData(query_handle) != ERROR_SUCCESS.0 {
+            return None;
+        }
+
+        if PdhGetFormattedCounterValue(counter_handle, PDH_FMT_DOUBLE, None, &mut counter_value)
+            != ERROR_SUCCESS.0
+        {
+            // If we fail, still clean up.
+            PdhCloseQuery(query_handle);
+            return None;
+        }
+
+        let use_percentage = counter_value.Anonymous.doubleValue;
+
+        // Cleanup.
+        PdhRemoveCounter(counter_handle);
+        PdhCloseQuery(query_handle);
+
+        let used_bytes = (total_bytes.get() as f64 / 100.0 * use_percentage) as u64;
+        Some(MemData {
+            used_bytes,
+            total_bytes,
+        })
+    }
+}
+
+#[cfg(all(target_os = "windows", test))]
+mod tests {
+    use sysinfo::{MemoryRefreshKind, RefreshKind};
+
+    use super::*;
+
+    #[test]
+    fn test_windows_get_swap_usage() {
+        let sys = System::new_with_specifics(
+            RefreshKind::nothing().with_memory(MemoryRefreshKind::nothing().with_swap()),
+        );
+
+        let swap_usage = get_swap_usage(&sys);
+        if sys.total_swap() > 0 {
+            // Not sure if we can guarantee this to always pass on a machine, so I'll just print out.
+            println!("swap: {swap_usage:?}");
         } else {
-            None
+            println!("No swap, skipping.");
         }
     }
 }