Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for total accumulated process CPU usage #1044

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/apple/app_store/process.rs
Expand Up @@ -68,6 +68,10 @@ impl ProcessExt for Process {
0.0
}

fn total_accumulated_cpu_usage(&self) -> f32 {
0.0
}

fn disk_usage(&self) -> DiskUsage {
DiskUsage::default()
}
Expand Down
6 changes: 3 additions & 3 deletions src/apple/cpu.rs
Expand Up @@ -255,7 +255,7 @@ pub(crate) fn update_cpu_usage<F: FnOnce(Arc<CpuData>, *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(
Expand All @@ -268,9 +268,9 @@ pub(crate) fn update_cpu_usage<F: FnOnce(Arc<CpuData>, *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);
}
}

Expand Down
4 changes: 4 additions & 0 deletions src/apple/macos/process.rs
Expand Up @@ -181,6 +181,10 @@ impl ProcessExt for Process {
self.cpu_usage
}

fn total_accumulated_cpu_usage(&self) -> f32 {
self.old_utime.saturating_add(self.old_stime) as f32
}

fn disk_usage(&self) -> DiskUsage {
DiskUsage {
read_bytes: self.read_bytes.saturating_sub(self.old_read_bytes),
Expand Down
11 changes: 11 additions & 0 deletions src/freebsd/process.rs
Expand Up @@ -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,
Expand Down Expand Up @@ -129,6 +130,10 @@ impl ProcessExt for Process {
self.cpu_usage
}

fn total_accumulated_cpu_usage(&self) -> f32 {
self.accum_cpu_usage
}

fn disk_usage(&self) -> DiskUsage {
DiskUsage {
written_bytes: self.written_bytes.saturating_sub(self.old_written_bytes),
Expand Down Expand Up @@ -207,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;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is quite right. Here is the relevant code from FreeBSD:

	secs = k->ki_p->ki_runtime / 1000000;
	psecs = k->ki_p->ki_runtime % 1000000;
	if (sumrusage) {
		secs += k->ki_p->ki_childtime.tv_sec;
		psecs += k->ki_p->ki_childtime.tv_usec;
	}

It is clear that child time is only added if sumrusage is true. It is set by the -S switch:

   -S      Change  the way the process times, namely cputime, systime, and
	       usertime, are calculated	by  summing  all  exited  children  to
	       their parent process.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you're right, I thought this needed to account for the child's time, but that is handled separately.


// 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 _);
Expand Down Expand Up @@ -275,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
Expand Down
38 changes: 38 additions & 0 deletions src/lib.rs
Expand Up @@ -203,6 +203,8 @@ mod doctest {

#[cfg(test)]
mod test {
use std::collections::HashMap;

use crate::*;

#[cfg(feature = "unknown-ci")]
Expand Down Expand Up @@ -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 {
Expand Down
7 changes: 7 additions & 0 deletions src/linux/process.rs
Expand Up @@ -194,6 +194,13 @@ impl ProcessExt for Process {
self.cpu_usage
}

fn total_accumulated_cpu_usage(&self) -> f32 {
// 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 {
DiskUsage {
written_bytes: self.written_bytes.saturating_sub(self.old_written_bytes),
Expand Down
14 changes: 14 additions & 0 deletions src/traits.rs
Expand Up @@ -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_accumulated_cpu_usage());
/// }
/// ```
fn total_accumulated_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.
Expand Down
4 changes: 4 additions & 0 deletions src/unknown/process.rs
Expand Up @@ -78,6 +78,10 @@ impl ProcessExt for Process {
0.0
}

fn total_accumulated_cpu_usage(&self) -> f32 {
0.0
}

fn disk_usage(&self) -> DiskUsage {
DiskUsage::default()
}
Expand Down
16 changes: 16 additions & 0 deletions src/windows/process.rs
Expand Up @@ -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 {
Expand All @@ -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
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, I don't think this is right. It should only return old_process_user_cpu + old_process_sys_cpu. The other parts are dividing with the global CPU time and number of CPUs, which just isn't right here.

}
static WINDOWS_8_1_OR_NEWER: Lazy<bool> = Lazy::new(|| unsafe {
let mut version_info: RTL_OSVERSIONINFOEXW = MaybeUninit::zeroed().assume_init();
Expand Down Expand Up @@ -599,6 +610,10 @@ impl ProcessExt for Process {
self.cpu_usage
}

fn total_accumulated_cpu_usage(&self) -> f32 {
self.cpu_calc_values.total_accumulated_cpu_usage()
}

fn disk_usage(&self) -> DiskUsage {
DiskUsage {
written_bytes: self.written_bytes.saturating_sub(self.old_written_bytes),
Expand Down Expand Up @@ -1108,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;

Expand Down