feat: add GPU process filter and improve sort stability (#117)

inureyes · web-flow · commit 28b0557ba970 · 2026-01-13T19:42:10.000+09:00
* feat: add GPU process filter and improve sort stability (#114) Add the ability to filter the process list to show only GPU processes (those with used_memory > 0) and improve sort stability to prevent processes from constantly shifting positions during monitoring. Changes: - Add gpu_filter_enabled flag to AppState for tracking filter state - Add 'f' keyboard shortcut to toggle GPU-only process filter - Apply GPU filter in the render pipeline before displaying processes - Update status bar to show [Filter:GPU] when filter is active - Use PID as secondary sort key in process sorting to ensure stable ordering when primary values are equal - Update help text to show new filter shortcut and filter status Closes #114 * fix: resolve GPU filter selection and performance issues This commit addresses two MEDIUM priority issues in PR #117: 1. Selection Index Inconsistency: Reset selected_process_index and start_index to 0 when enabling GPU filter to prevent out-of-bounds errors when the filtered list is shorter than the unfiltered list. This provides a clean UX by jumping to the top of the filtered list. 2. Memory Allocation Overhead: Use Cow<'_, [ProcessInfo]> pattern to eliminate unnecessary cloning. When filter is disabled, we now borrow the process list instead of cloning it, significantly reducing allocations on every render cycle. Changes: - event_handler.rs: Add index reset logic when toggling filter on - ui_loop.rs: Replace Vec clone with Cow for conditional ownership - app_state.rs: Auto-formatting changes only * chore: finalize PR with tests, docs, and lint fixes - Add comprehensive unit tests for GPU filter functionality - Add tests for sort stability with PID as secondary sort key - Update README with new 'f' key for GPU process filter - Update man page with complete keyboard shortcuts documentation - Fix clippy warnings in storage module by marking public API items as allowed unused (used by library consumers, not binary)
diff --git a/README.md b/README.md
@@ -299,6 +299,7 @@ http://gpu-node3:9090
   - Keyboard: Arrow keys, Page Up/Down, Tab switching
   - Mouse: Click column headers to sort (process view)
   - Sorting: 'd' (default), 'u' (utilization), 'g' (GPU memory), 'p' (PID), 'm' (memory), 'c' (CPU)
+  - Filtering: 'f' (toggle GPU process filter - show only processes with GPU memory usage)
   - Interface: '1'/'h' (help), 'q' (quit), ESC (close help)
 - **Visual Design:**
   - Color-coded status: Green (≤60%), Yellow (60-80%), Red (>80%)
diff --git a/docs/man/all-smi.1 b/docs/man/all-smi.1
@@ -127,7 +127,28 @@ Jump to the first/last item
 Refresh the display immediately
 .TP
 .B d
-Toggle debug log visibility
+Sort by default (hostname+index)
+.TP
+.B f
+Toggle GPU process filter (show only processes with GPU memory usage)
+.TP
+.B g
+Sort by GPU memory usage
+.TP
+.B m
+Sort by memory percentage
+.TP
+.B p
+Sort by PID
+.TP
+.B u
+Sort by utilization
+.TP
+.B c
+Toggle per-core CPU display
+.TP
+.B h / 1
+Toggle help screen
 .TP
 .B q, Ctrl+C
 Quit the application
diff --git a/src/app_state.rs b/src/app_state.rs
@@ -122,6 +122,8 @@ pub struct AppState {
     pub runtime_environment: RuntimeEnvironment,
     /// Version counter that increments when data changes, used to detect if re-render is needed
     pub data_version: u64,
+    /// Filter to show only GPU processes (processes with used_memory > 0)
+    pub gpu_filter_enabled: bool,
 }
 
 #[derive(Clone, Copy, PartialEq, Debug)]
@@ -213,6 +215,7 @@ impl AppState {
             is_local_mode: true, // Default to local mode
             runtime_environment: RuntimeEnvironment::detect(),
             data_version: 0,
+            gpu_filter_enabled: false, // GPU filter disabled by default
         }
     }
 
@@ -346,27 +349,42 @@ impl SortCriteria {
     ) -> Ordering {
         let base_ordering = match self {
             SortCriteria::Pid => a.pid.cmp(&b.pid),
-            SortCriteria::User => a.user.cmp(&b.user),
-            SortCriteria::Priority => a.priority.cmp(&b.priority),
-            SortCriteria::Nice => a.nice_value.cmp(&b.nice_value),
-            SortCriteria::VirtualMemory => a.memory_vms.cmp(&b.memory_vms),
-            SortCriteria::ResidentMemory => a.memory_rss.cmp(&b.memory_rss),
-            SortCriteria::State => a.state.cmp(&b.state),
+            SortCriteria::User => a.user.cmp(&b.user).then_with(|| a.pid.cmp(&b.pid)),
+            SortCriteria::Priority => a.priority.cmp(&b.priority).then_with(|| a.pid.cmp(&b.pid)),
+            SortCriteria::Nice => a
+                .nice_value
+                .cmp(&b.nice_value)
+                .then_with(|| a.pid.cmp(&b.pid)),
+            SortCriteria::VirtualMemory => a
+                .memory_vms
+                .cmp(&b.memory_vms)
+                .then_with(|| a.pid.cmp(&b.pid)),
+            SortCriteria::ResidentMemory => a
+                .memory_rss
+                .cmp(&b.memory_rss)
+                .then_with(|| a.pid.cmp(&b.pid)),
+            SortCriteria::State => a.state.cmp(&b.state).then_with(|| a.pid.cmp(&b.pid)),
             SortCriteria::CpuPercent => a
                 .cpu_percent
                 .partial_cmp(&b.cpu_percent)
-                .unwrap_or(Ordering::Equal),
+                .unwrap_or(Ordering::Equal)
+                .then_with(|| a.pid.cmp(&b.pid)),
             SortCriteria::MemoryPercent => a
                 .memory_percent
                 .partial_cmp(&b.memory_percent)
-                .unwrap_or(Ordering::Equal),
+                .unwrap_or(Ordering::Equal)
+                .then_with(|| a.pid.cmp(&b.pid)),
             SortCriteria::GpuPercent => a
                 .gpu_utilization
                 .partial_cmp(&b.gpu_utilization)
-                .unwrap_or(Ordering::Equal),
-            SortCriteria::GpuMemoryUsage => a.used_memory.cmp(&b.used_memory),
-            SortCriteria::CpuTime => a.cpu_time.cmp(&b.cpu_time),
-            SortCriteria::Command => a.command.cmp(&b.command),
+                .unwrap_or(Ordering::Equal)
+                .then_with(|| a.pid.cmp(&b.pid)),
+            SortCriteria::GpuMemoryUsage => a
+                .used_memory
+                .cmp(&b.used_memory)
+                .then_with(|| a.pid.cmp(&b.pid)),
+            SortCriteria::CpuTime => a.cpu_time.cmp(&b.cpu_time).then_with(|| a.pid.cmp(&b.pid)),
+            SortCriteria::Command => a.command.cmp(&b.command).then_with(|| a.pid.cmp(&b.pid)),
             // For GPU-related sorting or default, sort by PID
             _ => a.pid.cmp(&b.pid),
         };
@@ -397,4 +415,185 @@ mod tests {
         let default_state = AppState::new();
         assert!(default_state.is_local_mode);
     }
+
+    #[test]
+    fn test_gpu_filter_default() {
+        let state = AppState::new();
+        // GPU filter should be disabled by default
+        assert!(!state.gpu_filter_enabled);
+    }
+
+    #[test]
+    fn test_gpu_filter_toggle() {
+        let mut state = AppState::new();
+        assert!(!state.gpu_filter_enabled);
+
+        // Enable filter
+        state.gpu_filter_enabled = true;
+        assert!(state.gpu_filter_enabled);
+
+        // Disable filter
+        state.gpu_filter_enabled = false;
+        assert!(!state.gpu_filter_enabled);
+    }
+
+    #[test]
+    fn test_data_version_increment() {
+        let mut state = AppState::new();
+        let initial_version = state.data_version;
+
+        state.mark_data_changed();
+        assert_eq!(state.data_version, initial_version + 1);
+
+        state.mark_data_changed();
+        assert_eq!(state.data_version, initial_version + 2);
+    }
+
+    fn create_test_process(pid: u32, used_memory: u64) -> ProcessInfo {
+        ProcessInfo {
+            device_id: 0,
+            device_uuid: "test-uuid".to_string(),
+            pid,
+            used_memory,
+            process_name: format!("process_{pid}"),
+            user: "testuser".to_string(),
+            state: "S".to_string(),
+            command: format!("/usr/bin/process_{pid}"),
+            cpu_percent: 10.0,
+            memory_percent: 5.0,
+            gpu_utilization: 0.0,
+            priority: 20,
+            nice_value: 0,
+            memory_vms: 1024 * 1024,
+            memory_rss: 512 * 1024,
+            cpu_time: 100,
+            start_time: "00:00:00".to_string(),
+            ppid: 1,
+            threads: 1,
+            uses_gpu: used_memory > 0,
+        }
+    }
+
+    #[test]
+    fn test_sort_processes_by_pid_with_stability() {
+        // Test that sorting is stable - equal primary keys should be sorted by PID
+        let p1 = create_test_process(100, 1024);
+        let p2 = create_test_process(200, 1024);
+        let p3 = create_test_process(50, 1024);
+
+        let criteria = SortCriteria::GpuMemoryUsage;
+
+        // All have same GPU memory, so they should be sorted by PID as secondary key
+        // In descending order, higher PID comes first (reversed from ascending)
+        let ordering = criteria.sort_processes(&p1, &p2, SortDirection::Descending);
+        assert_eq!(
+            ordering,
+            Ordering::Greater,
+            "p1 (pid 100) should come after p2 (pid 200) in descending order"
+        );
+
+        // In ascending order, lower PID comes first
+        let ordering = criteria.sort_processes(&p3, &p1, SortDirection::Ascending);
+        assert_eq!(
+            ordering,
+            Ordering::Less,
+            "p3 (pid 50) should come before p1 (pid 100) in ascending order"
+        );
+    }
+
+    #[test]
+    fn test_sort_processes_by_gpu_memory() {
+        let p1 = create_test_process(100, 1024);
+        let p2 = create_test_process(200, 2048);
+
+        let criteria = SortCriteria::GpuMemoryUsage;
+
+        // In descending order, higher memory should come first
+        let ordering = criteria.sort_processes(&p1, &p2, SortDirection::Descending);
+        assert_eq!(
+            ordering,
+            Ordering::Greater,
+            "p1 (1024 MB) should come after p2 (2048 MB) in descending order"
+        );
+
+        // In ascending order, lower memory should come first
+        let ordering = criteria.sort_processes(&p1, &p2, SortDirection::Ascending);
+        assert_eq!(
+            ordering,
+            Ordering::Less,
+            "p1 (1024 MB) should come before p2 (2048 MB) in ascending order"
+        );
+    }
+
+    #[test]
+    fn test_sort_processes_by_cpu_percent_with_stability() {
+        let mut p1 = create_test_process(100, 0);
+        let mut p2 = create_test_process(200, 0);
+        let mut p3 = create_test_process(50, 0);
+
+        p1.cpu_percent = 50.0;
+        p2.cpu_percent = 50.0;
+        p3.cpu_percent = 50.0;
+
+        let criteria = SortCriteria::CpuPercent;
+
+        // All have same CPU%, so they should be sorted by PID as secondary key
+        // In ascending order, lower PID comes first
+        let ordering = criteria.sort_processes(&p1, &p2, SortDirection::Ascending);
+        assert_eq!(
+            ordering,
+            Ordering::Less,
+            "p1 (pid 100) should come before p2 (pid 200) when CPU% is equal (ascending)"
+        );
+
+        // In descending order, higher PID comes first (reversed)
+        let ordering = criteria.sort_processes(&p3, &p1, SortDirection::Descending);
+        assert_eq!(
+            ordering,
+            Ordering::Greater,
+            "p3 (pid 50) should come after p1 (pid 100) in descending order"
+        );
+    }
+
+    #[test]
+    fn test_sort_processes_multiple_criteria() {
+        let mut p1 = create_test_process(100, 1024);
+        let mut p2 = create_test_process(200, 2048);
+        let mut p3 = create_test_process(50, 1024);
+
+        p1.memory_percent = 10.0;
+        p2.memory_percent = 20.0;
+        p3.memory_percent = 10.0;
+
+        // Test MemoryPercent criteria
+        let criteria = SortCriteria::MemoryPercent;
+        let ordering = criteria.sort_processes(&p1, &p2, SortDirection::Descending);
+        assert_eq!(
+            ordering,
+            Ordering::Greater,
+            "p1 (10%) should come after p2 (20%) in descending order"
+        );
+
+        // p1 and p3 have same memory%, should be sorted by PID
+        // In descending order, the order is reversed: lower PID (p3=50) > higher PID (p1=100)
+        // So p1 (100) compared to p3 (50): base ordering = Less (100 > 50 in PID cmp)
+        // After reverse for descending: Greater
+        // Wait, let me think again:
+        // base_ordering: a.pid.cmp(&b.pid) where a=p1(100), b=p3(50) -> 100.cmp(&50) = Greater
+        // After reverse for descending: Less
+        let ordering = criteria.sort_processes(&p1, &p3, SortDirection::Descending);
+        assert_eq!(
+            ordering,
+            Ordering::Less,
+            "p1 (pid 100) should come before p3 (pid 50) in descending sort (reversed from ascending)"
+        );
+
+        // In ascending order, lower PID comes first
+        let ordering = criteria.sort_processes(&p1, &p3, SortDirection::Ascending);
+        assert_eq!(
+            ordering,
+            Ordering::Greater,
+            "p1 (pid 100) should come after p3 (pid 50) in ascending order"
+        );
+    }
 }
diff --git a/src/storage/mod.rs b/src/storage/mod.rs
@@ -12,9 +12,18 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+//! Storage monitoring module.
+//!
+//! This module provides storage/disk information reading capabilities
+//! for local system monitoring.
+
 pub mod info;
 pub mod reader;
 
-// Re-export commonly used items
+// Re-export commonly used items for the public library API.
+// These exports are used by the prelude module and external library users,
+// even though internal code may import from submodules directly.
+#[allow(unused_imports)]
 pub use info::StorageInfo;
+#[allow(unused_imports)]
 pub use reader::{create_storage_reader, LocalStorageReader, StorageReader};
diff --git a/src/storage/reader.rs b/src/storage/reader.rs
@@ -38,6 +38,7 @@ use crate::utils::{filter_docker_aware_disks, get_hostname};
 ///     }
 /// }
 /// ```
+#[allow(dead_code)] // Public API trait - used by library consumers
 pub trait StorageReader: Send + Sync {
     /// Get information about all detected storage devices.
     ///
@@ -69,6 +70,7 @@ pub trait StorageReader: Send + Sync {
 ///     println!("{}: {:.1}% used", storage.mount_point, usage_percent);
 /// }
 /// ```
+#[allow(dead_code)] // Public API struct - used by library consumers
 pub struct LocalStorageReader {
     hostname: String,
 }
@@ -77,6 +79,7 @@ impl LocalStorageReader {
     /// Create a new local storage reader.
     ///
     /// The hostname is cached at creation time to avoid repeated lookups.
+    #[allow(dead_code)] // Public API constructor - used by library consumers
     pub fn new() -> Self {
         Self {
             hostname: get_hostname(),
@@ -130,6 +133,7 @@ impl StorageReader for LocalStorageReader {
 /// let storage_info = reader.get_storage_info();
 /// println!("Found {} storage device(s)", storage_info.len());
 /// ```
+#[allow(dead_code)] // Public API factory function - used by library consumers
 pub fn create_storage_reader() -> Box<dyn StorageReader> {
     Box::new(LocalStorageReader::new())
 }
diff --git a/src/ui/chrome.rs b/src/ui/chrome.rs
@@ -134,14 +134,25 @@ pub fn print_function_keys<W: Write>(
         crate::app_state::SortCriteria::Temperature => "Sort:Temp",
     };
 
+    // Get GPU filter indicator
+    let filter_indicator = if state.gpu_filter_enabled {
+        "Filter:GPU"
+    } else {
+        ""
+    };
+
     let function_keys = if is_remote {
         // Remote mode: only GPU sorting
         format!(
             "h:Help q:Exit c:CPU Cores ←→:Tabs ↑↓:Scroll PgUp/PgDn:Page d:Default u:Util g:GPU-Mem [{sort_indicator}]"
         )
     } else {
         // Local mode: both process and GPU sorting
-        format!("h:Help q:Exit c:CPU Cores ←→:Tabs ↑↓:Scroll PgUp/PgDn:Page p:PID m:Memory d:Default u:Util g:GPU-Mem [{sort_indicator}]")
+        if state.gpu_filter_enabled {
+            format!("h:Help q:Exit c:CPU Cores f:Filter ←→:Scroll ↑↓:Scroll p:PID m:Memory g:GPU-Mem [{sort_indicator}] [{filter_indicator}]")
+        } else {
+            format!("h:Help q:Exit c:CPU Cores f:Filter ←→:Scroll ↑↓:Scroll p:PID m:Memory g:GPU-Mem [{sort_indicator}]")
+        }
     };
 
     let truncated_keys = if display_width(&function_keys) > cols as usize {
diff --git a/src/ui/help.rs b/src/ui/help.rs
@@ -175,6 +175,7 @@ fn render_shortcuts_section(
         ("Display Control:", "", "header"),
         ("  H", "Toggle this help screen", "shortcut"),
         ("  C", "Toggle per-core CPU display", "shortcut"),
+        ("  F", "Toggle GPU process filter", "shortcut"),
         ("  Q", "Exit application", "shortcut"),
         ("  ESC", "Close help or exit", "shortcut"),
         ("", "", ""),
@@ -236,6 +237,14 @@ fn render_shortcuts_section(
     let sort_status = get_current_sort_status(&state.sort_criteria);
     right_column.push(("  Sort mode:", &sort_status, "status"));
 
+    // Add filter status
+    let filter_status = if state.gpu_filter_enabled {
+        "GPU Only"
+    } else {
+        "All Processes"
+    };
+    right_column.push(("  Filter:", filter_status, "status"));
+
     // Handle special rows
     match line_idx {
         0 => center_text_colored("KEYBOARD SHORTCUTS & NAVIGATION", width, Color::Yellow),
diff --git a/src/ui/tabs.rs b/src/ui/tabs.rs
diff --git a/src/view/event_handler.rs b/src/view/event_handler.rs
diff --git a/src/view/ui_loop.rs b/src/view/ui_loop.rs
