refactor: optimize username cloning for Unix processes (#1816)

* refactor: rename/update comments for uid_to_username

* use arc<str> instead of cloning user repeatedly

* clippy

* oop

* oop2

* hmmm hopefully that works?

* ugh

src/collection/processes.rs

@@ -32,7 +32,7 @@ cfg_if! {
     }
 }
 
-use std::{borrow::Cow, time::Duration};
+use std::{sync::Arc, time::Duration};
 
 use super::{DataCollector, error::CollectionResult};
 
@@ -128,7 +128,7 @@ pub struct ProcessHarvest {
     pub uid: Option<libc::uid_t>,
 
     /// This is the process' user.
-    pub user: Cow<'static, str>,
+    pub user: Option<Arc<str>>,
 
     /// Gpu memory usage as bytes.
     #[cfg(feature = "gpu")]

src/collection/processes/linux/mod.rs

@@ -201,14 +201,7 @@ fn read_proc(
         (0, 0, 0, 0)
     };
 
-    let user = uid
+    let user = uid.and_then(|uid| user_table.uid_to_username(uid).ok());
-        .and_then(|uid| {
-            user_table
-                .get_uid_to_username_mapping(uid)
-                .map(Into::into)
-                .ok()
-        })
-        .unwrap_or_else(|| "N/A".into());
 
     let time = if let Ok(ticks_per_sec) = u32::try_from(rustix::param::clock_ticks_per_second()) {
         if ticks_per_sec == 0 {

src/collection/processes/unix/process_ext.rs

@@ -88,14 +88,7 @@ pub(crate) trait UnixProcessExt {
                 total_write: disk_usage.total_written_bytes,
                 process_state,
                 uid,
-                user: uid
+                user: uid.and_then(|uid| user_table.uid_to_username(uid).ok()),
-                    .and_then(|uid| {
-                        user_table
-                            .get_uid_to_username_mapping(uid)
-                            .map(Into::into)
-                            .ok()
-                    })
-                    .unwrap_or_else(|| "N/A".into()),
                 time: if process_val.start_time() == 0 {
                     // Workaround for sysinfo occasionally returning a start time equal to UNIX
                     // epoch, giving a run time in the range of 50+ years. We just

src/collection/processes/unix/user_table.rs

@@ -1,28 +1,33 @@
+use std::sync::Arc;
+
 use hashbrown::HashMap;
 
 use crate::collection::error::{CollectionError, CollectionResult};
 
 #[derive(Debug, Default)]
 pub struct UserTable {
-    pub uid_user_mapping: HashMap<libc::uid_t, String>,
+    pub uid_user_mapping: HashMap<libc::uid_t, Arc<str>>,
 }
 
 impl UserTable {
-    pub fn get_uid_to_username_mapping(&mut self, uid: libc::uid_t) -> CollectionResult<String> {
+    /// Get the username associated with a UID. On first access of a name, it will
+    /// be cached for future accesses.
+    pub fn uid_to_username(&mut self, uid: libc::uid_t) -> CollectionResult<Arc<str>> {
         if let Some(user) = self.uid_user_mapping.get(&uid) {
             Ok(user.clone())
         } else {
-            // SAFETY: getpwuid returns a null pointer if no passwd entry is found for the uid.
+            // SAFETY: getpwuid returns a null pointer if no passwd entry is found for the uid which we check.
             let passwd = unsafe { libc::getpwuid(uid) };
 
             if passwd.is_null() {
                 Err("passwd is inaccessible".into())
             } else {
                 // SAFETY: We return early if passwd is null.
-                let username = unsafe { std::ffi::CStr::from_ptr((*passwd).pw_name) }
+                let username: Arc<str> = unsafe { std::ffi::CStr::from_ptr((*passwd).pw_name) }
                     .to_str()
                     .map_err(|err| CollectionError::General(err.into()))?
-                    .to_string();
+                    .into();
+
                 self.uid_user_mapping.insert(uid, username.clone());
 
                 Ok(username)

src/collection/processes/windows.rs

@@ -108,8 +108,7 @@ pub fn sysinfo_process_data(
             process_state,
             user: process_val
                 .user_id()
-                .and_then(|uid| users.get_user_by_id(uid))
+                .and_then(|uid| users.get_user_by_id(uid).map(|user| user.name().into())),
-                .map_or_else(|| "N/A".into(), |user| user.name().to_owned().into()),
             time: if process_val.start_time() == 0 {
                 // Workaround for sysinfo occasionally returning a start time equal to UNIX
                 // epoch, giving a run time in the range of 50+ years. We just

src/widgets/process_table.rs

@@ -1188,9 +1188,9 @@ mod test {
             process_state: "N/A",
             process_char: '?',
             #[cfg(target_family = "unix")]
-            user: "root".to_string(),
+            user: Some("root".into()),
             #[cfg(not(target_family = "unix"))]
-            user: "N/A".to_string(),
+            user: Some("N/A".into()),
             num_similar: 0,
             disabled: false,
             time: Duration::from_secs(0),

src/widgets/process_table/process_columns.rs

@@ -151,16 +151,17 @@ impl SortsRow for ProcColumn {
             }
             ProcColumn::State => {
                 if descending {
-                    data.sort_by_cached_key(|pd| Reverse(pd.process_state.to_lowercase()));
+                    data.sort_by_cached_key(|pd| Reverse(pd.process_state));
                 } else {
-                    data.sort_by_cached_key(|pd| pd.process_state.to_lowercase());
+                    data.sort_by_cached_key(|pd| pd.process_state);
                 }
             }
             ProcColumn::User => {
+                // FIXME: Is there a better way here to keep the to_lowercase? Usually it shouldn't matter but...
                 if descending {
-                    data.sort_by_cached_key(|pd| Reverse(pd.user.to_lowercase()));
+                    data.sort_by_cached_key(|pd| Reverse(pd.user.clone()));
                 } else {
-                    data.sort_by_cached_key(|pd| pd.user.to_lowercase());
+                    data.sort_by_cached_key(|pd| pd.user.clone());
                 }
             }
             ProcColumn::Time => {

src/widgets/process_table/process_data.rs

@@ -3,6 +3,7 @@ use std::{
     cmp::{Ordering, max},
     fmt::Display,
     num::NonZeroU16,
+    sync::Arc,
     time::Duration,
 };
 
@@ -206,7 +207,7 @@ pub struct ProcWidgetData {
     pub total_write: u64,
     pub process_state: &'static str,
     pub process_char: char,
-    pub user: String,
+    pub user: Option<Arc<str>>,
     pub num_similar: u64,
     pub disabled: bool,
     pub time: Duration,
@@ -249,7 +250,7 @@ impl ProcWidgetData {
             total_write: process.total_write,
             process_state: process.process_state.0,
             process_char: process.process_state.1,
-            user: process.user.to_string(),
+            user: process.user.clone(),
             num_similar: 1,
             disabled: false,
             time: process.time,
@@ -318,7 +319,11 @@ impl ProcWidgetData {
             ProcColumn::TotalRead => dec_bytes_string(self.total_read),
             ProcColumn::TotalWrite => dec_bytes_string(self.total_write),
             ProcColumn::State => self.process_char.to_string(),
-            ProcColumn::User => self.user.clone(),
+            ProcColumn::User => self
+                .user
+                .as_ref()
+                .map(|user| user.to_string())
+                .unwrap_or_else(|| "N/A".to_string()),
             ProcColumn::Time => format_time(self.time),
             #[cfg(feature = "gpu")]
             ProcColumn::GpuMemValue | ProcColumn::GpuMemPercent => self.gpu_mem_usage.to_string(),
@@ -355,7 +360,11 @@ impl DataToCell<ProcColumn> for ProcWidgetData {
                     self.process_state.into()
                 }
             }
-            ProcColumn::User => self.user.clone().into(),
+            ProcColumn::User => self
+                .user
+                .as_ref()
+                .map(|user| user.to_string().into())
+                .unwrap_or_else(|| "N/A".into()),
             ProcColumn::Time => format_time(self.time).into(),
             #[cfg(feature = "gpu")]
             ProcColumn::GpuMemValue | ProcColumn::GpuMemPercent => {

src/widgets/process_table/query.rs

@@ -815,7 +815,10 @@ impl Prefix {
                     }),
                     PrefixType::Pid => r.is_match(process.pid.to_string().as_str()),
                     PrefixType::State => r.is_match(process.process_state.0),
-                    PrefixType::User => r.is_match(process.user.as_ref()),
+                    PrefixType::User => match process.user.as_ref() {
+                        Some(user) => r.is_match(user),
+                        None => r.is_match("N/A"),
+                    },
                     _ => true,
                 }
             } else {