diff --git a/CHANGELOG.md b/CHANGELOG.md index 482bf3fb..a6bf1f52 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,7 +20,7 @@ That said, these are more guidelines rather than hardset rules, though the proje --- -## [0.11.3]/[0.12.0] - Unreleased +## [0.11.3] - Unreleased ### Features @@ -30,6 +30,7 @@ That said, these are more guidelines rather than hardset rules, though the proje - [#1833](https://github.com/ClementTsang/bottom/pull/1833): Sort disk I/O using actual value rather than string representation. - [#1812](https://github.com/ClementTsang/bottom/pull/1812): Fix ARC collection on FreeBSD. +- [#1846](https://github.com/ClementTsang/bottom/pull/1846): Fix displayed average CPU value being wrong in graphs. ### Other diff --git a/src/app/data/store.rs b/src/app/data/store.rs index 4b98a3ce..bbc020be 100644 --- a/src/app/data/store.rs +++ b/src/app/data/store.rs @@ -29,7 +29,7 @@ pub struct StoredData { pub arc_harvest: Option, #[cfg(feature = "gpu")] pub gpu_harvest: Vec<(String, MemData)>, - pub cpu_harvest: Vec, + pub cpu_harvest: cpu::CpuHarvest, pub load_avg_harvest: cpu::LoadAvgHarvest, pub process_data: ProcessData, /// TODO: (points_rework_v1) Might be a better way to do this without having to store here? @@ -50,7 +50,7 @@ impl Default for StoredData { #[cfg(not(target_os = "windows"))] cache_harvest: None, swap_harvest: None, - cpu_harvest: Default::default(), + cpu_harvest: cpu::CpuHarvest::default(), load_avg_harvest: cpu::LoadAvgHarvest::default(), process_data: Default::default(), prev_io: Vec::default(), @@ -113,25 +113,7 @@ impl StoredData { } if let Some(cpu) = data.cpu { - self.cpu_harvest.clear(); - - if let Some(avg) = cpu.avg { - self.cpu_harvest.push(cpu::CpuData { - data_type: cpu::CpuDataType::Avg, - usage: avg, - }); - } - - self.cpu_harvest - .extend( - cpu.cpus - .into_iter() - .enumerate() - .map(|(core, usage)| cpu::CpuData { - data_type: cpu::CpuDataType::Cpu(core as u32), - usage, - }), - ); + self.cpu_harvest = cpu; } if let Some(load_avg) = data.load_avg { diff --git a/src/app/data/time_series.rs b/src/app/data/time_series.rs index fdbf2856..d93f2130 100644 --- a/src/app/data/time_series.rs +++ b/src/app/data/time_series.rs @@ -38,9 +38,6 @@ pub struct TimeSeriesData { /// CPU data. pub cpu: Vec, - /// Average CPU data. - pub avg_cpu: Values, - /// RAM memory data. pub ram: Values, @@ -73,12 +70,10 @@ impl TimeSeriesData { self.tx.insert_break(); } - if let Some(cpu_harvest) = &data.cpu { - let cpus = &cpu_harvest.cpus; - - match self.cpu.len().cmp(&cpus.len()) { + if let Some(cpu) = &data.cpu { + match self.cpu.len().cmp(&cpu.len()) { Ordering::Less => { - let diff = cpus.len() - self.cpu.len(); + let diff = cpu.len() - self.cpu.len(); self.cpu.reserve_exact(diff); for _ in 0..diff { @@ -86,7 +81,7 @@ impl TimeSeriesData { } } Ordering::Greater => { - let diff = self.cpu.len() - cpus.len(); + let diff = self.cpu.len() - cpu.len(); let offset = self.cpu.len() - diff; for curr in &mut self.cpu[offset..] { @@ -96,20 +91,13 @@ impl TimeSeriesData { Ordering::Equal => {} } - for (curr, new_data) in self.cpu.iter_mut().zip(cpus.iter()) { - curr.push((*new_data).into()); - } - - // If there isn't avg then we never had any to begin with. - if let Some(avg) = cpu_harvest.avg { - self.avg_cpu.push(avg.into()); + for (curr, new_data) in self.cpu.iter_mut().zip(cpu.iter()) { + curr.push(new_data.usage.into()); } } else { for c in &mut self.cpu { c.insert_break(); } - - self.avg_cpu.insert_break(); } if let Some(memory) = &data.memory { diff --git a/src/canvas/widgets/cpu_basic.rs b/src/canvas/widgets/cpu_basic.rs index cd4ecf02..8d2a9bda 100644 --- a/src/canvas/widgets/cpu_basic.rs +++ b/src/canvas/widgets/cpu_basic.rs @@ -158,8 +158,7 @@ impl Painter { CpuDataType::Avg => ("AVG".to_string(), self.styles.avg_cpu_colour), CpuDataType::Cpu(index) => ( format!("{index:<3}",), - self.styles.cpu_colour_styles - [(index as usize) % self.styles.cpu_colour_styles.len()], + self.styles.cpu_colour_styles[index % self.styles.cpu_colour_styles.len()], ), }; diff --git a/src/canvas/widgets/cpu_graph.rs b/src/canvas/widgets/cpu_graph.rs index 1140409f..06e82706 100644 --- a/src/canvas/widgets/cpu_graph.rs +++ b/src/canvas/widgets/cpu_graph.rs @@ -119,6 +119,7 @@ impl Painter { fn generate_points<'a>( &self, cpu_widget_state: &'a CpuWidgetState, data: &'a StoredData, show_avg_cpu: bool, ) -> Vec> { + let show_avg_offset = if show_avg_cpu { AVG_POSITION } else { 0 }; let current_scroll_position = cpu_widget_state.table.state.current_index; let cpu_entries = &data.cpu_harvest; let cpu_points = &data.timeseries_data.cpu; @@ -127,59 +128,40 @@ impl Painter { if current_scroll_position == ALL_POSITION { // This case ensures the other cases cannot have the position be equal to 0. - let capacity = if show_avg_cpu { - cpu_points.len() + 1 - } else { - cpu_points.len() - }; - let mut points = Vec::with_capacity(capacity); + cpu_points + .iter() + .enumerate() + .map(|(itx, values)| { + let style = if show_avg_cpu && itx == AVG_POSITION { + self.styles.avg_cpu_colour + } else if itx == ALL_POSITION { + self.styles.all_cpu_colour + } else { + self.styles.cpu_colour_styles + [(itx - show_avg_offset) % self.styles.cpu_colour_styles.len()] + }; - points.extend(cpu_points.iter().enumerate().map(|(itx, values)| { - let style_index = itx % self.styles.cpu_colour_styles.len(); - let style = self.styles.cpu_colour_styles[style_index]; - - GraphData::default().style(style).time(time).values(values) - })); - - // We draw avg last so it is drawn on top. - if show_avg_cpu { - points.push( - GraphData::default() - .style(self.styles.avg_cpu_colour) - .time(time) - .values(&data.timeseries_data.avg_cpu), - ); - } - - points + GraphData::default().style(style).time(time).values(values) + }) + .rev() + .collect() } else if let Some(CpuData { .. }) = cpu_entries.get(current_scroll_position - 1) { // We generally subtract one from current scroll position because of the all entry. - let show_avg_offset = if show_avg_cpu { AVG_POSITION } else { 0 }; - let is_avg = show_avg_cpu && current_scroll_position == AVG_POSITION; - - let style = if is_avg { + let style = if show_avg_cpu && current_scroll_position == AVG_POSITION { self.styles.avg_cpu_colour } else { - self.styles.cpu_colour_styles[(current_scroll_position - 1 - show_avg_offset) - % self.styles.cpu_colour_styles.len()] + let offset_position = current_scroll_position - 1; + self.styles.cpu_colour_styles + [(offset_position - show_avg_offset) % self.styles.cpu_colour_styles.len()] }; - if is_avg { - vec![ - GraphData::default() - .style(style) - .time(time) - .values(&data.timeseries_data.avg_cpu), - ] - } else { - vec![ - GraphData::default() - .style(style) - .time(time) - .values(&cpu_points[current_scroll_position - 1 - show_avg_offset]), - ] - } + vec![ + GraphData::default() + .style(style) + .time(time) + .values(&cpu_points[current_scroll_position - 1]), + ] } else { vec![] } diff --git a/src/collection/cpu.rs b/src/collection/cpu.rs index 2d288451..7441c3bc 100644 --- a/src/collection/cpu.rs +++ b/src/collection/cpu.rs @@ -8,7 +8,7 @@ pub type LoadAvgHarvest = [f32; 3]; #[derive(Debug, Clone, Copy)] pub enum CpuDataType { Avg, - Cpu(u32), + Cpu(usize), } #[derive(Debug, Clone)] @@ -17,8 +17,4 @@ pub struct CpuData { pub usage: f32, } -#[derive(Debug, Clone, Default)] -pub struct CpuHarvest { - pub avg: Option, - pub cpus: Vec, -} +pub type CpuHarvest = Vec; diff --git a/src/collection/cpu/sysinfo.rs b/src/collection/cpu/sysinfo.rs index 2693fa99..acf4e752 100644 --- a/src/collection/cpu/sysinfo.rs +++ b/src/collection/cpu/sysinfo.rs @@ -3,19 +3,31 @@ use sysinfo::System; -use super::CpuHarvest; +use super::{CpuData, CpuDataType, CpuHarvest}; use crate::collection::error::CollectionResult; pub fn get_cpu_data_list(sys: &System, show_average_cpu: bool) -> CollectionResult { - let avg = show_average_cpu.then(|| sys.global_cpu_usage()); + let mut cpus = vec![]; - let cpus = sys - .cpus() - .iter() - .map(|cpu| cpu.cpu_usage()) - .collect::>(); + if show_average_cpu { + cpus.push(CpuData { + data_type: CpuDataType::Avg, + usage: sys.global_cpu_usage(), + }) + } - Ok(CpuHarvest { avg, cpus }) + cpus.extend( + sys.cpus() + .iter() + .enumerate() + .map(|(i, cpu)| CpuData { + data_type: CpuDataType::Cpu(i), + usage: cpu.cpu_usage(), + }) + .collect::>(), + ); + + Ok(cpus) } #[cfg(target_family = "unix")] diff --git a/src/widgets/cpu_graph.rs b/src/widgets/cpu_graph.rs index af371fd9..75dfa04d 100644 --- a/src/widgets/cpu_graph.rs +++ b/src/widgets/cpu_graph.rs @@ -102,7 +102,6 @@ impl DataToCell for CpuWidgetTableData { } => match data_type { CpuDataType::Avg => painter.styles.avg_cpu_colour, CpuDataType::Cpu(index) => { - let index = *index as usize; painter.styles.cpu_colour_styles[index % painter.styles.cpu_colour_styles.len()] } },