From ecefa302b9ef5320ad4cce0b606aca70a7b459e2 Mon Sep 17 00:00:00 2001 From: Jack Wills <32690432+mrjackwills@users.noreply.github.com> Date: Mon, 16 Jun 2025 20:54:40 +0000 Subject: [PATCH] refactor: reduce lines of log cloned Instead of cloning every single line of logs, now we only clone those logs that are visible +- a padding. Containers with hunders or thousands of lines of logs can see a huge reducing in CPU and memory usage --- src/app_data/container_state.rs | 21 +++++++++-- src/app_data/mod.rs | 64 +++++++++++++++++++++++++++++++-- src/ui/draw_blocks/logs.rs | 4 ++- 3 files changed, 82 insertions(+), 7 deletions(-) diff --git a/src/app_data/container_state.rs b/src/app_data/container_state.rs index 095c679..7a0e9bb 100644 --- a/src/app_data/container_state.rs +++ b/src/app_data/container_state.rs @@ -586,10 +586,25 @@ impl Logs { } } - pub fn to_vec(&self) -> Vec> { - self.logs.items.clone() + /// Get the logs vec, but instead of cloning to whole vec, only clone items with x of the currently selected index + /// Where x is the abs different of the index plus the panel height & a padding + /// The rest can be just empty list items + /// TODO test me, pass in 1000 lines of "something", expect response to be different! + pub fn to_vec(&self, height: usize, padding: usize) -> Vec> { + let current_index = self.logs.state.selected().unwrap_or_default(); + self.logs + .items + .iter() + .enumerate() + .map(|(index, item)| { + if current_index.abs_diff(index) <= height + padding { + item.clone() + } else { + ListItem::from("") + } + }) + .collect() } - /// The rest of the methods are basically forwarding from the underlying StatefulList pub fn get_state_title(&self) -> String { self.logs.get_state_title() diff --git a/src/app_data/mod.rs b/src/app_data/mod.rs index 8bf5ae3..943f67d 100644 --- a/src/app_data/mod.rs +++ b/src/app_data/mod.rs @@ -677,12 +677,12 @@ impl AppData { } /// Get mutable Vec of current containers logs - pub fn get_logs(&self) -> Vec> { + pub fn get_logs(&self, height: u16, padding: usize) -> Vec> { self.containers .state .selected() .and_then(|i| self.containers.items.get(i)) - .map_or(vec![], |i| i.logs.to_vec()) + .map_or(vec![], |i| i.logs.to_vec(height.into(), padding)) } /// Get mutable Option of the currently selected container Logs state @@ -1953,7 +1953,7 @@ mod tests { assert_eq!(result.as_ref().unwrap().selected(), Some(2)); assert_eq!(result.unwrap().offset(), 0); - let result = app_data.get_logs(); + let result = app_data.get_logs(4, 1); assert_eq!(result.len(), 3); let result = app_data.get_log_title(); @@ -2324,4 +2324,62 @@ mod tests { let result = app_data.get_log_state(); assert!(result.is_none()); } + + // *************** // + // Get logs method // + // *************** // + + #[test] + /// get_logs() returns vec of item, but the items are empty unless they are in the *visible" zone, based on height, index, and padding + fn test_app_data_update_get_logs() { + let (ids, containers) = gen_containers(); + + let mut app_data = gen_appdata(&containers); + + app_data.containers_start(); + let logs = (0..=999).map(|i| format!("{i} {i}")).collect::>(); + + app_data.update_log_by_id(logs, &ids[0]); + + let result = app_data.get_logs(10, 10); + for (index, item) in result.iter().enumerate() { + if index < 979 { + assert_eq!(item, &ListItem::new("")); + } else { + assert_eq!(item, &ListItem::new(format!("{index}"))); + } + } + + let result = app_data.get_logs(100, 20); + for (index, item) in result.iter().enumerate() { + if index < 879 { + assert_eq!(item, &ListItem::new("")); + } else { + assert_eq!(item, &ListItem::new(format!("{index}"))); + } + } + + app_data.log_start(); + let result = app_data.get_logs(10, 10); + for (index, item) in result.iter().enumerate() { + if index > 20 { + assert_eq!(item, &ListItem::new("")); + } else { + assert_eq!(item, &ListItem::new(format!("{index}"))); + } + } + + for _ in 0..=500 { + app_data.log_next(); + } + + let result = app_data.get_logs(10, 10); + for (index, item) in result.iter().enumerate() { + if (481..=521).contains(&index) { + assert_eq!(item, &ListItem::new(format!("{index}"))); + } else { + assert_eq!(item, &ListItem::new("")); + } + } + } } diff --git a/src/ui/draw_blocks/logs.rs b/src/ui/draw_blocks/logs.rs index 71acd1a..6381e5b 100644 --- a/src/ui/draw_blocks/logs.rs +++ b/src/ui/draw_blocks/logs.rs @@ -39,7 +39,8 @@ pub fn draw( } f.render_widget(paragraph, area); } else { - let logs = app_data.lock().get_logs(); + let padding = usize::from(area.height / 5); + let logs = app_data.lock().get_logs(area.height, padding); if logs.is_empty() { let mut paragraph = Paragraph::new("no logs found") .block(block) @@ -52,6 +53,7 @@ pub fn draw( let items = List::new(logs) .block(block) .highlight_symbol(RIGHT_ARROW) + .scroll_padding(padding) .highlight_style(Style::default().add_modifier(Modifier::BOLD)); // This should always return Some, as logs is not empty if let Some(log_state) = app_data.lock().get_log_state() {