diff --git a/src/common/system.rs b/src/common/system.rs index 219d9881d..5fd0d0548 100644 --- a/src/common/system.rs +++ b/src/common/system.rs @@ -367,6 +367,9 @@ impl System { /// exist (it will **NOT** be removed from the processes if it doesn't exist anymore). If it /// isn't listed yet, it'll be added. /// + /// ⚠️ If you need to refresh multiple processes at once, use [`refresh_pids`] instead! It has + /// much better performance. + /// /// It is the same as calling: /// /// ```no_run @@ -394,6 +397,8 @@ impl System { /// let mut s = System::new_all(); /// s.refresh_process(Pid::from(1337)); /// ``` + /// + /// [`refresh_pids`]: #method.refresh_pids pub fn refresh_process(&mut self, pid: Pid) -> bool { self.refresh_process_specifics( pid, @@ -409,6 +414,9 @@ impl System { /// exist (it will **NOT** be removed from the processes if it doesn't exist anymore). If it /// isn't listed yet, it'll be added. /// + /// ⚠️ If you need to refresh multiple processes at once, use [`refresh_pids_specifics`] + /// instead! It has much better performance. + /// /// ⚠️ On Linux, `sysinfo` keeps the `stat` files open by default. You can change this behaviour /// by using [`set_open_files_limit`][crate::set_open_files_limit]. /// @@ -418,6 +426,8 @@ impl System { /// let mut s = System::new_all(); /// s.refresh_process_specifics(Pid::from(1337), ProcessRefreshKind::new()); /// ``` + /// + /// [`refresh_pids_specifics`]: #method.refresh_pids_specifics pub fn refresh_process_specifics( &mut self, pid: Pid, diff --git a/src/unix/apple/macos/system.rs b/src/unix/apple/macos/system.rs index d0191eb2e..6c182ae1f 100644 --- a/src/unix/apple/macos/system.rs +++ b/src/unix/apple/macos/system.rs @@ -8,6 +8,7 @@ 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::time::Instant; struct ProcessorCpuLoadInfo { cpu_load: processor_cpu_load_info_t, @@ -55,6 +56,8 @@ pub(crate) struct SystemTimeInfo { timebase_to_ns: f64, clock_per_sec: f64, old_cpu_info: ProcessorCpuLoadInfo, + last_update: Option, + prev_time_interval: f64, } unsafe impl Send for SystemTimeInfo {} @@ -97,40 +100,52 @@ impl SystemTimeInfo { 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, + last_update: None, + prev_time_interval: 0., }) } } pub fn get_time_interval(&mut self, port: mach_port_t) -> f64 { - let mut total = 0; - let new_cpu_info = match ProcessorCpuLoadInfo::new(port) { - Some(cpu_info) => cpu_info, - None => return 0., - }; - let cpu_count = std::cmp::min(self.old_cpu_info.cpu_count, new_cpu_info.cpu_count); - unsafe { - for i in 0..cpu_count { - let new_load: &processor_cpu_load_info = &*new_cpu_info.cpu_load.offset(i as _); - let old_load: &processor_cpu_load_info = - &*self.old_cpu_info.cpu_load.offset(i as _); - for (new, old) in new_load.cpu_ticks.iter().zip(old_load.cpu_ticks.iter()) { - if new > old { - total += new.saturating_sub(*old); + let need_cpu_usage_update = self + .last_update + .map(|last_update| last_update.elapsed() > crate::MINIMUM_CPU_UPDATE_INTERVAL) + .unwrap_or(true); + if need_cpu_usage_update { + let mut total = 0; + let new_cpu_info = match ProcessorCpuLoadInfo::new(port) { + Some(cpu_info) => cpu_info, + None => return 0., + }; + let cpu_count = std::cmp::min(self.old_cpu_info.cpu_count, new_cpu_info.cpu_count); + unsafe { + for i in 0..cpu_count { + let new_load: &processor_cpu_load_info = &*new_cpu_info.cpu_load.offset(i as _); + let old_load: &processor_cpu_load_info = + &*self.old_cpu_info.cpu_load.offset(i as _); + for (new, old) in new_load.cpu_ticks.iter().zip(old_load.cpu_ticks.iter()) { + if new > old { + total += new.saturating_sub(*old); + } } } } self.old_cpu_info = new_cpu_info; + self.last_update = Some(Instant::now()); // Now we convert the ticks to nanoseconds (if the interval is less than // `MINIMUM_CPU_UPDATE_INTERVAL`, we replace it with it instead): let base_interval = total as f64 / cpu_count as f64 * self.clock_per_sec; let smallest = crate::MINIMUM_CPU_UPDATE_INTERVAL.as_secs_f64() * 1_000_000_000.0; - if base_interval < smallest { + self.prev_time_interval = if base_interval < smallest { smallest } else { base_interval / self.timebase_to_ns - } + }; + self.prev_time_interval + } else { + self.prev_time_interval } } } diff --git a/tests/process.rs b/tests/process.rs index f6f841a6f..e0963de56 100644 --- a/tests/process.rs +++ b/tests/process.rs @@ -813,3 +813,47 @@ fn test_parent_change() { // We kill the child to clean up. child.kill(); } + +// We want to ensure that if `System::refresh_process*` methods are called +// one after the other, it won't impact the CPU usage computation badly. +#[test] +fn test_multiple_single_process_refresh() { + if !sysinfo::IS_SUPPORTED_SYSTEM || cfg!(feature = "apple-sandbox") || cfg!(windows) { + // Windows never updates its parent PID so no need to check anything. + return; + } + + let file_name = "target/test_binary3"; + build_test_binary(file_name); + let mut p_a = std::process::Command::new(format!("./{file_name}")) + .arg("1") + .spawn() + .unwrap(); + let mut p_b = std::process::Command::new(format!("./{file_name}")) + .arg("1") + .spawn() + .unwrap(); + + let pid_a = Pid::from_u32(p_a.id() as _); + let pid_b = Pid::from_u32(p_b.id() as _); + + let mut s = System::new(); + let process_refresh_kind = ProcessRefreshKind::new().with_cpu(); + s.refresh_process_specifics(pid_a, process_refresh_kind); + s.refresh_process_specifics(pid_b, process_refresh_kind); + + std::thread::sleep(std::time::Duration::from_secs(1)); + s.refresh_process_specifics(pid_a, process_refresh_kind); + s.refresh_process_specifics(pid_b, process_refresh_kind); + + let cpu_a = s.process(pid_a).unwrap().cpu_usage(); + let cpu_b = s.process(pid_b).unwrap().cpu_usage(); + + p_a.kill().expect("failed to kill process a"); + p_b.kill().expect("failed to kill process b"); + + let _ = p_a.wait(); + let _ = p_b.wait(); + + assert!(cpu_b - 5. < cpu_a && cpu_b + 5. > cpu_a); +}