From 435f428f3122d1d12f8ba858216dd3065b2d70aa Mon Sep 17 00:00:00 2001 From: Karthik Uthaman Date: Thu, 19 Dec 2024 23:08:55 -0800 Subject: [PATCH 01/19] Introduce ProcessRefreshKind::Thread --- src/common/system.rs | 3 +++ src/unix/linux/process.rs | 25 ++++++++++++++----------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/common/system.rs b/src/common/system.rs index 7bcadc9af..6a4a03c06 100644 --- a/src/common/system.rs +++ b/src/common/system.rs @@ -1848,6 +1848,7 @@ pub struct ProcessRefreshKind { environ: UpdateKind, cmd: UpdateKind, exe: UpdateKind, + thread: bool, } impl ProcessRefreshKind { @@ -1887,6 +1888,7 @@ impl ProcessRefreshKind { environ: UpdateKind::OnlyIfNotSet, cmd: UpdateKind::OnlyIfNotSet, exe: UpdateKind::OnlyIfNotSet, + thread: false, } } @@ -1929,6 +1931,7 @@ It will retrieve the following information: ); impl_get_set!(ProcessRefreshKind, cmd, with_cmd, without_cmd, UpdateKind); impl_get_set!(ProcessRefreshKind, exe, with_exe, without_exe, UpdateKind); + impl_get_set!(ProcessRefreshKind, thread, with_thread, without_thread); } /// Used to determine what you want to refresh specifically on the [`Cpu`] type. diff --git a/src/unix/linux/process.rs b/src/unix/linux/process.rs index e14e8427e..bc7a084da 100644 --- a/src/unix/linux/process.rs +++ b/src/unix/linux/process.rs @@ -660,6 +660,7 @@ fn get_all_pid_entries( parent_pid: Option, entry: DirEntry, data: &mut Vec, + refresh_kind: ProcessRefreshKind, ) -> Option { let Ok(file_type) = entry.file_type() else { return None; @@ -678,17 +679,19 @@ fn get_all_pid_entries( let name = name?; let pid = Pid::from(usize::from_str(name.to_str()?).ok()?); - let tasks_dir = Path::join(&entry, "task"); - - let tasks = if let Ok(entries) = fs::read_dir(tasks_dir) { - let mut tasks = HashSet::new(); - for task in entries - .into_iter() - .filter_map(|entry| get_all_pid_entries(Some(name), Some(pid), entry.ok()?, data)) - { - tasks.insert(task); + let tasks = if refresh_kind.thread() { + let tasks_dir = Path::join(&entry, "task"); + if let Ok(entries) = fs::read_dir(tasks_dir) { + let mut tasks = HashSet::new(); + for task in entries.into_iter().filter_map(|entry| { + get_all_pid_entries(Some(name), Some(pid), entry.ok()?, data, refresh_kind) + }) { + tasks.insert(task); + } + Some(tasks) + } else { + None } - Some(tasks) } else { None }; @@ -773,7 +776,7 @@ pub(crate) fn refresh_procs( .map(|entry| { let Ok(entry) = entry else { return Vec::new() }; let mut entries = Vec::new(); - get_all_pid_entries(None, None, entry, &mut entries); + get_all_pid_entries(None, None, entry, &mut entries, refresh_kind); entries }) .flatten() From a75f42e8332c603218f5421f7475ddf5588723c1 Mon Sep 17 00:00:00 2001 From: Karthik Uthaman Date: Tue, 28 Jan 2025 17:49:42 -0800 Subject: [PATCH 02/19] address comments --- src/common/system.rs | 2 +- src/unix/linux/process.rs | 14 ++++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/common/system.rs b/src/common/system.rs index 6a4a03c06..f516dddd1 100644 --- a/src/common/system.rs +++ b/src/common/system.rs @@ -1888,7 +1888,7 @@ impl ProcessRefreshKind { environ: UpdateKind::OnlyIfNotSet, cmd: UpdateKind::OnlyIfNotSet, exe: UpdateKind::OnlyIfNotSet, - thread: false, + thread: true, } } diff --git a/src/unix/linux/process.rs b/src/unix/linux/process.rs index bc7a084da..2e869934a 100644 --- a/src/unix/linux/process.rs +++ b/src/unix/linux/process.rs @@ -660,7 +660,7 @@ fn get_all_pid_entries( parent_pid: Option, entry: DirEntry, data: &mut Vec, - refresh_kind: ProcessRefreshKind, + enable_thread_stats: bool, ) -> Option { let Ok(file_type) = entry.file_type() else { return None; @@ -679,12 +679,18 @@ fn get_all_pid_entries( let name = name?; let pid = Pid::from(usize::from_str(name.to_str()?).ok()?); - let tasks = if refresh_kind.thread() { + let tasks = if enable_thread_stats { let tasks_dir = Path::join(&entry, "task"); if let Ok(entries) = fs::read_dir(tasks_dir) { let mut tasks = HashSet::new(); for task in entries.into_iter().filter_map(|entry| { - get_all_pid_entries(Some(name), Some(pid), entry.ok()?, data, refresh_kind) + get_all_pid_entries( + Some(name), + Some(pid), + entry.ok()?, + data, + enable_thread_stats, + ) }) { tasks.insert(task); } @@ -776,7 +782,7 @@ pub(crate) fn refresh_procs( .map(|entry| { let Ok(entry) = entry else { return Vec::new() }; let mut entries = Vec::new(); - get_all_pid_entries(None, None, entry, &mut entries, refresh_kind); + get_all_pid_entries(None, None, entry, &mut entries, refresh_kind.thread()); entries }) .flatten() From bd4cf6d81f3a7da0fdda2af2ef2551a6d8f62605 Mon Sep 17 00:00:00 2001 From: Karthik Uthaman Date: Tue, 28 Jan 2025 18:01:10 -0800 Subject: [PATCH 03/19] Make sure we add with_thread() in refresh_processes() --- src/common/system.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/common/system.rs b/src/common/system.rs index f516dddd1..195fb3a71 100644 --- a/src/common/system.rs +++ b/src/common/system.rs @@ -307,7 +307,8 @@ impl System { .with_memory() .with_cpu() .with_disk_usage() - .with_exe(UpdateKind::OnlyIfNotSet), + .with_exe(UpdateKind::OnlyIfNotSet) + .with_thread(), ) } From 6ef1e968ac6a6ae730c23a41cfd695120739b0f2 Mon Sep 17 00:00:00 2001 From: Karthik Uthaman Date: Tue, 28 Jan 2025 19:09:05 -0800 Subject: [PATCH 04/19] add test --- tests/process.rs | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/process.rs b/tests/process.rs index 4b64c7af1..c8e3efb5c 100644 --- a/tests/process.rs +++ b/tests/process.rs @@ -430,6 +430,7 @@ fn test_refresh_tasks() { let pid = Pid::from_u32(std::process::id() as _); // Checks that the task is listed as it should. + // refresh_processes by default refreshes tasks. let mut s = System::new(); s.refresh_processes(ProcessesToUpdate::All, false); @@ -467,6 +468,42 @@ fn test_refresh_tasks() { .is_none()); } +// Checks that `RefreshKind::Thread` when disabled doesnt get thread information. +#[test] +#[cfg(all( + any(target_os = "linux", target_os = "android"), + not(feature = "unknown-ci") +))] +fn test_refresh_thread() { + if !sysinfo::IS_SUPPORTED_SYSTEM || cfg!(feature = "apple-sandbox") { + return; + } + let task_name = "task_1_second"; + std::thread::Builder::new() + .name(task_name.into()) + .spawn(|| { + std::thread::sleep(std::time::Duration::from_secs(1)); + }) + .unwrap(); + + let pid = Pid::from_u32(std::process::id() as _); + + // Refresh everything but threads. + let mut s = System::new(); + s.refresh_processes_specifics( + ProcessesToUpdate::All, + false, + ProcessRefreshKind::everything().without_thread(), + ); + + // Check that we have an empty thread list. + assert!(s.process(pid).unwrap().tasks().is_none()); + assert!(s + .processes_by_exact_name(task_name.as_ref()) + .next() + .is_none()); +} + // Checks that `refresh_process` is removing dead processes when asked. #[test] fn test_refresh_process() { From d32056a4f56903a5e6e5a329987522f98ba17c34 Mon Sep 17 00:00:00 2001 From: Karthik Uthaman Date: Tue, 28 Jan 2025 19:20:34 -0800 Subject: [PATCH 05/19] documentation --- src/common/system.rs | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/common/system.rs b/src/common/system.rs index 195fb3a71..fcc623813 100644 --- a/src/common/system.rs +++ b/src/common/system.rs @@ -262,7 +262,7 @@ impl System { self.inner.refresh_cpu_specifics(refresh_kind) } - /// Gets all processes and updates their information. + /// Gets all processes and updates their information, along with all the threads/tasks each process has. /// /// It does the same as: /// @@ -276,7 +276,8 @@ impl System { /// .with_memory() /// .with_cpu() /// .with_disk_usage() - /// .with_exe(UpdateKind::OnlyIfNotSet), + /// .with_exe(UpdateKind::OnlyIfNotSet) + /// .with_thread(), /// ); /// ``` /// @@ -287,6 +288,11 @@ impl System { /// ⚠️ 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]. /// + /// ⚠️ On Linux, if you dont need the threads/tasks of each process, you can use + /// `refresh_processes_specifics` with `ProcessRefreshKind::everything().without_threads()`. + /// Refreshesing all processes and their threads can be quite expensive. For more information + /// see [`ProcessRefreshKind`]. + /// /// Example: /// /// ```no_run @@ -1819,6 +1825,16 @@ pub enum ProcessesToUpdate<'a> { /// the information won't be retrieved if the information is accessible without needing /// extra computation. /// +/// ⚠️ ** Linux Specific ** ⚠️ +/// When using `ProcessRefreshKind::everything()`, in linux we will fetch all relevant +/// information from `/proc//` as well as all the information from `/proc//task//` +/// dirs. This makes the refresh mechanism a lot slower depending on the number of threads +/// each process has. +/// +/// If you dont care about threads information, use `ProcessRefreshKind::everything().without_thread()` +/// as much as possible. +/// +/// In windows, this will not have any effect. /// ``` /// use sysinfo::{ProcessesToUpdate, ProcessRefreshKind, System}; /// From 890bacea8227c858c005033b83f9ba43356cab31 Mon Sep 17 00:00:00 2001 From: Karthik Uthaman Date: Tue, 28 Jan 2025 20:47:23 -0800 Subject: [PATCH 06/19] Make test_refresh_tasks deterministic --- tests/process.rs | 126 ++++++++++++++++++++++++++++++----------------- 1 file changed, 80 insertions(+), 46 deletions(-) diff --git a/tests/process.rs b/tests/process.rs index c8e3efb5c..88b3756b4 100644 --- a/tests/process.rs +++ b/tests/process.rs @@ -3,6 +3,7 @@ #![cfg(feature = "system")] use bstr::ByteSlice; +use std::{sync::mpsc, time::Duration}; use sysinfo::{Pid, ProcessRefreshKind, ProcessesToUpdate, RefreshKind, System, UpdateKind}; macro_rules! start_proc { @@ -411,61 +412,94 @@ fn test_refresh_process_doesnt_remove() { // Checks that `refresh_processes` is adding and removing task. #[test] -#[cfg(all( - any(target_os = "linux", target_os = "android"), - not(feature = "unknown-ci") -))] fn test_refresh_tasks() { + // Skip if unsupported. if !sysinfo::IS_SUPPORTED_SYSTEM || cfg!(feature = "apple-sandbox") { return; } - let task_name = "task_1_second"; + + // 1) Spawn a thread that waits on a channel, so we control when it exits. + let task_name = "controlled_test_thread"; + let (tx, rx) = mpsc::channel::<()>(); + std::thread::Builder::new() - .name(task_name.into()) - .spawn(|| { - std::thread::sleep(std::time::Duration::from_secs(1)); + .name(task_name.to_string()) + .spawn(move || { + // Wait until the main thread signals we can exit. + let _ = rx.recv(); }) .unwrap(); let pid = Pid::from_u32(std::process::id() as _); - - // Checks that the task is listed as it should. - // refresh_processes by default refreshes tasks. - let mut s = System::new(); - s.refresh_processes(ProcessesToUpdate::All, false); - - assert!(s - .process(pid) - .unwrap() - .tasks() - .map(|tasks| tasks.iter().any(|task_pid| s - .process(*task_pid) - .map(|task| task.name() == task_name) - .unwrap_or(false))) - .unwrap_or(false)); - assert!(s - .processes_by_exact_name(task_name.as_ref()) - .next() - .is_some()); - - // Let's give some time to the system to clean up... - std::thread::sleep(std::time::Duration::from_secs(2)); - - s.refresh_processes(ProcessesToUpdate::All, true); - - assert!(!s - .process(pid) - .unwrap() - .tasks() - .map(|tasks| tasks.iter().any(|task_pid| s - .process(*task_pid) - .map(|task| task.name() == task_name) - .unwrap_or(false))) - .unwrap_or(false)); - assert!(s - .processes_by_exact_name(task_name.as_ref()) - .next() - .is_none()); + let mut sys = System::new(); + + // Wait until the new thread shows up in the process/tasks list. + // We do a short loop and check each time by refreshing processes. + const MAX_POLLS: usize = 20; + const POLL_INTERVAL: Duration = Duration::from_millis(100); + + for _ in 0..MAX_POLLS { + println!("Waiting for thread start..."); + sys.refresh_processes(ProcessesToUpdate::All, /*refresh_users=*/ false); + + // Check if our thread is present in two ways: + // (a) via parent's tasks + // (b) by exact name + let parent_proc = sys.process(pid); + let tasks_contain_thread = parent_proc + .and_then(|p| p.tasks()) + .map(|tids| { + tids.iter().any(|tid| { + sys.process(*tid) + .map(|t| t.name() == task_name) + .unwrap_or(false) + }) + }) + .unwrap_or(false); + + let by_exact_name_exists = sys + .processes_by_exact_name(task_name.as_ref()) + .next() + .is_some(); + + if tasks_contain_thread && by_exact_name_exists { + // We confirmed the thread is now visible + break; + } + std::thread::sleep(POLL_INTERVAL); + } + + // 3) Signal the thread to exit. + drop(tx); + + // 4) Wait until the thread is gone from the system’s process/tasks list. + for _ in 0..MAX_POLLS { + println!("Waiting for thread stop..."); + sys.refresh_processes(ProcessesToUpdate::All, /*refresh_users=*/ true); + + let parent_proc = sys.process(pid as sysinfo::Pid); + let tasks_contain_thread = parent_proc + .and_then(|p| p.tasks()) + .map(|tids| { + tids.iter().any(|tid| { + sys.process(*tid) + .map(|t| t.name() == task_name) + .unwrap_or(false) + }) + }) + .unwrap_or(false); + + let by_exact_name_exists = sys + .processes_by_exact_name(task_name.as_ref()) + .next() + .is_some(); + + // If it's gone from both checks, we're good. + if !tasks_contain_thread && !by_exact_name_exists { + break; + } + std::thread::sleep(POLL_INTERVAL); + } } // Checks that `RefreshKind::Thread` when disabled doesnt get thread information. From 4f8e4e90278656b9d524bfbe964ad84cc7ea0dc7 Mon Sep 17 00:00:00 2001 From: Karthik Uthaman Date: Wed, 29 Jan 2025 10:40:44 -0800 Subject: [PATCH 07/19] rename thread to tasks --- src/common/system.rs | 10 +++++----- src/unix/linux/process.rs | 2 +- tests/process.rs | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/common/system.rs b/src/common/system.rs index fcc623813..7780cf1a3 100644 --- a/src/common/system.rs +++ b/src/common/system.rs @@ -277,7 +277,7 @@ impl System { /// .with_cpu() /// .with_disk_usage() /// .with_exe(UpdateKind::OnlyIfNotSet) - /// .with_thread(), + /// .with_tasks(), /// ); /// ``` /// @@ -314,7 +314,7 @@ impl System { .with_cpu() .with_disk_usage() .with_exe(UpdateKind::OnlyIfNotSet) - .with_thread(), + .with_tasks(), ) } @@ -1865,7 +1865,7 @@ pub struct ProcessRefreshKind { environ: UpdateKind, cmd: UpdateKind, exe: UpdateKind, - thread: bool, + tasks: bool, } impl ProcessRefreshKind { @@ -1905,7 +1905,7 @@ impl ProcessRefreshKind { environ: UpdateKind::OnlyIfNotSet, cmd: UpdateKind::OnlyIfNotSet, exe: UpdateKind::OnlyIfNotSet, - thread: true, + tasks: true, } } @@ -1948,7 +1948,7 @@ It will retrieve the following information: ); impl_get_set!(ProcessRefreshKind, cmd, with_cmd, without_cmd, UpdateKind); impl_get_set!(ProcessRefreshKind, exe, with_exe, without_exe, UpdateKind); - impl_get_set!(ProcessRefreshKind, thread, with_thread, without_thread); + impl_get_set!(ProcessRefreshKind, tasks, with_tasks, without_tasks); } /// Used to determine what you want to refresh specifically on the [`Cpu`] type. diff --git a/src/unix/linux/process.rs b/src/unix/linux/process.rs index 2e869934a..959ca061c 100644 --- a/src/unix/linux/process.rs +++ b/src/unix/linux/process.rs @@ -782,7 +782,7 @@ pub(crate) fn refresh_procs( .map(|entry| { let Ok(entry) = entry else { return Vec::new() }; let mut entries = Vec::new(); - get_all_pid_entries(None, None, entry, &mut entries, refresh_kind.thread()); + get_all_pid_entries(None, None, entry, &mut entries, refresh_kind.tasks()); entries }) .flatten() diff --git a/tests/process.rs b/tests/process.rs index 88b3756b4..b1fe9e462 100644 --- a/tests/process.rs +++ b/tests/process.rs @@ -527,7 +527,7 @@ fn test_refresh_thread() { s.refresh_processes_specifics( ProcessesToUpdate::All, false, - ProcessRefreshKind::everything().without_thread(), + ProcessRefreshKind::everything().without_tasks(), ); // Check that we have an empty thread list. From 788a0a38c26c1a5ce6c59410227ad97241b020e1 Mon Sep 17 00:00:00 2001 From: Karthik Uthaman Date: Wed, 29 Jan 2025 14:02:47 -0800 Subject: [PATCH 08/19] Impl Default for ProcessRefreshKind --- src/common/system.rs | 19 +++++++++++++- tests/process.rs | 62 +++++++++++++++++--------------------------- 2 files changed, 42 insertions(+), 39 deletions(-) diff --git a/src/common/system.rs b/src/common/system.rs index 7148eab89..90ce44518 100644 --- a/src/common/system.rs +++ b/src/common/system.rs @@ -1904,7 +1904,7 @@ pub enum ProcessesToUpdate<'a> { /// ``` /// /// [`Process`]: crate::Process -#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +#[derive(Clone, Copy, Debug, PartialEq, Eq)] pub struct ProcessRefreshKind { cpu: bool, disk_usage: bool, @@ -1918,6 +1918,23 @@ pub struct ProcessRefreshKind { tasks: bool, } +impl Default for ProcessRefreshKind { + fn default() -> Self { + Self { + cpu: false, + disk_usage: false, + memory: false, + user: UpdateKind::Never, + cwd: UpdateKind::Never, + root: UpdateKind::Never, + environ: UpdateKind::Never, + cmd: UpdateKind::Never, + exe: UpdateKind::Never, + tasks: true, // Process by default includes all tasks. + } + } +} + impl ProcessRefreshKind { /// Creates a new `ProcessRefreshKind` with every refresh set to `false`. /// diff --git a/tests/process.rs b/tests/process.rs index 44936e746..1a8b4a089 100644 --- a/tests/process.rs +++ b/tests/process.rs @@ -439,7 +439,6 @@ fn test_refresh_tasks() { const POLL_INTERVAL: Duration = Duration::from_millis(100); for _ in 0..MAX_POLLS { - println!("Waiting for thread start..."); sys.refresh_processes(ProcessesToUpdate::All, /*refresh_users=*/ false); // Check if our thread is present in two ways: @@ -469,12 +468,35 @@ fn test_refresh_tasks() { std::thread::sleep(POLL_INTERVAL); } + // At this point we know the thread is visible in the system's process/tasks list. + // Lets validate a few more things: + // * ProcessRefreshKind::nothing() should have thread information. + // * ProcessRefreshKind::nothing().with_tasks() should have thread information. + // * ProcessRefreshKind::nothing().without_tasks() should have thread information. + // * ProcessRefreshKind::everything() should have thread information. + // * ProcessRefreshKind::everything() should have thread information. + // * ProcessRefreshKind::everything().without_tasks() should not have thread information. + + let expectations = [ + (ProcessRefreshKind::nothing(), true), + (ProcessRefreshKind::nothing().with_tasks(), true), + (ProcessRefreshKind::nothing().without_tasks(), false), + (ProcessRefreshKind::everything(), true), + (ProcessRefreshKind::everything().with_tasks(), true), + (ProcessRefreshKind::everything().without_tasks(), false), + ]; + for (kind, expect_tasks) in expectations.iter() { + let mut sys_new = System::new(); + sys_new.refresh_processes_specifics(ProcessesToUpdate::All, true, *kind); + let proc = sys_new.process(pid).unwrap(); + assert_eq!(proc.tasks().is_some(), *expect_tasks); + } + // 3) Signal the thread to exit. drop(tx); // 4) Wait until the thread is gone from the system’s process/tasks list. for _ in 0..MAX_POLLS { - println!("Waiting for thread stop..."); sys.refresh_processes(ProcessesToUpdate::All, /*refresh_users=*/ true); let parent_proc = sys.process(pid as sysinfo::Pid); @@ -502,42 +524,6 @@ fn test_refresh_tasks() { } } -// Checks that `RefreshKind::Thread` when disabled doesnt get thread information. -#[test] -#[cfg(all( - any(target_os = "linux", target_os = "android"), - not(feature = "unknown-ci") -))] -fn test_refresh_thread() { - if !sysinfo::IS_SUPPORTED_SYSTEM || cfg!(feature = "apple-sandbox") { - return; - } - let task_name = "task_1_second"; - std::thread::Builder::new() - .name(task_name.into()) - .spawn(|| { - std::thread::sleep(std::time::Duration::from_secs(1)); - }) - .unwrap(); - - let pid = Pid::from_u32(std::process::id() as _); - - // Refresh everything but threads. - let mut s = System::new(); - s.refresh_processes_specifics( - ProcessesToUpdate::All, - false, - ProcessRefreshKind::everything().without_tasks(), - ); - - // Check that we have an empty thread list. - assert!(s.process(pid).unwrap().tasks().is_none()); - assert!(s - .processes_by_exact_name(task_name.as_ref()) - .next() - .is_none()); -} - // Checks that `refresh_process` is removing dead processes when asked. #[test] fn test_refresh_process() { From 93f9488321db21c614a3e8ed526bb56f2df71bf9 Mon Sep 17 00:00:00 2001 From: Karthik Uthaman Date: Wed, 29 Jan 2025 14:05:42 -0800 Subject: [PATCH 09/19] docs --- src/common/system.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/common/system.rs b/src/common/system.rs index 90ce44518..2879cabcf 100644 --- a/src/common/system.rs +++ b/src/common/system.rs @@ -1918,18 +1918,22 @@ pub struct ProcessRefreshKind { tasks: bool, } +/// Default implementation for [`ProcessRefreshKind`]. +/// Sets everything to default values except for `tasks` which is set to `true`. +/// This is because by default, a [`Process`] was fetching all tasks and we want to keep this +/// behavior. impl Default for ProcessRefreshKind { fn default() -> Self { Self { cpu: false, disk_usage: false, memory: false, - user: UpdateKind::Never, - cwd: UpdateKind::Never, - root: UpdateKind::Never, - environ: UpdateKind::Never, - cmd: UpdateKind::Never, - exe: UpdateKind::Never, + user: UpdateKind::default(), + cwd: UpdateKind::default(), + root: UpdateKind::default(), + environ: UpdateKind::default(), + cmd: UpdateKind::default(), + exe: UpdateKind::default(), tasks: true, // Process by default includes all tasks. } } From 306042c03c35fa932d20e149985a13fbfccc17ae Mon Sep 17 00:00:00 2001 From: Karthik Uthaman Date: Wed, 29 Jan 2025 14:43:57 -0800 Subject: [PATCH 10/19] Add cfg directive back --- tests/process.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/process.rs b/tests/process.rs index 1a8b4a089..b5ce54380 100644 --- a/tests/process.rs +++ b/tests/process.rs @@ -412,6 +412,10 @@ fn test_refresh_process_doesnt_remove() { // Checks that `refresh_processes` is adding and removing task. #[test] +#[cfg(all( + any(target_os = "linux", target_os = "android"), + not(feature = "unknown-ci") +))] fn test_refresh_tasks() { // Skip if unsupported. if !sysinfo::IS_SUPPORTED_SYSTEM || cfg!(feature = "apple-sandbox") { From 8fa146f24fb86052a2db218674eda1e3a816ad6b Mon Sep 17 00:00:00 2001 From: Karthik Uthaman Date: Wed, 29 Jan 2025 15:14:49 -0800 Subject: [PATCH 11/19] remove imports and use local resolution --- tests/process.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/process.rs b/tests/process.rs index b5ce54380..e2bc351cf 100644 --- a/tests/process.rs +++ b/tests/process.rs @@ -3,7 +3,6 @@ #![cfg(feature = "system")] use bstr::ByteSlice; -use std::{sync::mpsc, time::Duration}; use sysinfo::{Pid, ProcessRefreshKind, ProcessesToUpdate, RefreshKind, System, UpdateKind}; macro_rules! start_proc { @@ -424,7 +423,7 @@ fn test_refresh_tasks() { // 1) Spawn a thread that waits on a channel, so we control when it exits. let task_name = "controlled_test_thread"; - let (tx, rx) = mpsc::channel::<()>(); + let (tx, rx) = std::sync::mpsc::channel::<()>(); std::thread::Builder::new() .name(task_name.to_string()) @@ -440,7 +439,7 @@ fn test_refresh_tasks() { // Wait until the new thread shows up in the process/tasks list. // We do a short loop and check each time by refreshing processes. const MAX_POLLS: usize = 20; - const POLL_INTERVAL: Duration = Duration::from_millis(100); + const POLL_INTERVAL: std::time::Duration = std::time::Duration::from_millis(100); for _ in 0..MAX_POLLS { sys.refresh_processes(ProcessesToUpdate::All, /*refresh_users=*/ false); From 727e438f4d94e2a174d790f3df65fe6fb72d16dc Mon Sep 17 00:00:00 2001 From: sigsegved <1487102+sigsegved@users.noreply.github.com> Date: Thu, 30 Jan 2025 13:22:53 -0800 Subject: [PATCH 12/19] Update src/common/system.rs Co-authored-by: Guillaume Gomez --- src/common/system.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/system.rs b/src/common/system.rs index 2879cabcf..930c8aa5d 100644 --- a/src/common/system.rs +++ b/src/common/system.rs @@ -290,7 +290,7 @@ impl System { /// by using [`set_open_files_limit`][crate::set_open_files_limit]. /// /// ⚠️ On Linux, if you dont need the threads/tasks of each process, you can use - /// `refresh_processes_specifics` with `ProcessRefreshKind::everything().without_threads()`. + /// `refresh_processes_specifics` with `ProcessRefreshKind::everything().without_tasks()`. /// Refreshesing all processes and their threads can be quite expensive. For more information /// see [`ProcessRefreshKind`]. /// From 432db81700354176b1713727f7513c26ff91478f Mon Sep 17 00:00:00 2001 From: sigsegved <1487102+sigsegved@users.noreply.github.com> Date: Thu, 30 Jan 2025 13:28:05 -0800 Subject: [PATCH 13/19] Update src/unix/linux/process.rs Co-authored-by: Guillaume Gomez --- src/unix/linux/process.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/unix/linux/process.rs b/src/unix/linux/process.rs index 6f7eb353b..aa0a2566c 100644 --- a/src/unix/linux/process.rs +++ b/src/unix/linux/process.rs @@ -664,7 +664,7 @@ fn get_all_pid_entries( parent_pid: Option, entry: DirEntry, data: &mut Vec, - enable_thread_stats: bool, + enable_task_stats: bool, ) -> Option { let Ok(file_type) = entry.file_type() else { return None; From 01aa1e7eaa8bba5a9e451c367684a91b6c30933d Mon Sep 17 00:00:00 2001 From: Karthik Uthaman Date: Thu, 30 Jan 2025 14:04:50 -0800 Subject: [PATCH 14/19] Address feedback --- src/common/system.rs | 17 ++++++++--------- src/unix/linux/process.rs | 10 ++-------- 2 files changed, 10 insertions(+), 17 deletions(-) diff --git a/src/common/system.rs b/src/common/system.rs index 930c8aa5d..2383f4d67 100644 --- a/src/common/system.rs +++ b/src/common/system.rs @@ -263,7 +263,7 @@ impl System { self.inner.refresh_cpu_specifics(refresh_kind) } - /// Gets all processes and updates their information, along with all the threads/tasks each process has. + /// Gets all processes and updates their information, along with all the tasks each process has. /// /// It does the same as: /// @@ -278,7 +278,6 @@ impl System { /// .with_cpu() /// .with_disk_usage() /// .with_exe(UpdateKind::OnlyIfNotSet) - /// .with_tasks(), /// ); /// ``` /// @@ -289,9 +288,9 @@ impl System { /// ⚠️ 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]. /// - /// ⚠️ On Linux, if you dont need the threads/tasks of each process, you can use + /// ⚠️ On Linux, if you dont need the tasks of each process, you can use /// `refresh_processes_specifics` with `ProcessRefreshKind::everything().without_tasks()`. - /// Refreshesing all processes and their threads can be quite expensive. For more information + /// Refreshesing all processes and their tasks can be quite expensive. For more information /// see [`ProcessRefreshKind`]. /// /// Example: @@ -1878,13 +1877,12 @@ pub enum ProcessesToUpdate<'a> { /// ⚠️ ** Linux Specific ** ⚠️ /// When using `ProcessRefreshKind::everything()`, in linux we will fetch all relevant /// information from `/proc//` as well as all the information from `/proc//task//` -/// dirs. This makes the refresh mechanism a lot slower depending on the number of threads +/// dirs. This makes the refresh mechanism a lot slower depending on the number of tasks /// each process has. /// -/// If you dont care about threads information, use `ProcessRefreshKind::everything().without_thread()` +/// If you don't care about tasks information, use `ProcessRefreshKind::everything().without_thread()` /// as much as possible. /// -/// In windows, this will not have any effect. /// ``` /// use sysinfo::{ProcessesToUpdate, ProcessRefreshKind, System}; /// @@ -1940,8 +1938,9 @@ impl Default for ProcessRefreshKind { } impl ProcessRefreshKind { - /// Creates a new `ProcessRefreshKind` with every refresh set to `false`. - /// + /// Creates a new `ProcessRefreshKind` with every refresh set to `false`, except for `tasks` + /// This is because by default, [`Process`] was fetching all tasks and we want to keep this + /// behavior. /// ``` /// use sysinfo::{ProcessRefreshKind, UpdateKind}; /// diff --git a/src/unix/linux/process.rs b/src/unix/linux/process.rs index aa0a2566c..f6d53efe8 100644 --- a/src/unix/linux/process.rs +++ b/src/unix/linux/process.rs @@ -683,18 +683,12 @@ fn get_all_pid_entries( let name = name?; let pid = Pid::from(usize::from_str(name.to_str()?).ok()?); - let tasks = if enable_thread_stats { + let tasks = if enable_task_stats { let tasks_dir = Path::join(&entry, "task"); if let Ok(entries) = fs::read_dir(tasks_dir) { let mut tasks = HashSet::new(); for task in entries.into_iter().filter_map(|entry| { - get_all_pid_entries( - Some(name), - Some(pid), - entry.ok()?, - data, - enable_thread_stats, - ) + get_all_pid_entries(Some(name), Some(pid), entry.ok()?, data, enable_task_stats) }) { tasks.insert(task); } From 81cb272e11323f1298980dbb41d30a4e99b1d12e Mon Sep 17 00:00:00 2001 From: sigsegved <1487102+sigsegved@users.noreply.github.com> Date: Fri, 31 Jan 2025 11:54:17 -0800 Subject: [PATCH 15/19] Update src/common/system.rs Co-authored-by: Guillaume Gomez --- src/common/system.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/common/system.rs b/src/common/system.rs index 2383f4d67..49a04cad4 100644 --- a/src/common/system.rs +++ b/src/common/system.rs @@ -1938,9 +1938,10 @@ impl Default for ProcessRefreshKind { } impl ProcessRefreshKind { - /// Creates a new `ProcessRefreshKind` with every refresh set to `false`, except for `tasks` - /// This is because by default, [`Process`] was fetching all tasks and we want to keep this - /// behavior. + /// Creates a new `ProcessRefreshKind` with every refresh set to `false`, except for `tasks`. + /// By default, we want to list all processes and tasks are considered processes on their own + /// in linux so we still fetch them by default. However, the processes information are not + /// refreshed. /// ``` /// use sysinfo::{ProcessRefreshKind, UpdateKind}; /// From b7d9be46d514b87899a7122fc0232f6da1d33441 Mon Sep 17 00:00:00 2001 From: sigsegved <1487102+sigsegved@users.noreply.github.com> Date: Fri, 31 Jan 2025 11:54:43 -0800 Subject: [PATCH 16/19] Update src/common/system.rs Co-authored-by: Guillaume Gomez --- src/common/system.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/common/system.rs b/src/common/system.rs index 49a04cad4..1d07f1ca7 100644 --- a/src/common/system.rs +++ b/src/common/system.rs @@ -1916,10 +1916,10 @@ pub struct ProcessRefreshKind { tasks: bool, } -/// Default implementation for [`ProcessRefreshKind`]. -/// Sets everything to default values except for `tasks` which is set to `true`. -/// This is because by default, a [`Process`] was fetching all tasks and we want to keep this -/// behavior. + /// Creates a new `ProcessRefreshKind` with every refresh set to `false`, except for `tasks`. + /// By default, we want to list all processes and tasks are considered processes on their own + /// in linux so we still fetch them by default. However, the processes information are not + /// refreshed. impl Default for ProcessRefreshKind { fn default() -> Self { Self { From 3834c8222334dc257455117bf70b52b779d0eb2e Mon Sep 17 00:00:00 2001 From: Karthik Uthaman Date: Fri, 31 Jan 2025 11:54:37 -0800 Subject: [PATCH 17/19] PR feedback --- src/common/system.rs | 2 +- tests/process.rs | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/common/system.rs b/src/common/system.rs index 1d07f1ca7..ab8d4995b 100644 --- a/src/common/system.rs +++ b/src/common/system.rs @@ -1880,7 +1880,7 @@ pub enum ProcessesToUpdate<'a> { /// dirs. This makes the refresh mechanism a lot slower depending on the number of tasks /// each process has. /// -/// If you don't care about tasks information, use `ProcessRefreshKind::everything().without_thread()` +/// If you don't care about tasks information, use `ProcessRefreshKind::everything().without_tasks()` /// as much as possible. /// /// ``` diff --git a/tests/process.rs b/tests/process.rs index e2bc351cf..9ff8b3395 100644 --- a/tests/process.rs +++ b/tests/process.rs @@ -471,14 +471,14 @@ fn test_refresh_tasks() { std::thread::sleep(POLL_INTERVAL); } - // At this point we know the thread is visible in the system's process/tasks list. - // Lets validate a few more things: - // * ProcessRefreshKind::nothing() should have thread information. - // * ProcessRefreshKind::nothing().with_tasks() should have thread information. - // * ProcessRefreshKind::nothing().without_tasks() should have thread information. - // * ProcessRefreshKind::everything() should have thread information. - // * ProcessRefreshKind::everything() should have thread information. - // * ProcessRefreshKind::everything().without_tasks() should not have thread information. + // At this point we know the task is visible in the system's process/tasks list. + // Let's validate a few more things: + // * ProcessRefreshKind::nothing() should have task information. + // * ProcessRefreshKind::nothing().with_tasks() should have task information. + // * ProcessRefreshKind::nothing().without_tasks() shouldn't have task information. + // * ProcessRefreshKind::everything() should have task information. + // * ProcessRefreshKind::everything() should have task information. + // * ProcessRefreshKind::everything().without_tasks() should not have task information. let expectations = [ (ProcessRefreshKind::nothing(), true), From 1aa10c43d16ce5e8ae082198e42fa61475bc3c76 Mon Sep 17 00:00:00 2001 From: sigsegved <1487102+sigsegved@users.noreply.github.com> Date: Fri, 31 Jan 2025 14:43:00 -0800 Subject: [PATCH 18/19] Update src/common/system.rs Co-authored-by: Guillaume Gomez --- src/common/system.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/system.rs b/src/common/system.rs index ab8d4995b..f9e2d2f26 100644 --- a/src/common/system.rs +++ b/src/common/system.rs @@ -1877,7 +1877,7 @@ pub enum ProcessesToUpdate<'a> { /// ⚠️ ** Linux Specific ** ⚠️ /// When using `ProcessRefreshKind::everything()`, in linux we will fetch all relevant /// information from `/proc//` as well as all the information from `/proc//task//` -/// dirs. This makes the refresh mechanism a lot slower depending on the number of tasks +/// folders. This makes the refresh mechanism a lot slower depending on the number of tasks /// each process has. /// /// If you don't care about tasks information, use `ProcessRefreshKind::everything().without_tasks()` From ff0d741aa8221de15bb55cdb2f6e18b2e53c624b Mon Sep 17 00:00:00 2001 From: Karthik Uthaman Date: Fri, 31 Jan 2025 14:49:06 -0800 Subject: [PATCH 19/19] indent doc --- src/common/system.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/common/system.rs b/src/common/system.rs index f9e2d2f26..0c71f7a67 100644 --- a/src/common/system.rs +++ b/src/common/system.rs @@ -1916,10 +1916,10 @@ pub struct ProcessRefreshKind { tasks: bool, } - /// Creates a new `ProcessRefreshKind` with every refresh set to `false`, except for `tasks`. - /// By default, we want to list all processes and tasks are considered processes on their own - /// in linux so we still fetch them by default. However, the processes information are not - /// refreshed. +/// Creates a new `ProcessRefreshKind` with every refresh set to `false`, except for `tasks`. +/// By default, we want to list all processes and tasks are considered processes on their own +/// in linux so we still fetch them by default. However, the processes information are not +/// refreshed. impl Default for ProcessRefreshKind { fn default() -> Self { Self {