From 1dc9346d3bba695908d0a86a3d2ff07ea2fb0736 Mon Sep 17 00:00:00 2001 From: Clement Tsang <34804052+ClementTsang@users.noreply.github.com> Date: Thu, 20 Aug 2020 22:33:12 -0700 Subject: [PATCH] refactor: Remove ps calls Removes and refactor ps calls that... should have not been there in the first place. --- .vscode/settings.json | 5 + Cargo.lock | 1 + Cargo.toml | 2 +- src/app/data_harvester.rs | 23 +-- src/app/data_harvester/processes.rs | 243 ++++++++++++++-------------- src/app/process_killer.rs | 1 + src/app/query.rs | 1 + src/data_conversion.rs | 4 +- src/lib.rs | 5 + src/utils/error.rs | 3 + 10 files changed, 151 insertions(+), 137 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 5074ded4..4ede05b1 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -13,6 +13,7 @@ "WASD", "Wojnarowski", "andys", + "cmdline", "crossterm", "curr", "czvf", @@ -29,9 +30,12 @@ "paren", "processthreadsapi", "regexes", + "rsplitn", "rustfmt", "shilangyu", "softirq", + "splitn", + "statm", "stime", "subwidget", "sysconf", @@ -43,6 +47,7 @@ "use curr usage", "utime", "virt", + "vsize", "whitespaces", "winapi", "winnt", diff --git a/Cargo.lock b/Cargo.lock index 4d9be6aa..6b8219ca 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -145,6 +145,7 @@ dependencies = [ "heim", "itertools", "lazy_static", + "libc", "log", "predicates", "regex", diff --git a/Cargo.toml b/Cargo.toml index 7a908c72..4ba087e8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -40,7 +40,7 @@ backtrace = "0.3" serde = {version = "1.0", features = ["derive"] } unicode-segmentation = "1.6.0" unicode-width = "0.1.7" -# libc = "0.2.74" +libc = "0.2.74" # tui = {version = "0.10.0", features = ["crossterm"], default-features = false, git = "https://github.com/fdehau/tui-rs.git"} tui = {version = "0.9.5", features = ["crossterm"], default-features = false } diff --git a/src/app/data_harvester.rs b/src/app/data_harvester.rs index e04d8354..042638b0 100644 --- a/src/app/data_harvester.rs +++ b/src/app/data_harvester.rs @@ -72,7 +72,7 @@ pub struct DataCollector { pub data: Data, sys: System, #[cfg(target_os = "linux")] - prev_pid_stats: HashMap, + pid_mapping: HashMap, #[cfg(target_os = "linux")] prev_idle: f64, #[cfg(target_os = "linux")] @@ -87,7 +87,7 @@ pub struct DataCollector { widgets_to_harvest: UsedWidgets, battery_manager: Option, battery_list: Option>, - // page_file_size_kb: u64, + page_file_size_kb: u64, } impl Default for DataCollector { @@ -96,7 +96,7 @@ impl Default for DataCollector { data: Data::default(), sys: System::new_all(), #[cfg(target_os = "linux")] - prev_pid_stats: HashMap::new(), + pid_mapping: HashMap::new(), #[cfg(target_os = "linux")] prev_idle: 0_f64, #[cfg(target_os = "linux")] @@ -111,11 +111,15 @@ impl Default for DataCollector { widgets_to_harvest: UsedWidgets::default(), battery_manager: None, battery_list: None, - // page_file_size_kb: if cfg!(target_os = "linux") { - // unsafe { libc::sysconf(libc::_SC_PAGESIZE) as u64 / 1024 } - // } else { - // 0 - // }, + page_file_size_kb: { + #[cfg(target_os = "linux")] + unsafe { + libc::sysconf(libc::_SC_PAGESIZE) as u64 / 1024 + } + + #[cfg(not(target_os = "linux"))] + 0 + }, } } } @@ -201,12 +205,13 @@ impl DataCollector { processes::linux_get_processes_list( &mut self.prev_idle, &mut self.prev_non_idle, - &mut self.prev_pid_stats, + &mut self.pid_mapping, self.use_current_cpu_total, current_instant .duration_since(self.last_collection_time) .as_secs(), self.mem_total_kb, + self.page_file_size_kb, ) } #[cfg(not(target_os = "linux"))] diff --git a/src/app/data_harvester/processes.rs b/src/app/data_harvester/processes.rs index 3877c35a..8bf23795 100644 --- a/src/app/data_harvester/processes.rs +++ b/src/app/data_harvester/processes.rs @@ -2,12 +2,10 @@ use std::path::PathBuf; use sysinfo::ProcessStatus; #[cfg(target_os = "linux")] -use crate::utils::error; +use crate::utils::error::{self, BottomError}; + #[cfg(target_os = "linux")] -use std::{ - collections::{hash_map::RandomState, HashMap}, - process::Command, -}; +use std::collections::{hash_map::RandomState, HashMap}; #[cfg(not(target_os = "linux"))] use sysinfo::{ProcessExt, ProcessorExt, System, SystemExt}; @@ -66,7 +64,7 @@ pub struct ProcessHarvest { // pub rss_kb: u64, // pub virt_kb: u64, pub name: String, - pub path: String, + pub command: String, pub read_bytes_per_sec: u64, pub write_bytes_per_sec: u64, pub total_read_bytes: u64, @@ -81,17 +79,21 @@ pub struct PrevProcDetails { pub total_write_bytes: u64, pub cpu_time: f64, pub proc_stat_path: PathBuf, + // pub proc_statm_path: PathBuf, pub proc_exe_path: PathBuf, pub proc_io_path: PathBuf, + pub proc_cmdline_path: PathBuf, + pub just_read: bool, } impl PrevProcDetails { pub fn new(pid: u32) -> Self { - let pid_string = pid.to_string(); PrevProcDetails { - proc_io_path: PathBuf::from(format!("/proc/{}/io", pid_string)), - proc_exe_path: PathBuf::from(format!("/proc/{}/exe", pid_string)), - proc_stat_path: PathBuf::from(format!("/proc/{}/stat", pid_string)), + proc_io_path: PathBuf::from(format!("/proc/{}/io", pid)), + proc_exe_path: PathBuf::from(format!("/proc/{}/exe", pid)), + proc_stat_path: PathBuf::from(format!("/proc/{}/stat", pid)), + // proc_statm_path: PathBuf::from(format!("/proc/{}/statm", pid)), + proc_cmdline_path: PathBuf::from(format!("/proc/{}/cmdline", pid)), ..PrevProcDetails::default() } } @@ -172,23 +174,32 @@ fn get_process_io(path: &PathBuf) -> std::io::Result { } #[cfg(target_os = "linux")] -fn get_linux_process_io_usage(io_stats: &[&str]) -> (u64, u64) { +fn get_linux_process_io_usage(stat: &[&str]) -> (u64, u64) { // Represents read_bytes and write_bytes ( - io_stats[9].parse::().unwrap_or(0), - io_stats[11].parse::().unwrap_or(0), + stat[9].parse::().unwrap_or(0), + stat[11].parse::().unwrap_or(0), ) } #[cfg(target_os = "linux")] -fn get_process_stats(path: &PathBuf) -> std::io::Result { +fn get_linux_process_vsize_rss(stat: &[&str]) -> (u64, u64) { + // Represents vsize and rss (bytes and page numbers respectively) + ( + stat[20].parse::().unwrap_or(0), + stat[21].parse::().unwrap_or(0), + ) +} + +#[cfg(target_os = "linux")] +fn read_path_contents(path: &PathBuf) -> std::io::Result { Ok(std::fs::read_to_string(path)?) } #[cfg(target_os = "linux")] -fn get_linux_process_state(proc_stats: &[&str]) -> (char, String) { +fn get_linux_process_state(stat: &[&str]) -> (char, String) { // The -2 offset is because of us cutting off name + pid - if let Some(first_char) = proc_stats[0].chars().collect::>().first() { + if let Some(first_char) = stat[0].chars().collect::>().first() { ( *first_char, ProcessStatus::from(*first_char).to_string().to_string(), @@ -201,103 +212,98 @@ fn get_linux_process_state(proc_stats: &[&str]) -> (char, String) { /// Note that cpu_fraction should be represented WITHOUT the x100 factor! #[cfg(target_os = "linux")] fn get_linux_cpu_usage( - proc_stats: &[&str], cpu_usage: f64, cpu_fraction: f64, before_proc_val: f64, + proc_stats: &[&str], cpu_usage: f64, cpu_fraction: f64, prev_proc_val: &mut f64, use_current_cpu_total: bool, -) -> std::io::Result<(f64, f64)> { - fn get_process_cpu_stats(stats: &[&str]) -> f64 { - // utime + stime (matches top), the -2 offset is because of us cutting off name + pid - stats[11].parse::().unwrap_or(0_f64) + stats[12].parse::().unwrap_or(0_f64) +) -> std::io::Result { + fn get_process_cpu_stats(stat: &[&str]) -> f64 { + // utime + stime (matches top), the -2 offset is because of us cutting off name + pid (normally 13, 14) + stat[11].parse::().unwrap_or(0_f64) + stat[12].parse::().unwrap_or(0_f64) } // Based heavily on https://stackoverflow.com/a/23376195 and https://stackoverflow.com/a/1424556 - let after_proc_val = get_process_cpu_stats(&proc_stats); + let new_proc_val = get_process_cpu_stats(&proc_stats); if cpu_usage == 0.0 { - Ok((0_f64, after_proc_val)) + Ok(0_f64) } else if use_current_cpu_total { - Ok(( - (after_proc_val - before_proc_val) / cpu_usage * 100_f64, - after_proc_val, - )) + let res = Ok((new_proc_val - *prev_proc_val) / cpu_usage * 100_f64); + *prev_proc_val = new_proc_val; + res } else { - Ok(( - (after_proc_val - before_proc_val) / cpu_usage * 100_f64 * cpu_fraction, - after_proc_val, - )) + let res = Ok((new_proc_val - *prev_proc_val) / cpu_usage * 100_f64 * cpu_fraction); + *prev_proc_val = new_proc_val; + res } } #[allow(clippy::too_many_arguments)] #[cfg(target_os = "linux")] -fn convert_ps( - process: &str, cpu_usage: f64, cpu_fraction: f64, - prev_pid_stats: &mut HashMap, - new_pid_stats: &mut HashMap, use_current_cpu_total: bool, - time_difference_in_secs: u64, mem_total_kb: u64, -) -> std::io::Result { - let pid = (&process[..10]) - .trim() - .to_string() - .parse::() - .unwrap_or(0); - let name = (&process[11..111]).trim().to_string(); - let mem_usage_percent = (&process[112..116]) - .trim() - .to_string() - .parse::() - .unwrap_or(0_f64); - let path = (&process[117..]).trim().to_string(); - - let mut new_pid_stat = if let Some(prev_proc_stats) = prev_pid_stats.remove(&pid) { - prev_proc_stats - } else { - PrevProcDetails::new(pid) - }; - - let (cpu_usage_percent, process_state_char, process_state) = - if let Ok(stat_results) = get_process_stats(&new_pid_stat.proc_stat_path) { - if let Some(tmp_split) = stat_results.split(')').collect::>().last() { - let proc_stats = tmp_split.split_whitespace().collect::>(); - let (process_state_char, process_state) = get_linux_process_state(&proc_stats); - - let (cpu_usage_percent, after_proc_val) = get_linux_cpu_usage( - &proc_stats, - cpu_usage, - cpu_fraction, - new_pid_stat.cpu_time, - use_current_cpu_total, - )?; - new_pid_stat.cpu_time = after_proc_val; - - (cpu_usage_percent, process_state_char, process_state) - } else { - (0.0, '?', String::new()) - } +fn read_proc( + pid: u32, cpu_usage: f64, cpu_fraction: f64, + pid_mapping: &mut HashMap, use_current_cpu_total: bool, + time_difference_in_secs: u64, mem_total_kb: u64, page_file_kb: u64, +) -> error::Result { + let pid_stat = pid_mapping + .entry(pid) + .or_insert_with(|| PrevProcDetails::new(pid)); + let stat_results = read_path_contents(&pid_stat.proc_stat_path)?; + let name = stat_results + .splitn(2, '(') + .collect::>() + .last() + .ok_or(BottomError::MinorError())? + .rsplitn(2, ')') + .collect::>() + .last() + .ok_or(BottomError::MinorError())? + .to_string(); + let command = { + let cmd = read_path_contents(&pid_stat.proc_cmdline_path)?; + if cmd.trim().is_empty() { + format!("[{}]", name) } else { - (0.0, '?', String::new()) - }; + cmd + } + }; + let stat = stat_results + .split(')') + .collect::>() + .last() + .ok_or(BottomError::MinorError())? + .split_whitespace() + .collect::>(); + let (process_state_char, process_state) = get_linux_process_state(&stat); + let cpu_usage_percent = get_linux_cpu_usage( + &stat, + cpu_usage, + cpu_fraction, + &mut pid_stat.cpu_time, + use_current_cpu_total, + )?; + let (_vsize, rss) = get_linux_process_vsize_rss(&stat); + let mem_usage_kb = rss * page_file_kb; + let mem_usage_percent = mem_usage_kb as f64 * 100.0 / mem_total_kb as f64; // This can fail if permission is denied! let (total_read_bytes, total_write_bytes, read_bytes_per_sec, write_bytes_per_sec) = - if let Ok(io_results) = get_process_io(&new_pid_stat.proc_io_path) { + if let Ok(io_results) = get_process_io(&pid_stat.proc_io_path) { let io_stats = io_results.split_whitespace().collect::>(); let (total_read_bytes, total_write_bytes) = get_linux_process_io_usage(&io_stats); let read_bytes_per_sec = if time_difference_in_secs == 0 { 0 } else { - total_read_bytes.saturating_sub(new_pid_stat.total_read_bytes) - / time_difference_in_secs + total_read_bytes.saturating_sub(pid_stat.total_read_bytes) / time_difference_in_secs }; let write_bytes_per_sec = if time_difference_in_secs == 0 { 0 } else { - total_write_bytes.saturating_sub(new_pid_stat.total_write_bytes) + total_write_bytes.saturating_sub(pid_stat.total_write_bytes) / time_difference_in_secs }; - new_pid_stat.total_read_bytes = total_read_bytes; - new_pid_stat.total_write_bytes = total_write_bytes; + pid_stat.total_read_bytes = total_read_bytes; + pid_stat.total_write_bytes = total_write_bytes; ( total_read_bytes, @@ -309,15 +315,12 @@ fn convert_ps( (0, 0, 0, 0) }; - new_pid_stats.insert(pid, new_pid_stat); - - // TODO: Is there a way to re-use these stats so I don't have to do so many syscalls? Ok(ProcessHarvest { pid, name, - path, + command, mem_usage_percent, - mem_usage_kb: (mem_usage_percent * mem_total_kb as f64 / 100.0) as u64, + mem_usage_kb, cpu_usage_percent, total_read_bytes, total_write_bytes, @@ -331,45 +334,35 @@ fn convert_ps( #[cfg(target_os = "linux")] pub fn linux_get_processes_list( prev_idle: &mut f64, prev_non_idle: &mut f64, - prev_pid_stats: &mut HashMap, use_current_cpu_total: bool, - time_difference_in_secs: u64, mem_total_kb: u64, + pid_mapping: &mut HashMap, use_current_cpu_total: bool, + time_difference_in_secs: u64, mem_total_kb: u64, page_file_kb: u64, ) -> crate::utils::error::Result> { - let ps_result = Command::new("ps") - .args(&["-axo", "pid:10,comm:100,%mem:5,args:100", "--noheader"]) - .output()?; - let ps_stdout = String::from_utf8_lossy(&ps_result.stdout); - let split_string = ps_stdout.split('\n'); if let Ok((cpu_usage, cpu_fraction)) = cpu_usage_calculation(prev_idle, prev_non_idle) { - let process_list = split_string.collect::>(); - let mut new_pid_stats = HashMap::new(); - - let process_vector: Vec = process_list - .iter() - .filter_map(|process| { - if process.trim().is_empty() { - None - } else if let Ok(process_object) = convert_ps( - process, - cpu_usage, - cpu_fraction, - prev_pid_stats, - &mut new_pid_stats, - use_current_cpu_total, - time_difference_in_secs, - mem_total_kb, - ) { - if !process_object.name.is_empty() { - Some(process_object) - } else { - None + let process_vector: Vec = std::fs::read_dir("/proc")? + .filter_map(|dir| { + if let Ok(dir) = dir { + let pid = dir.file_name().to_string_lossy().trim().parse::(); + if let Ok(pid) = pid { + // I skip checking if the path is also a directory, it's not needed I think? + if let Ok(process_object) = read_proc( + pid, + cpu_usage, + cpu_fraction, + pid_mapping, + use_current_cpu_total, + time_difference_in_secs, + mem_total_kb, + page_file_kb, + ) { + return Some(process_object); + } } - } else { - None } + + None }) .collect(); - *prev_pid_stats = new_pid_stats; Ok(process_vector) } else { Ok(Vec::new()) @@ -405,12 +398,12 @@ pub fn windows_macos_get_processes_list( } else { process_val.name().to_string() }; - let path = { - let path = process_val.cmd().join(" "); - if path.is_empty() { + let command = { + let command = process_val.cmd().join(" "); + if command.is_empty() { name.to_string() } else { - path + command } }; @@ -430,7 +423,7 @@ pub fn windows_macos_get_processes_list( process_vector.push(ProcessHarvest { pid: process_val.pid() as u32, name, - path, + command, mem_usage_percent: if mem_total_kb > 0 { process_val.memory() as f64 * 100.0 / mem_total_kb as f64 } else { diff --git a/src/app/process_killer.rs b/src/app/process_killer.rs index 05faf945..68754c39 100644 --- a/src/app/process_killer.rs +++ b/src/app/process_killer.rs @@ -35,6 +35,7 @@ impl Process { /// Kills a process, given a PID. pub fn kill_process_given_pid(pid: u32) -> crate::utils::error::Result<()> { if cfg!(target_os = "linux") || cfg!(target_os = "macos") { + // FIXME: Use libc instead of a command... let output = Command::new("kill").arg(pid.to_string()).output()?; if !(output.status).success() { return Err(BottomError::GenericError( diff --git a/src/app/query.rs b/src/app/query.rs index 41d18272..2a6d4cb4 100644 --- a/src/app/query.rs +++ b/src/app/query.rs @@ -674,6 +674,7 @@ impl Prefix { process.cpu_percent_usage, numerical_query.value, ), + // FIXME: This doesn't work with mem values! PrefixType::Mem => matches_condition( &numerical_query.condition, process.mem_percent_usage, diff --git a/src/data_conversion.rs b/src/data_conversion.rs index 0bf19d0b..d62b723b 100644 --- a/src/data_conversion.rs +++ b/src/data_conversion.rs @@ -398,7 +398,7 @@ pub fn convert_process_data( pid: process.pid, name: match name_type { ProcessNamingType::Name => process.name.to_string(), - ProcessNamingType::Path => process.path.to_string(), + ProcessNamingType::Path => process.command.to_string(), }, cpu_percent_usage: process.cpu_usage_percent, mem_percent_usage: process.mem_usage_percent, @@ -425,7 +425,7 @@ pub fn convert_process_data( let entry = grouped_hashmap .entry(match name_type { ProcessNamingType::Name => process.name.to_string(), - ProcessNamingType::Path => process.path.to_string(), + ProcessNamingType::Path => process.command.to_string(), }) .or_insert(SingleProcessData { pid: process.pid, diff --git a/src/lib.rs b/src/lib.rs index 53d7430b..6929e095 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,3 +1,8 @@ +#![warn(rust_2018_idioms)] +#[allow(unused_imports)] +#[macro_use] +extern crate log; + use std::{ boxed::Box, io::{stdout, Write}, diff --git a/src/utils/error.rs b/src/utils/error.rs index 5069cc78..4c607ebe 100644 --- a/src/utils/error.rs +++ b/src/utils/error.rs @@ -24,6 +24,8 @@ pub enum BottomError { ConversionError(String), /// An error to represent errors with querying. QueryError(Cow<'static, str>), + /// An error that just signifies something minor went wrong; no message. + MinorError(), } impl std::fmt::Display for BottomError { @@ -50,6 +52,7 @@ impl std::fmt::Display for BottomError { write!(f, "unable to convert: {}", message) } BottomError::QueryError(ref message) => write!(f, "{}", message), + BottomError::MinorError() => write!(f, "Minor error."), } } }