refactor: clean up some data collection refresh code, refresh sysinfo lists on loop (#1076)

* refactor: clean up some data collection refresh/init code

* move out battery

* missing setting memory value for proc

* stop segfault on macos

* unit

* oopsie

5 files changed, 70 insertions, 64 deletions

diff --git a/src/app/data_harvester.rs b/src/app/data_harvester.rs
index 0f1eee0a..4d8f7991 100644
--- a/src/app/data_harvester.rs
+++ b/src/app/data_harvester.rs
@@ -1,6 +1,6 @@
 //! This is the main file to house data collection functions.
 
-use std::time::Instant;
+use std::time::{Duration, Instant};
 
 #[cfg(target_os = "linux")]
 use fxhash::FxHashMap;
@@ -100,12 +100,6 @@ impl Data {
 pub struct DataCollector {
     pub data: Data,
     sys: System,
-    #[cfg(target_os = "linux")]
-    pid_mapping: FxHashMap<crate::Pid, processes::PrevProcDetails>,
-    #[cfg(target_os = "linux")]
-    prev_idle: f64,
-    #[cfg(target_os = "linux")]
-    prev_non_idle: f64,
     mem_total_kb: u64,
     temperature_type: TemperatureType,
     use_current_cpu_total: bool,
@@ -115,11 +109,19 @@ pub struct DataCollector {
     total_tx: u64,
     show_average_cpu: bool,
     widgets_to_harvest: UsedWidgets,
+    filters: DataFilters,
+
+    #[cfg(target_os = "linux")]
+    pid_mapping: FxHashMap<crate::Pid, processes::PrevProcDetails>,
+    #[cfg(target_os = "linux")]
+    prev_idle: f64,
+    #[cfg(target_os = "linux")]
+    prev_non_idle: f64,
+
     #[cfg(feature = "battery")]
     battery_manager: Option<Manager>,
     #[cfg(feature = "battery")]
     battery_list: Option<Vec<Battery>>,
-    filters: DataFilters,
 
     #[cfg(target_family = "unix")]
     user_table: self::processes::UserTable,
@@ -156,35 +158,6 @@ impl DataCollector {
     }
 
     pub fn init(&mut self) {
-        self.sys.refresh_memory();
-        self.mem_total_kb = self.sys.total_memory();
-
-        // Refresh network list once at the start.
-        if self.widgets_to_harvest.use_net {
-            self.sys.refresh_networks_list(); // TODO: refresh on a timer?
-        }
-
-        if self.widgets_to_harvest.use_proc || self.widgets_to_harvest.use_cpu {
-            self.sys.refresh_cpu();
-        }
-
-        #[cfg(not(target_os = "linux"))]
-        {
-            // Refresh components list once.
-            if self.widgets_to_harvest.use_temp {
-                self.sys.refresh_components_list(); // TODO: refresh on a timer?
-            }
-
-            if cfg!(target_os = "windows") && self.widgets_to_harvest.use_proc {
-                self.sys.refresh_users_list();
-            }
-
-            // Refresh disk list once...
-            if cfg!(target_os = "freebsd") && self.widgets_to_harvest.use_disk {
-                self.sys.refresh_disks_list();
-            }
-        }
-
         #[cfg(feature = "battery")]
         {
             if self.widgets_to_harvest.use_battery {
@@ -200,6 +173,17 @@ impl DataCollector {
             }
         }
 
+        if self.widgets_to_harvest.use_net {
+            self.sys.refresh_networks_list();
+        }
+        if self.widgets_to_harvest.use_temp {
+            self.sys.refresh_components_list();
+        }
+        #[cfg(target_os = "windows")]
+        if self.widgets_to_harvest.use_proc {
+            self.sys.refresh_users_list();
+        }
+
         futures::executor::block_on(self.update_data());
 
         std::thread::sleep(std::time::Duration::from_millis(250));
@@ -226,53 +210,64 @@ impl DataCollector {
         self.show_average_cpu = show_average_cpu;
     }
 
-    pub async fn update_data(&mut self) {
-        if self.widgets_to_harvest.use_proc || self.widgets_to_harvest.use_cpu {
+    /// Refresh sysinfo data.
+    fn refresh_sysinfo_data(&mut self) {
+        // Refresh once every minute. If it's too frequent it can cause segfaults.
+        const LIST_REFRESH_TIME: Duration = Duration::from_secs(60);
+        let refresh_start = Instant::now();
+
+        if self.widgets_to_harvest.use_cpu || self.widgets_to_harvest.use_proc {
             self.sys.refresh_cpu();
         }
 
-        if self.widgets_to_harvest.use_mem {
+        if self.widgets_to_harvest.use_mem || self.widgets_to_harvest.use_proc {
             self.sys.refresh_memory();
+            self.mem_total_kb = self.sys.total_memory() / 1024; // FIXME: This is sorta not really correct atm due to units, fix in future PR.
         }
 
         if self.widgets_to_harvest.use_net {
+            if refresh_start.duration_since(self.last_collection_time) > LIST_REFRESH_TIME {
+                self.sys.refresh_networks_list();
+            }
             self.sys.refresh_networks();
         }
 
         #[cfg(not(target_os = "linux"))]
         {
             if self.widgets_to_harvest.use_proc {
+                #[cfg(target_os = "windows")]
+                if refresh_start.duration_since(self.last_collection_time) > LIST_REFRESH_TIME {
+                    self.sys.refresh_users_list();
+                }
+
                 self.sys.refresh_processes();
             }
 
             if self.widgets_to_harvest.use_temp {
+                if refresh_start.duration_since(self.last_collection_time) > LIST_REFRESH_TIME {
+                    self.sys.refresh_components_list();
+                }
                 self.sys.refresh_components();
             }
         }
+    }
 
-        #[cfg(target_os = "freebsd")]
-        if self.widgets_to_harvest.use_disk {
-            self.sys.refresh_disks();
-        }
+    pub async fn update_data(&mut self) {
+        self.refresh_sysinfo_data();
 
         let current_instant = Instant::now();
 
         self.update_cpu_usage();
+        self.update_temps();
+        self.update_memory_usage();
         self.update_processes(
             #[cfg(target_os = "linux")]
             current_instant,
         );
-        self.update_temps();
-        self.update_memory_usage();
         self.update_network_usage(current_instant);
 
         #[cfg(feature = "battery")]
-        if let Some(battery_manager) = &self.battery_manager {
-            if let Some(battery_list) = &mut self.battery_list {
-                self.data.list_of_batteries =
-                    Some(batteries::refresh_batteries(battery_manager, battery_list));
-            }
-        }
+        self.update_batteries();
 
         let (disk_res, io_res) = futures::join!(
             disks::get_disk_usage(
@@ -348,7 +343,7 @@ impl DataCollector {
                             &self.sys,
                             self.use_current_cpu_total,
                             self.unnormalized_cpu,
-                            self.mem_total_kb,
+                            self.mem_total_kb * 1024,
                             &mut self.user_table,
                         )
                     }
@@ -358,7 +353,7 @@ impl DataCollector {
                             &self.sys,
                             self.use_current_cpu_total,
                             self.unnormalized_cpu,
-                            self.mem_total_kb,
+                            self.mem_total_kb * 1024,
                         )
                     }
                 }
@@ -431,6 +426,17 @@ impl DataCollector {
             self.data.network = Some(net_data);
         }
     }
+
+    #[inline]
+    #[cfg(feature = "battery")]
+    fn update_batteries(&mut self) {
+        if let Some(battery_manager) = &self.battery_manager {
+            if let Some(battery_list) = &mut self.battery_list {
+                self.data.list_of_batteries =
+                    Some(batteries::refresh_batteries(battery_manager, battery_list));
+            }
+        }
+    }
 }
 
 #[cfg(target_os = "freebsd")]
diff --git a/src/app/data_harvester/processes/freebsd.rs b/src/app/data_harvester/processes/freebsd.rs
index 4c5a9e45..f724510b 100644
--- a/src/app/data_harvester/processes/freebsd.rs
+++ b/src/app/data_harvester/processes/freebsd.rs
@@ -25,14 +25,14 @@ struct ProcessRow {
 }
 
 pub fn get_process_data(
-    sys: &System, use_current_cpu_total: bool, unnormalized_cpu: bool, mem_total_kb: u64,
+    sys: &System, use_current_cpu_total: bool, unnormalized_cpu: bool, mem_total: u64,
     user_table: &mut UserTable,
 ) -> crate::utils::error::Result<Vec<ProcessHarvest>> {
     super::macos_freebsd::get_process_data(
         sys,
         use_current_cpu_total,
         unnormalized_cpu,
-        mem_total_kb,
+        mem_total,
         user_table,
         get_freebsd_process_cpu_usage,
     )
diff --git a/src/app/data_harvester/processes/macos.rs b/src/app/data_harvester/processes/macos.rs
index a2b61b69..7679e3ba 100644
--- a/src/app/data_harvester/processes/macos.rs
+++ b/src/app/data_harvester/processes/macos.rs
@@ -7,14 +7,14 @@ use crate::{data_harvester::processes::UserTable, Pid};
 mod sysctl_bindings;
 
 pub fn get_process_data(
-    sys: &System, use_current_cpu_total: bool, unnormalized_cpu: bool, mem_total_kb: u64,
+    sys: &System, use_current_cpu_total: bool, unnormalized_cpu: bool, mem_total: u64,
     user_table: &mut UserTable,
 ) -> crate::utils::error::Result<Vec<ProcessHarvest>> {
     super::macos_freebsd::get_process_data(
         sys,
         use_current_cpu_total,
         unnormalized_cpu,
-        mem_total_kb,
+        mem_total,
         user_table,
         get_macos_process_cpu_usage,
     )
diff --git a/src/app/data_harvester/processes/macos_freebsd.rs b/src/app/data_harvester/processes/macos_freebsd.rs
index 251bc847..6e0cc786 100644
--- a/src/app/data_harvester/processes/macos_freebsd.rs
+++ b/src/app/data_harvester/processes/macos_freebsd.rs
@@ -9,7 +9,7 @@ use super::ProcessHarvest;
 use crate::{data_harvester::processes::UserTable, utils::error::Result, Pid};
 
 pub fn get_process_data<F>(
-    sys: &System, use_current_cpu_total: bool, unnormalized_cpu: bool, mem_total_kb: u64,
+    sys: &System, use_current_cpu_total: bool, unnormalized_cpu: bool, mem_total: u64,
     user_table: &mut UserTable, backup_cpu_proc_usage: F,
 ) -> Result<Vec<ProcessHarvest>>
 where
@@ -88,8 +88,8 @@ where
             },
             name,
             command,
-            mem_usage_percent: if mem_total_kb > 0 {
-                process_val.memory() as f64 * 100.0 / mem_total_kb as f64
+            mem_usage_percent: if mem_total > 0 {
+                process_val.memory() as f64 * 100.0 / mem_total as f64
             } else {
                 0.0
             },
diff --git a/src/app/data_harvester/processes/windows.rs b/src/app/data_harvester/processes/windows.rs
index 8d190d00..cbdcea5c 100644
--- a/src/app/data_harvester/processes/windows.rs
+++ b/src/app/data_harvester/processes/windows.rs
@@ -5,7 +5,7 @@ use sysinfo::{CpuExt, PidExt, ProcessExt, System, SystemExt, UserExt};
 use super::ProcessHarvest;
 
 pub fn get_process_data(
-    sys: &System, use_current_cpu_total: bool, unnormalized_cpu: bool, mem_total_kb: u64,
+    sys: &System, use_current_cpu_total: bool, unnormalized_cpu: bool, mem_total: u64,
 ) -> crate::utils::error::Result<Vec<ProcessHarvest>> {
     let mut process_vector: Vec<ProcessHarvest> = Vec::new();
     let process_hashmap = sys.processes();
@@ -63,8 +63,8 @@ pub fn get_process_data(
             parent_pid: process_val.parent().map(|p| p.as_u32() as _),
             name,
             command,
-            mem_usage_percent: if mem_total_kb > 0 {
-                process_val.memory() as f64 * 100.0 / mem_total_kb as f64
+            mem_usage_percent: if mem_total > 0 {
+                process_val.memory() as f64 * 100.0 / mem_total as f64
             } else {
                 0.0
             },