From a5a25ea7c39ba5430f2d0011d60385b61e34c46b Mon Sep 17 00:00:00 2001 From: Bruce Guenter Date: Mon, 14 Aug 2023 13:09:28 -0600 Subject: [PATCH 01/17] Add support for total accumulated process CPU usage Most, if not all, CPU usage accounting for processes provides values that count from the creation of the process. This total value is useful for a variety of accounting tasks beyond the snapshot value that is currently available in sysinfo. This change adds a `fn total_cpu_usage` to `trait ProcessExt` to provide that value. --- src/apple/app_store/process.rs | 4 ++++ src/apple/macos/process.rs | 4 ++++ src/freebsd/process.rs | 4 ++++ src/linux/process.rs | 8 +++++++- src/traits.rs | 14 ++++++++++++++ src/unknown/process.rs | 4 ++++ src/windows/process.rs | 4 ++++ 7 files changed, 41 insertions(+), 1 deletion(-) diff --git a/src/apple/app_store/process.rs b/src/apple/app_store/process.rs index d7df72afb..600163631 100644 --- a/src/apple/app_store/process.rs +++ b/src/apple/app_store/process.rs @@ -68,6 +68,10 @@ impl ProcessExt for Process { 0.0 } + fn total_cpu_usage(&self) -> f32 { + 0.0 + } + fn disk_usage(&self) -> DiskUsage { DiskUsage::default() } diff --git a/src/apple/macos/process.rs b/src/apple/macos/process.rs index 102a8a6bc..034c83573 100644 --- a/src/apple/macos/process.rs +++ b/src/apple/macos/process.rs @@ -181,6 +181,10 @@ impl ProcessExt for Process { self.cpu_usage } + fn total_cpu_usage(&self) -> f32 { + (self.utime + self.stime) as f32 + } + fn disk_usage(&self) -> DiskUsage { DiskUsage { read_bytes: self.read_bytes.saturating_sub(self.old_read_bytes), diff --git a/src/freebsd/process.rs b/src/freebsd/process.rs index 52064e179..8dbce1808 100644 --- a/src/freebsd/process.rs +++ b/src/freebsd/process.rs @@ -129,6 +129,10 @@ impl ProcessExt for Process { self.cpu_usage } + fn total_cpu_usage(&self) -> f32 { + FIXME + } + fn disk_usage(&self) -> DiskUsage { DiskUsage { written_bytes: self.written_bytes.saturating_sub(self.old_written_bytes), diff --git a/src/linux/process.rs b/src/linux/process.rs index 7c2d599e0..2808db296 100644 --- a/src/linux/process.rs +++ b/src/linux/process.rs @@ -17,6 +17,8 @@ use crate::sys::utils::{ use crate::utils::into_iter; use crate::{DiskUsage, Gid, Pid, ProcessExt, ProcessRefreshKind, ProcessStatus, Signal, Uid}; +const HZ: f32 = 100.; + #[doc(hidden)] impl From for ProcessStatus { fn from(status: char) -> ProcessStatus { @@ -194,6 +196,10 @@ impl ProcessExt for Process { self.cpu_usage } + fn total_cpu_usage(&self) -> f32 { + self.utime.saturating_add(self.stime) as f32 / HZ + } + fn disk_usage(&self) -> DiskUsage { DiskUsage { written_bytes: self.written_bytes.saturating_sub(self.old_written_bytes), @@ -258,7 +264,7 @@ pub(crate) fn compute_cpu_usage(p: &mut Process, total_time: f32, max_value: f32 .saturating_sub(p.old_utime) .saturating_add(p.stime.saturating_sub(p.old_stime)) as f32 / total_time - * 100.) + * HZ) .min(max_value); for task in p.tasks.values_mut() { diff --git a/src/traits.rs b/src/traits.rs index c41280df3..05919f7a2 100644 --- a/src/traits.rs +++ b/src/traits.rs @@ -375,6 +375,20 @@ pub trait ProcessExt: Debug { /// ``` fn cpu_usage(&self) -> f32; + /// Returns the total accumulated CPU usage (in + /// CPU-seconds). Notice that it might be bigger than the total + /// clock run time of a process if run on a multi-core machine. + /// + /// ```no_run + /// use sysinfo::{Pid, ProcessExt, System, SystemExt}; + /// + /// let s = System::new_all(); + /// if let Some(process) = s.process(Pid::from(1337)) { + /// println!("{}sec", process.total_cpu_usage()); + /// } + /// ``` + fn total_cpu_usage(&self) -> f32; + /// Returns number of bytes read and written to disk. /// /// ⚠️ On Windows and FreeBSD, this method actually returns **ALL** I/O read and written bytes. diff --git a/src/unknown/process.rs b/src/unknown/process.rs index 4e116d7df..d59d933b6 100644 --- a/src/unknown/process.rs +++ b/src/unknown/process.rs @@ -78,6 +78,10 @@ impl ProcessExt for Process { 0.0 } + fn total_cpu_usage(&self) -> f32 { + 0.0 + } + fn disk_usage(&self) -> DiskUsage { DiskUsage::default() } diff --git a/src/windows/process.rs b/src/windows/process.rs index 0e9ff1cd5..02c6e53c3 100644 --- a/src/windows/process.rs +++ b/src/windows/process.rs @@ -599,6 +599,10 @@ impl ProcessExt for Process { self.cpu_usage } + fn total_cpu_usage(&self) -> f32 { + FIXME + } + fn disk_usage(&self) -> DiskUsage { DiskUsage { written_bytes: self.written_bytes.saturating_sub(self.old_written_bytes), From cf86f66b6ddb8bc43a554a386901140ec8e90db9 Mon Sep 17 00:00:00 2001 From: Bruce Guenter Date: Mon, 14 Aug 2023 13:41:05 -0600 Subject: [PATCH 02/17] Change method name to `total_accumulated_cpu_usage` --- src/apple/app_store/process.rs | 2 +- src/apple/cpu.rs | 6 +++--- src/apple/macos/process.rs | 2 +- src/freebsd/process.rs | 2 +- src/linux/process.rs | 2 +- src/traits.rs | 4 ++-- src/unknown/process.rs | 2 +- src/windows/process.rs | 2 +- 8 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/apple/app_store/process.rs b/src/apple/app_store/process.rs index 600163631..acef5b0d8 100644 --- a/src/apple/app_store/process.rs +++ b/src/apple/app_store/process.rs @@ -68,7 +68,7 @@ impl ProcessExt for Process { 0.0 } - fn total_cpu_usage(&self) -> f32 { + fn total_accumulated_cpu_usage(&self) -> f32 { 0.0 } diff --git a/src/apple/cpu.rs b/src/apple/cpu.rs index 1fe7bc287..8809bdd18 100644 --- a/src/apple/cpu.rs +++ b/src/apple/cpu.rs @@ -255,7 +255,7 @@ pub(crate) fn update_cpu_usage, *mut i32) -> (f32, usize) let mut cpu_info: *mut i32 = std::ptr::null_mut(); let mut num_cpu_info = 0u32; - let mut total_cpu_usage = 0f32; + let mut total_accumulated_cpu_usage = 0f32; unsafe { if host_processor_info( @@ -268,9 +268,9 @@ pub(crate) fn update_cpu_usage, *mut i32) -> (f32, usize) { let (total_percentage, len) = f(Arc::new(CpuData::new(cpu_info, num_cpu_info)), cpu_info); - total_cpu_usage = total_percentage / len as f32; + total_accumulated_cpu_usage = total_percentage / len as f32; } - global_cpu.set_cpu_usage(total_cpu_usage); + global_cpu.set_cpu_usage(total_accumulated_cpu_usage); } } diff --git a/src/apple/macos/process.rs b/src/apple/macos/process.rs index 034c83573..feee803a6 100644 --- a/src/apple/macos/process.rs +++ b/src/apple/macos/process.rs @@ -181,7 +181,7 @@ impl ProcessExt for Process { self.cpu_usage } - fn total_cpu_usage(&self) -> f32 { + fn total_accumulated_cpu_usage(&self) -> f32 { (self.utime + self.stime) as f32 } diff --git a/src/freebsd/process.rs b/src/freebsd/process.rs index 8dbce1808..58b8049c8 100644 --- a/src/freebsd/process.rs +++ b/src/freebsd/process.rs @@ -129,7 +129,7 @@ impl ProcessExt for Process { self.cpu_usage } - fn total_cpu_usage(&self) -> f32 { + fn total_accumulated_cpu_usage(&self) -> f32 { FIXME } diff --git a/src/linux/process.rs b/src/linux/process.rs index 2808db296..29a661819 100644 --- a/src/linux/process.rs +++ b/src/linux/process.rs @@ -196,7 +196,7 @@ impl ProcessExt for Process { self.cpu_usage } - fn total_cpu_usage(&self) -> f32 { + fn total_accumulated_cpu_usage(&self) -> f32 { self.utime.saturating_add(self.stime) as f32 / HZ } diff --git a/src/traits.rs b/src/traits.rs index 05919f7a2..672e36307 100644 --- a/src/traits.rs +++ b/src/traits.rs @@ -384,10 +384,10 @@ pub trait ProcessExt: Debug { /// /// let s = System::new_all(); /// if let Some(process) = s.process(Pid::from(1337)) { - /// println!("{}sec", process.total_cpu_usage()); + /// println!("{}sec", process.total_accumulated_cpu_usage()); /// } /// ``` - fn total_cpu_usage(&self) -> f32; + fn total_accumulated_cpu_usage(&self) -> f32; /// Returns number of bytes read and written to disk. /// diff --git a/src/unknown/process.rs b/src/unknown/process.rs index d59d933b6..a4185e5ce 100644 --- a/src/unknown/process.rs +++ b/src/unknown/process.rs @@ -78,7 +78,7 @@ impl ProcessExt for Process { 0.0 } - fn total_cpu_usage(&self) -> f32 { + fn total_accumulated_cpu_usage(&self) -> f32 { 0.0 } diff --git a/src/windows/process.rs b/src/windows/process.rs index 02c6e53c3..9c1371fe0 100644 --- a/src/windows/process.rs +++ b/src/windows/process.rs @@ -599,7 +599,7 @@ impl ProcessExt for Process { self.cpu_usage } - fn total_cpu_usage(&self) -> f32 { + fn total_accumulated_cpu_usage(&self) -> f32 { FIXME } From 2a888c952d1ef59b8f5a52206dd4979dd9991f1c Mon Sep 17 00:00:00 2001 From: Bruce Guenter Date: Tue, 15 Aug 2023 09:11:59 -0600 Subject: [PATCH 03/17] Fix the Linux `HZ` constant mistake --- src/linux/process.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/linux/process.rs b/src/linux/process.rs index 29a661819..748ebf37e 100644 --- a/src/linux/process.rs +++ b/src/linux/process.rs @@ -17,8 +17,6 @@ use crate::sys::utils::{ use crate::utils::into_iter; use crate::{DiskUsage, Gid, Pid, ProcessExt, ProcessRefreshKind, ProcessStatus, Signal, Uid}; -const HZ: f32 = 100.; - #[doc(hidden)] impl From for ProcessStatus { fn from(status: char) -> ProcessStatus { @@ -197,7 +195,10 @@ impl ProcessExt for Process { } fn total_accumulated_cpu_usage(&self) -> f32 { - self.utime.saturating_add(self.stime) as f32 / HZ + // The external values for CPU times are in "ticks", which are + // scaled by "HZ", which is pegged externally at 100 + // ticks/second. + self.utime.saturating_add(self.stime) as f32 / 100. } fn disk_usage(&self) -> DiskUsage { @@ -264,7 +265,7 @@ pub(crate) fn compute_cpu_usage(p: &mut Process, total_time: f32, max_value: f32 .saturating_sub(p.old_utime) .saturating_add(p.stime.saturating_sub(p.old_stime)) as f32 / total_time - * HZ) + * 100.) .min(max_value); for task in p.tasks.values_mut() { From 081947309380bd169b42fadcd46b950e2030deab Mon Sep 17 00:00:00 2001 From: Bruce Guenter Date: Tue, 15 Aug 2023 09:24:42 -0600 Subject: [PATCH 04/17] Fix MacOS compilation typo --- src/apple/macos/process.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/apple/macos/process.rs b/src/apple/macos/process.rs index feee803a6..9e3576fdb 100644 --- a/src/apple/macos/process.rs +++ b/src/apple/macos/process.rs @@ -182,7 +182,7 @@ impl ProcessExt for Process { } fn total_accumulated_cpu_usage(&self) -> f32 { - (self.utime + self.stime) as f32 + self.old_utime.saturating_add(self.old_stime) as f32 } fn disk_usage(&self) -> DiskUsage { From 3586f8b5f55d200f3ec75602aa1f570821561f1d Mon Sep 17 00:00:00 2001 From: Bruce Guenter Date: Tue, 15 Aug 2023 10:19:40 -0600 Subject: [PATCH 05/17] Implement Windows calculation --- src/windows/process.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/windows/process.rs b/src/windows/process.rs index 9c1371fe0..9408b9978 100644 --- a/src/windows/process.rs +++ b/src/windows/process.rs @@ -212,6 +212,7 @@ struct CPUsageCalculationValues { old_process_user_cpu: u64, old_system_sys_cpu: u64, old_system_user_cpu: u64, + nb_cpus: u64, } impl CPUsageCalculationValues { @@ -221,8 +222,18 @@ impl CPUsageCalculationValues { old_process_user_cpu: 0, old_system_sys_cpu: 0, old_system_user_cpu: 0, + nb_cpus: 0, } } + + fn total_accumulated_cpu_usage(&self) -> f32 { + self.old_process_user_cpu + .saturating_add(self.old_process_sys_cpu) as f32 + / self + .old_system_user_cpu + .saturating_add(self.old_system_sys_cpu) as f32 + * self.nb_cpus as f32 + } } static WINDOWS_8_1_OR_NEWER: Lazy = Lazy::new(|| unsafe { let mut version_info: RTL_OSVERSIONINFOEXW = MaybeUninit::zeroed().assume_init(); @@ -600,7 +611,7 @@ impl ProcessExt for Process { } fn total_accumulated_cpu_usage(&self) -> f32 { - FIXME + self.cpu_calc_values.total_accumulated_cpu_usage() } fn disk_usage(&self) -> DiskUsage { @@ -1112,6 +1123,7 @@ pub(crate) fn compute_cpu_usage(p: &mut Process, nb_cpus: u64) { p.cpu_calc_values.old_process_sys_cpu = sys; p.cpu_calc_values.old_system_user_cpu = global_user_time; p.cpu_calc_values.old_system_sys_cpu = global_kernel_time; + p.cpu_calc_values.nb_cpus = nb_cpus; let denominator = delta_global_user_time.saturating_add(delta_global_kernel_time) as f32; From 46b9b74d15a15b977ab67488b1754182e825f055 Mon Sep 17 00:00:00 2001 From: Bruce Guenter Date: Tue, 15 Aug 2023 16:12:39 -0600 Subject: [PATCH 06/17] Fill in the FreeBSD implementation --- src/freebsd/process.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/freebsd/process.rs b/src/freebsd/process.rs index 58b8049c8..f55bfc392 100644 --- a/src/freebsd/process.rs +++ b/src/freebsd/process.rs @@ -54,6 +54,7 @@ pub struct Process { pub(crate) virtual_memory: u64, pub(crate) updated: bool, cpu_usage: f32, + accum_cpu_usage: f32, start_time: u64, run_time: u64, pub(crate) status: ProcessStatus, @@ -130,7 +131,7 @@ impl ProcessExt for Process { } fn total_accumulated_cpu_usage(&self) -> f32 { - FIXME + self.accum_cpu_usage } fn disk_usage(&self) -> DiskUsage { @@ -211,6 +212,11 @@ pub(crate) unsafe fn get_process_data( }; let status = ProcessStatus::from(kproc.ki_stat); + // from FreeBSD source /bin/ps/print.c + let accum_cpu_usage = (kproc.ki_runtime as f64 / 1000000.0) as f32 + + kproc.ki_childtime.tv_sec as f32 + + kproc.ki_childtime.tv_usec as f32 / 1000000.0; + // from FreeBSD source /src/usr.bin/top/machine.c let virtual_memory = kproc.ki_size as _; let memory = (kproc.ki_rssize as u64).saturating_mul(page_size as _); @@ -279,6 +285,7 @@ pub(crate) unsafe fn get_process_data( start_time, run_time: now.saturating_sub(start_time), cpu_usage, + accum_cpu_usage, virtual_memory, memory, // procstat_getfiles From 668c7f977c4c68bfc3599dc0aa4198a52325d2d7 Mon Sep 17 00:00:00 2001 From: Bruce Guenter Date: Mon, 28 Aug 2023 10:53:49 -0600 Subject: [PATCH 07/17] Add test for new method --- src/lib.rs | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index 54b9f0bf9..f30928587 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -203,6 +203,8 @@ mod doctest { #[cfg(test)] mod test { + use std::collections::HashMap; + use crate::*; #[cfg(feature = "unknown-ci")] @@ -283,6 +285,42 @@ mod test { .any(|(_, proc_)| proc_.cpu_usage() > 0.0)); } + #[test] + fn check_processes_total_accumulated_cpu_usage() { + if System::IS_SUPPORTED { + let mut s = System::new(); + + s.refresh_processes(); + let all_procs: HashMap<_, _> = s + .processes() + .iter() + .map(|(pid, proc)| (*pid, proc.total_accumulated_cpu_usage())) + .collect(); + // All accumulated CPU usages will be non-negative. + assert!(all_procs.values().all(|&usage| usage >= 0.0)); + + // Wait a bit to update CPU usage values + std::thread::sleep(System::MINIMUM_CPU_UPDATE_INTERVAL); + s.refresh_processes(); + // They will still all be non-negative. + assert!(s + .processes() + .values() + .all(|proc| proc.total_accumulated_cpu_usage() >= 0.0)); + // They will all have either remained the same or + // increased no more than a valid amount. + let max_delta = + s.cpus().len() as f32 * System::MINIMUM_CPU_UPDATE_INTERVAL.as_secs_f64() as f32; + assert!(s + .processes() + .iter() + .all(|(pid, proc)| all_procs.get(pid).map_or(true, |&prev| { + let delta = proc.total_accumulated_cpu_usage() - prev; + delta >= 0.0 && delta <= max_delta + }))); + } + } + #[test] fn check_cpu_usage() { if !System::IS_SUPPORTED { From 61835a16e22bda063db70679edb951ff4774b97d Mon Sep 17 00:00:00 2001 From: Bruce Guenter Date: Tue, 12 Sep 2023 14:49:19 +0200 Subject: [PATCH 08/17] Update the FreeBSD calculation to not include child usage --- src/freebsd/process.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/freebsd/process.rs b/src/freebsd/process.rs index f55bfc392..da0219548 100644 --- a/src/freebsd/process.rs +++ b/src/freebsd/process.rs @@ -213,9 +213,7 @@ pub(crate) unsafe fn get_process_data( let status = ProcessStatus::from(kproc.ki_stat); // from FreeBSD source /bin/ps/print.c - let accum_cpu_usage = (kproc.ki_runtime as f64 / 1000000.0) as f32 - + kproc.ki_childtime.tv_sec as f32 - + kproc.ki_childtime.tv_usec as f32 / 1000000.0; + let accum_cpu_usage = (kproc.ki_runtime as f64 / 1000000.0) as f32; // from FreeBSD source /src/usr.bin/top/machine.c let virtual_memory = kproc.ki_size as _; From 2cd08343e3fbe753be5b0b56ff8eca0ff02a992d Mon Sep 17 00:00:00 2001 From: Bruce Guenter Date: Tue, 12 Sep 2023 17:41:58 +0200 Subject: [PATCH 09/17] Improve test asserts to pin down failures --- src/lib.rs | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index f30928587..9c4ec7c76 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -297,27 +297,28 @@ mod test { .map(|(pid, proc)| (*pid, proc.total_accumulated_cpu_usage())) .collect(); // All accumulated CPU usages will be non-negative. - assert!(all_procs.values().all(|&usage| usage >= 0.0)); + all_procs.values().for_each(|&usage| assert!(usage >= 0.0)); // Wait a bit to update CPU usage values std::thread::sleep(System::MINIMUM_CPU_UPDATE_INTERVAL); s.refresh_processes(); // They will still all be non-negative. - assert!(s - .processes() + s.processes() .values() - .all(|proc| proc.total_accumulated_cpu_usage() >= 0.0)); + .for_each(|proc| assert!(proc.total_accumulated_cpu_usage() >= 0.0)); // They will all have either remained the same or // increased no more than a valid amount. let max_delta = s.cpus().len() as f32 * System::MINIMUM_CPU_UPDATE_INTERVAL.as_secs_f64() as f32; - assert!(s - .processes() - .iter() - .all(|(pid, proc)| all_procs.get(pid).map_or(true, |&prev| { + s.processes().iter().for_each(|(pid, proc)| { + if let Some(prev) = all_procs.get(pid) { let delta = proc.total_accumulated_cpu_usage() - prev; - delta >= 0.0 && delta <= max_delta - }))); + assert!( + delta >= 0.0 && delta <= max_delta, + "CPU time delta is out of range delta={delta} max_delta={max_delta}" + ); + } + }); } } From 1bc46bd1382239425e546fb209ef0728059e93f7 Mon Sep 17 00:00:00 2001 From: Bruce Guenter Date: Tue, 12 Sep 2023 17:58:35 +0200 Subject: [PATCH 10/17] Fix too-new format usage --- src/lib.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 9c4ec7c76..2a314345b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -315,7 +315,9 @@ mod test { let delta = proc.total_accumulated_cpu_usage() - prev; assert!( delta >= 0.0 && delta <= max_delta, - "CPU time delta is out of range delta={delta} max_delta={max_delta}" + "CPU time delta is out of range delta={} max_delta={}", + delta, + max_delta, ); } }); From 9174a33bfc325651abb8fb14084b0f058dbde532 Mon Sep 17 00:00:00 2001 From: Bruce Guenter Date: Wed, 13 Sep 2023 13:15:51 +0200 Subject: [PATCH 11/17] Use the Linux clock tick value retrieved from sysconf --- src/linux/process.rs | 8 +++++--- src/linux/system.rs | 5 +++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/linux/process.rs b/src/linux/process.rs index 748ebf37e..acad24b85 100644 --- a/src/linux/process.rs +++ b/src/linux/process.rs @@ -89,10 +89,11 @@ pub struct Process { old_written_bytes: u64, read_bytes: u64, written_bytes: u64, + clock_cycle: u64, } impl Process { - pub(crate) fn new(pid: Pid) -> Process { + pub(crate) fn new(pid: Pid, info: &SystemInfo) -> Process { Process { name: String::with_capacity(20), pid, @@ -128,6 +129,7 @@ impl Process { old_written_bytes: 0, read_bytes: 0, written_bytes: 0, + clock_cycle: info.clock_cycle, } } } @@ -198,7 +200,7 @@ impl ProcessExt for Process { // The external values for CPU times are in "ticks", which are // scaled by "HZ", which is pegged externally at 100 // ticks/second. - self.utime.saturating_add(self.stime) as f32 / 100. + self.utime.saturating_add(self.stime) as f32 / self.clock_cycle as f32 } fn disk_usage(&self) -> DiskUsage { @@ -376,7 +378,7 @@ fn retrieve_all_new_process_info( refresh_kind: ProcessRefreshKind, uptime: u64, ) -> Process { - let mut p = Process::new(pid); + let mut p = Process::new(pid, info); let mut tmp = PathHandler::new(path); let name = parts[1]; diff --git a/src/linux/system.rs b/src/linux/system.rs index 25dd08722..bb07911cb 100644 --- a/src/linux/system.rs +++ b/src/linux/system.rs @@ -220,7 +220,8 @@ impl SystemExt for System { const MINIMUM_CPU_UPDATE_INTERVAL: Duration = Duration::from_millis(200); fn new_with_specifics(refreshes: RefreshKind) -> System { - let process_list = Process::new(Pid(0)); + let info = SystemInfo::new(); + let process_list = Process::new(Pid(0), &info); let mut s = System { process_list, mem_total: 0, @@ -237,7 +238,7 @@ impl SystemExt for System { disks: Disks::new(), networks: Networks::new(), users: Vec::new(), - info: SystemInfo::new(), + info, }; s.refresh_specifics(refreshes); s From 00183747b92d5befcf2aadac508df5f0f8cc885a Mon Sep 17 00:00:00 2001 From: Bruce Guenter Date: Fri, 15 Sep 2023 13:49:37 +0200 Subject: [PATCH 12/17] Use actual times when calculating the max delta to avoid false positives --- src/lib.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 2a314345b..48516a0e0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -204,6 +204,7 @@ mod doctest { #[cfg(test)] mod test { use std::collections::HashMap; + use std::time::Instant; use crate::*; @@ -291,6 +292,7 @@ mod test { let mut s = System::new(); s.refresh_processes(); + let first_time = Instant::now(); let all_procs: HashMap<_, _> = s .processes() .iter() @@ -302,14 +304,14 @@ mod test { // Wait a bit to update CPU usage values std::thread::sleep(System::MINIMUM_CPU_UPDATE_INTERVAL); s.refresh_processes(); + let duration = Instant::now().duration_since(first_time).as_secs_f32(); // They will still all be non-negative. s.processes() .values() .for_each(|proc| assert!(proc.total_accumulated_cpu_usage() >= 0.0)); // They will all have either remained the same or // increased no more than a valid amount. - let max_delta = - s.cpus().len() as f32 * System::MINIMUM_CPU_UPDATE_INTERVAL.as_secs_f64() as f32; + let max_delta = s.cpus().len() as f32 * duration; s.processes().iter().for_each(|(pid, proc)| { if let Some(prev) = all_procs.get(pid) { let delta = proc.total_accumulated_cpu_usage() - prev; From c6b0456de964263a5b57ad2b90a371b4f079d8c3 Mon Sep 17 00:00:00 2001 From: Bruce Guenter Date: Tue, 19 Sep 2023 13:52:57 -0600 Subject: [PATCH 13/17] Fixes for MacOS --- src/apple/macos/process.rs | 10 +++++- src/apple/macos/system.rs | 73 ++++++++++++++++++++++++-------------- src/lib.rs | 5 ++- 3 files changed, 59 insertions(+), 29 deletions(-) diff --git a/src/apple/macos/process.rs b/src/apple/macos/process.rs index 9e3576fdb..9d4f02d0a 100644 --- a/src/apple/macos/process.rs +++ b/src/apple/macos/process.rs @@ -11,6 +11,7 @@ use libc::{c_int, c_void, kill, size_t}; use crate::{DiskUsage, Gid, Pid, ProcessExt, ProcessRefreshKind, ProcessStatus, Signal, Uid}; +use super::system::{TimeBaseInfo, NANOS_PER_SECOND}; use crate::sys::process::ThreadStatus; use crate::sys::system::Wrap; @@ -32,6 +33,7 @@ pub struct Process { run_time: u64, pub(crate) updated: bool, cpu_usage: f32, + accum_cpu_usage: u64, user_id: Option, effective_user_id: Option, group_id: Option, @@ -62,6 +64,7 @@ impl Process { memory: 0, virtual_memory: 0, cpu_usage: 0., + accum_cpu_usage: 0, old_utime: 0, old_stime: 0, updated: true, @@ -93,6 +96,7 @@ impl Process { memory: 0, virtual_memory: 0, cpu_usage: 0., + accum_cpu_usage: 0, old_utime: 0, old_stime: 0, updated: true, @@ -182,7 +186,8 @@ impl ProcessExt for Process { } fn total_accumulated_cpu_usage(&self) -> f32 { - self.old_utime.saturating_add(self.old_stime) as f32 + let timebase = TimeBaseInfo::get(); + (self.accum_cpu_usage as f64 * timebase.timebase_to_ns / NANOS_PER_SECOND) as f32 } fn disk_usage(&self) -> DiskUsage { @@ -618,6 +623,9 @@ pub(crate) fn update_process( if refresh_kind.cpu() { compute_cpu_usage(p, task_info, system_time, user_time, time_interval); } + p.accum_cpu_usage = task_info + .pti_total_user + .saturating_add(task_info.pti_total_system); p.memory = task_info.pti_resident_size; p.virtual_memory = task_info.pti_virtual_size; diff --git a/src/apple/macos/system.rs b/src/apple/macos/system.rs index 6c5a79031..f3f26b7d6 100644 --- a/src/apple/macos/system.rs +++ b/src/apple/macos/system.rs @@ -10,6 +10,9 @@ use libc::{ processor_cpu_load_info_t, sysconf, vm_page_size, PROCESSOR_CPU_LOAD_INFO, _SC_CLK_TCK, }; use std::ptr::null_mut; +use std::sync::OnceLock; + +pub(crate) const NANOS_PER_SECOND: f64 = 1_000_000_000.; struct ProcessorCpuLoadInfo { cpu_load: processor_cpu_load_info_t, @@ -53,20 +56,18 @@ impl Drop for ProcessorCpuLoadInfo { } } -pub(crate) struct SystemTimeInfo { - timebase_to_ns: f64, - clock_per_sec: f64, - old_cpu_info: ProcessorCpuLoadInfo, +#[derive(Clone, Copy)] +pub(crate) struct TimeBaseInfo { + pub timebase_to_ns: f64, + pub clock_per_sec: f64, } -unsafe impl Send for SystemTimeInfo {} -unsafe impl Sync for SystemTimeInfo {} - -impl SystemTimeInfo { +impl TimeBaseInfo { #[allow(deprecated)] // Everything related to mach_timebase_info_data_t - pub fn new(port: mach_port_t) -> Option { - unsafe { - let clock_ticks_per_sec = sysconf(_SC_CLK_TCK); + pub(crate) fn get() -> Self { + static INFO: OnceLock = OnceLock::new(); + *INFO.get_or_init(|| { + let clock_ticks_per_sec = unsafe { sysconf(_SC_CLK_TCK) }; // FIXME: Maybe check errno here? Problem is that if errno is not 0 before this call, // we will get an error which isn't related... @@ -79,28 +80,46 @@ impl SystemTimeInfo { // } let mut info = mach_timebase_info_data_t { numer: 0, denom: 0 }; - if mach_timebase_info(&mut info) != libc::KERN_SUCCESS { + if unsafe { mach_timebase_info(&mut info) } != libc::KERN_SUCCESS { sysinfo_debug!("mach_timebase_info failed, using default value of 1"); info.numer = 1; info.denom = 1; } + Self { + timebase_to_ns: info.numer as f64 / info.denom as f64, + clock_per_sec: NANOS_PER_SECOND / clock_ticks_per_sec as f64, + } + }) + } +} - let old_cpu_info = match ProcessorCpuLoadInfo::new(port) { - Some(cpu_info) => cpu_info, - None => { - sysinfo_debug!("host_processor_info failed, using old CPU tick measure system"); - return None; - } - }; +pub(crate) struct SystemTimeInfo { + pub(crate) timebase_to_ns: f64, + clock_per_sec: f64, + old_cpu_info: ProcessorCpuLoadInfo, +} - let nano_per_seconds = 1_000_000_000.; - sysinfo_debug!(""); - Some(Self { - timebase_to_ns: info.numer as f64 / info.denom as f64, - clock_per_sec: nano_per_seconds / clock_ticks_per_sec as f64, - old_cpu_info, - }) - } +unsafe impl Send for SystemTimeInfo {} +unsafe impl Sync for SystemTimeInfo {} + +impl SystemTimeInfo { + pub fn new(port: mach_port_t) -> Option { + let time_base = TimeBaseInfo::get(); + + let old_cpu_info = match ProcessorCpuLoadInfo::new(port) { + Some(cpu_info) => cpu_info, + None => { + sysinfo_debug!("host_processor_info failed, using old CPU tick measure system"); + return None; + } + }; + + sysinfo_debug!(""); + Some(Self { + timebase_to_ns: time_base.timebase_to_ns, + clock_per_sec: time_base.clock_per_sec, + old_cpu_info, + }) } pub fn get_time_interval(&mut self, port: mach_port_t) -> f64 { diff --git a/src/lib.rs b/src/lib.rs index 48516a0e0..92d6e5b55 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -291,7 +291,9 @@ mod test { if System::IS_SUPPORTED { let mut s = System::new(); + s.refresh_cpu(); s.refresh_processes(); + s.refresh_processes(); // Needed on some OS to fully populate the accumulated CPU usage let first_time = Instant::now(); let all_procs: HashMap<_, _> = s .processes() @@ -317,9 +319,10 @@ mod test { let delta = proc.total_accumulated_cpu_usage() - prev; assert!( delta >= 0.0 && delta <= max_delta, - "CPU time delta is out of range delta={} max_delta={}", + "CPU time delta is out of range delta={} max_delta={} pid={}", delta, max_delta, + pid, ); } }); From 21ef386aba3d5f6b728b3468e0e245bb34dcdca4 Mon Sep 17 00:00:00 2001 From: Bruce Guenter Date: Tue, 19 Sep 2023 16:05:10 -0600 Subject: [PATCH 14/17] Fix Windows computations --- src/windows/process.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/windows/process.rs b/src/windows/process.rs index 9408b9978..f891ccc23 100644 --- a/src/windows/process.rs +++ b/src/windows/process.rs @@ -229,12 +229,9 @@ impl CPUsageCalculationValues { fn total_accumulated_cpu_usage(&self) -> f32 { self.old_process_user_cpu .saturating_add(self.old_process_sys_cpu) as f32 - / self - .old_system_user_cpu - .saturating_add(self.old_system_sys_cpu) as f32 - * self.nb_cpus as f32 } } + static WINDOWS_8_1_OR_NEWER: Lazy = Lazy::new(|| unsafe { let mut version_info: RTL_OSVERSIONINFOEXW = MaybeUninit::zeroed().assume_init(); From 2eaa4ddfa2295aa840068ac9825d2ee5340acc80 Mon Sep 17 00:00:00 2001 From: Bruce Guenter Date: Thu, 21 Sep 2023 12:18:04 -0600 Subject: [PATCH 15/17] Drop the static timebase info and use the copy in `struct System` --- src/unix/apple/macos/process.rs | 16 ++++---- src/unix/apple/macos/system.rs | 73 +++++++++++++-------------------- src/unix/apple/system.rs | 8 ++++ 3 files changed, 45 insertions(+), 52 deletions(-) diff --git a/src/unix/apple/macos/process.rs b/src/unix/apple/macos/process.rs index e5f4a500c..73f1c0886 100644 --- a/src/unix/apple/macos/process.rs +++ b/src/unix/apple/macos/process.rs @@ -11,7 +11,6 @@ use libc::{c_int, c_void, kill, size_t}; use crate::{DiskUsage, Gid, Pid, ProcessExt, ProcessRefreshKind, ProcessStatus, Signal, Uid}; -use super::system::{TimeBaseInfo, NANOS_PER_SECOND}; use crate::sys::process::ThreadStatus; use crate::sys::system::Wrap; @@ -33,7 +32,7 @@ pub struct Process { run_time: u64, pub(crate) updated: bool, cpu_usage: f32, - accum_cpu_usage: u64, + accum_cpu_usage: f32, user_id: Option, effective_user_id: Option, group_id: Option, @@ -64,7 +63,7 @@ impl Process { memory: 0, virtual_memory: 0, cpu_usage: 0., - accum_cpu_usage: 0, + accum_cpu_usage: 0., old_utime: 0, old_stime: 0, updated: true, @@ -96,7 +95,7 @@ impl Process { memory: 0, virtual_memory: 0, cpu_usage: 0., - accum_cpu_usage: 0, + accum_cpu_usage: 0., old_utime: 0, old_stime: 0, updated: true, @@ -186,8 +185,7 @@ impl ProcessExt for Process { } fn total_accumulated_cpu_usage(&self) -> f32 { - let timebase = TimeBaseInfo::get(); - (self.accum_cpu_usage as f64 * timebase.timebase_to_ns / NANOS_PER_SECOND) as f32 + self.accum_cpu_usage } fn disk_usage(&self) -> DiskUsage { @@ -575,6 +573,7 @@ pub(crate) fn update_process( now: u64, refresh_kind: ProcessRefreshKind, check_if_alive: bool, + timebase_to_seconds: f64, ) -> Result, ()> { unsafe { if let Some(ref mut p) = (*wrap.0.get()).get_mut(&pid) { @@ -623,9 +622,10 @@ pub(crate) fn update_process( if refresh_kind.cpu() { compute_cpu_usage(p, task_info, system_time, user_time, time_interval); } - p.accum_cpu_usage = task_info + p.accum_cpu_usage = (task_info .pti_total_user - .saturating_add(task_info.pti_total_system); + .saturating_add(task_info.pti_total_system) as f64 + * timebase_to_seconds) as f32; p.memory = task_info.pti_resident_size; p.virtual_memory = task_info.pti_virtual_size; diff --git a/src/unix/apple/macos/system.rs b/src/unix/apple/macos/system.rs index f3f26b7d6..c433fde64 100644 --- a/src/unix/apple/macos/system.rs +++ b/src/unix/apple/macos/system.rs @@ -10,7 +10,6 @@ use libc::{ processor_cpu_load_info_t, sysconf, vm_page_size, PROCESSOR_CPU_LOAD_INFO, _SC_CLK_TCK, }; use std::ptr::null_mut; -use std::sync::OnceLock; pub(crate) const NANOS_PER_SECOND: f64 = 1_000_000_000.; @@ -56,18 +55,21 @@ impl Drop for ProcessorCpuLoadInfo { } } -#[derive(Clone, Copy)] -pub(crate) struct TimeBaseInfo { - pub timebase_to_ns: f64, - pub clock_per_sec: f64, +pub(crate) struct SystemTimeInfo { + timebase_to_ns: f64, + pub timebase_to_sec: f64, + clock_per_sec: f64, + old_cpu_info: ProcessorCpuLoadInfo, } -impl TimeBaseInfo { +unsafe impl Send for SystemTimeInfo {} +unsafe impl Sync for SystemTimeInfo {} + +impl SystemTimeInfo { #[allow(deprecated)] // Everything related to mach_timebase_info_data_t - pub(crate) fn get() -> Self { - static INFO: OnceLock = OnceLock::new(); - *INFO.get_or_init(|| { - let clock_ticks_per_sec = unsafe { sysconf(_SC_CLK_TCK) }; + pub fn new(port: mach_port_t) -> Option { + unsafe { + let clock_ticks_per_sec = sysconf(_SC_CLK_TCK); // FIXME: Maybe check errno here? Problem is that if errno is not 0 before this call, // we will get an error which isn't related... @@ -80,46 +82,29 @@ impl TimeBaseInfo { // } let mut info = mach_timebase_info_data_t { numer: 0, denom: 0 }; - if unsafe { mach_timebase_info(&mut info) } != libc::KERN_SUCCESS { + if mach_timebase_info(&mut info) != libc::KERN_SUCCESS { sysinfo_debug!("mach_timebase_info failed, using default value of 1"); info.numer = 1; info.denom = 1; } - Self { - timebase_to_ns: info.numer as f64 / info.denom as f64, - clock_per_sec: NANOS_PER_SECOND / clock_ticks_per_sec as f64, - } - }) - } -} - -pub(crate) struct SystemTimeInfo { - pub(crate) timebase_to_ns: f64, - clock_per_sec: f64, - old_cpu_info: ProcessorCpuLoadInfo, -} - -unsafe impl Send for SystemTimeInfo {} -unsafe impl Sync for SystemTimeInfo {} -impl SystemTimeInfo { - pub fn new(port: mach_port_t) -> Option { - let time_base = TimeBaseInfo::get(); - - let old_cpu_info = match ProcessorCpuLoadInfo::new(port) { - Some(cpu_info) => cpu_info, - None => { - sysinfo_debug!("host_processor_info failed, using old CPU tick measure system"); - return None; - } - }; + let old_cpu_info = match ProcessorCpuLoadInfo::new(port) { + Some(cpu_info) => cpu_info, + None => { + sysinfo_debug!("host_processor_info failed, using old CPU tick measure system"); + return None; + } + }; - sysinfo_debug!(""); - Some(Self { - timebase_to_ns: time_base.timebase_to_ns, - clock_per_sec: time_base.clock_per_sec, - old_cpu_info, - }) + sysinfo_debug!(""); + let timebase_to_ns = info.numer as f64 / info.denom as f64; + Some(Self { + timebase_to_ns, + timebase_to_sec: timebase_to_ns / NANOS_PER_SECOND, + clock_per_sec: NANOS_PER_SECOND / clock_ticks_per_sec as f64, + old_cpu_info, + }) + } } pub fn get_time_interval(&mut self, port: mach_port_t) -> f64 { diff --git a/src/unix/apple/system.rs b/src/unix/apple/system.rs index 04891eaa0..d7ebcaae4 100644 --- a/src/unix/apple/system.rs +++ b/src/unix/apple/system.rs @@ -237,6 +237,10 @@ impl SystemExt for System { let arg_max = get_arg_max(); let port = self.port; let time_interval = self.clock_info.as_mut().map(|c| c.get_time_interval(port)); + let timebase_to_sec = self + .clock_info + .as_ref() + .map_or(1.0, |ci| ci.timebase_to_sec); let entries: Vec = { let wrap = &Wrap(UnsafeCell::new(&mut self.process_list)); @@ -253,6 +257,7 @@ impl SystemExt for System { now, refresh_kind, false, + timebase_to_sec, ) { Ok(x) => x, _ => None, @@ -293,6 +298,9 @@ impl SystemExt for System { now, refresh_kind, true, + self.clock_info + .as_ref() + .map_or(1.0, |ci| ci.timebase_to_sec), ) } { Ok(Some(p)) => { From 61bdf992d3f85b62379feeeecb8f8949a1bf867e Mon Sep 17 00:00:00 2001 From: Bruce Guenter Date: Thu, 21 Sep 2023 13:24:03 -0600 Subject: [PATCH 16/17] Clippy fix --- src/unix/apple/macos/process.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/unix/apple/macos/process.rs b/src/unix/apple/macos/process.rs index 73f1c0886..1fc376294 100644 --- a/src/unix/apple/macos/process.rs +++ b/src/unix/apple/macos/process.rs @@ -565,6 +565,7 @@ unsafe fn create_new_process( Ok(Some(p)) } +#[allow(clippy::too_many_arguments)] pub(crate) fn update_process( wrap: &Wrap, pid: Pid, From 8e14b4591b09f2aacb3a4f333418c0af698f0f41 Mon Sep 17 00:00:00 2001 From: Bruce Guenter Date: Thu, 21 Sep 2023 17:16:36 -0600 Subject: [PATCH 17/17] Fix Windows usage and improve test --- src/lib.rs | 14 ++++++++++++++ src/windows/process.rs | 4 ++++ 2 files changed, 18 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index fab3c90f0..233a9e7df 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -263,6 +263,7 @@ mod test { if System::IS_SUPPORTED { let mut s = System::new(); + // Grab the intial accumulated CPU usages s.refresh_cpu(); s.refresh_processes(); s.refresh_processes(); // Needed on some OS to fully populate the accumulated CPU usage @@ -272,17 +273,22 @@ mod test { .iter() .map(|(pid, proc)| (*pid, proc.total_accumulated_cpu_usage())) .collect(); + // All accumulated CPU usages will be non-negative. all_procs.values().for_each(|&usage| assert!(usage >= 0.0)); + // At least one will be positive. + assert!(all_procs.values().any(|&usage| usage > 0.0)); // Wait a bit to update CPU usage values std::thread::sleep(System::MINIMUM_CPU_UPDATE_INTERVAL); s.refresh_processes(); let duration = Instant::now().duration_since(first_time).as_secs_f32(); + // They will still all be non-negative. s.processes() .values() .for_each(|proc| assert!(proc.total_accumulated_cpu_usage() >= 0.0)); + // They will all have either remained the same or // increased no more than a valid amount. let max_delta = s.cpus().len() as f32 * duration; @@ -298,6 +304,14 @@ mod test { ); } }); + + // At least one of them will have accumulated some CPU time. + #[cfg(not(windows))] // Windows CPU timers appear to have insufficient resolution + assert!(s.processes().iter().any(|(pid, proc)| { + all_procs + .get(pid) + .map_or(false, |&prev| proc.total_accumulated_cpu_usage() > prev) + })); } } diff --git a/src/windows/process.rs b/src/windows/process.rs index f891ccc23..763c1480a 100644 --- a/src/windows/process.rs +++ b/src/windows/process.rs @@ -58,6 +58,8 @@ use winapi::um::winnt::{ RTL_OSVERSIONINFOEXW, TOKEN_QUERY, TOKEN_USER, ULARGE_INTEGER, }; +const FILETIMES_PER_SECOND: f32 = 10_000_000.0; // 100 nanosecond units + impl fmt::Display for ProcessStatus { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { f.write_str(match *self { @@ -207,6 +209,7 @@ pub struct Process { written_bytes: u64, } +#[derive(Debug)] struct CPUsageCalculationValues { old_process_sys_cpu: u64, old_process_user_cpu: u64, @@ -229,6 +232,7 @@ impl CPUsageCalculationValues { fn total_accumulated_cpu_usage(&self) -> f32 { self.old_process_user_cpu .saturating_add(self.old_process_sys_cpu) as f32 + / FILETIMES_PER_SECOND } }