From 8b2b3327b3e85ec22cc3151431c4056fc05a5aee Mon Sep 17 00:00:00 2001 From: Clement Tsang <34804052+ClementTsang@users.noreply.github.com> Date: Wed, 15 Jul 2026 04:11:49 -0400 Subject: [PATCH] bug: fix first disk I/O value reported at startup being incorrect (#2152) Because of the way I was initializing things, this caused it to report a huge spike at the start since it was doing a subtraction against 0. This fixes it by just using a hashmap (which also fixed another FIXME) and initializing the prev comparison structure on first view + skipping the first instance. --- CHANGELOG.md | 1 + src/app/data/store.rs | 121 +++++++++++++++++++++++++++++------------- 2 files changed, 85 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e3ae135..cbbfdf29 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ That said, these are more guidelines rather than hard rules, though the project - [#2145](https://github.com/ClementTsang/bottom/pull/2145): Fix parsing issue certain comm entries to be missing. - [#2146](https://github.com/ClementTsang/bottom/pull/2146): Fix draw bug with the pipe gauge in basic mode if the value was 100%. - [#2150](https://github.com/ClementTsang/bottom/pull/2150): Fix missing deserialize options for read/write columns in the disk widget. +- [#2152](https://github.com/ClementTsang/bottom/pull/2152): Fix first disk I/O value reported at startup being way too high. ## 0.14.4 - 2026-07-09 diff --git a/src/app/data/store.rs b/src/app/data/store.rs index 7e37ec5d..401541f7 100644 --- a/src/app/data/store.rs +++ b/src/app/data/store.rs @@ -1,8 +1,12 @@ use std::{ + borrow::Borrow, + hash::{Hash, Hasher}, time::{Duration, Instant}, vec::Vec, }; +use rustc_hash::FxHashMap; + use super::{ProcessData, TimeSeriesData}; #[cfg(feature = "battery")] use crate::collection::batteries; @@ -19,6 +23,43 @@ use crate::{ widgets::{DiskWidgetData, TempWidgetData}, }; +/// Because otherwise you can't do lookups for something like `(String, String)` as a key. +trait PairKey { + fn pair(&self) -> (&str, &str); +} + +impl PairKey for (String, String) { + fn pair(&self) -> (&str, &str) { + (&self.0, &self.1) + } +} + +impl PairKey for (&str, &str) { + fn pair(&self) -> (&str, &str) { + *self + } +} + +impl<'a> Borrow for (String, String) { + fn borrow(&self) -> &(dyn PairKey + 'a) { + self + } +} + +impl Hash for dyn PairKey + '_ { + fn hash(&self, state: &mut H) { + self.pair().hash(state) + } +} + +impl PartialEq for dyn PairKey + '_ { + fn eq(&self, other: &Self) -> bool { + self.pair() == other.pair() + } +} + +impl Eq for dyn PairKey + '_ {} + /// A collection of data. This is where we dump data into. /// /// TODO: Maybe reduce visibility of internal data, make it only accessible @@ -42,7 +83,7 @@ pub struct StoredData { pub process_data: ProcessData, /// TODO: (points_rework_v1) Might be a better way to do this without having /// to store here? - pub prev_io: Vec<(u64, u64)>, + pub prev_io: FxHashMap<(String, String), (u64, u64)>, pub disk_harvest: Vec, pub temp_data: Vec, #[cfg(feature = "battery")] @@ -62,7 +103,7 @@ impl Default for StoredData { cpu_harvest: CpuHarvest::default(), load_avg_harvest: LoadAvgHarvest::default(), process_data: Default::default(), - prev_io: Vec::default(), + prev_io: FxHashMap::default(), disk_harvest: Vec::default(), temp_data: Vec::default(), #[cfg(feature = "battery")] @@ -179,7 +220,6 @@ impl StoredData { self.last_update_time = harvested_time; } - // TODO: There's a spike on the first hit. We should probably fix this and the index issue. fn eat_disks(&mut self, disks: Vec, io: IoHarvest, harvested_time: Instant) { let time_since_last_harvest = harvested_time .duration_since(self.last_update_time) @@ -187,41 +227,35 @@ impl StoredData { self.disk_harvest.clear(); - let prev_io_diff = disks.len().saturating_sub(self.prev_io.len()); - self.prev_io.reserve(prev_io_diff); - self.prev_io.extend((0..prev_io_diff).map(|_| (0, 0))); - - // FIXME: prev_io is indexed by position (itx), not by device name, which might cause problems - // if the order changes or something. - for (itx, device) in disks.into_iter().enumerate() { + for disk in disks { let Some(checked_name) = ({ #[cfg(target_os = "windows")] { - match &device.volume_name { + match &disk.volume_name { Some(volume_name) => Some(volume_name.as_str()), - None => device.name.split('/').next_back(), + None => disk.name.split('/').next_back(), } } #[cfg(not(target_os = "windows"))] { #[cfg(any(feature = "zfs", target_os = "freebsd"))] { - if !device.name.starts_with('/') { - Some(device.name.as_str()) // use the whole name + if !disk.name.starts_with('/') { + Some(disk.name.as_str()) // use the whole name } else { #[cfg(target_os = "freebsd")] { - Some(device.mount_point.as_str()) // use mount_point for sysinfo + Some(disk.mount_point.as_str()) // use mount_point for sysinfo } #[cfg(not(target_os = "freebsd"))] { - device.name.split('/').next_back() // use device name + disk.name.split('/').next_back() // use device name } } } #[cfg(not(any(feature = "zfs", target_os = "freebsd")))] { - device.name.split('/').next_back() + disk.name.split('/').next_back() } } }) else { @@ -258,35 +292,48 @@ impl StoredData { }; let (mut io_read_rate_bytes, mut io_write_rate_bytes) = (None, None); - if let Some(Some(io_device)) = io_device - && let Some(prev_io) = self.prev_io.get_mut(itx) - { - io_read_rate_bytes = Some( - ((io_device.read_bytes.saturating_sub(prev_io.0)) as f64 - / time_since_last_harvest) - .round() as u64, - ); + if let Some(Some(io_device)) = io_device { + if let Some(prev_io) = self + .prev_io + .get_mut(&(disk.mount_point.as_str(), checked_name) as &dyn PairKey) + { + io_read_rate_bytes = Some( + ((io_device.read_bytes.saturating_sub(prev_io.0)) as f64 + / time_since_last_harvest) + .round() as u64, + ); - io_write_rate_bytes = Some( - ((io_device.write_bytes.saturating_sub(prev_io.1)) as f64 - / time_since_last_harvest) - .round() as u64, - ); + io_write_rate_bytes = Some( + ((io_device.write_bytes.saturating_sub(prev_io.1)) as f64 + / time_since_last_harvest) + .round() as u64, + ); - *prev_io = (io_device.read_bytes, io_device.write_bytes); + *prev_io = (io_device.read_bytes, io_device.write_bytes); + } else { + // Skip on first run. + io_read_rate_bytes = Some(0); + io_write_rate_bytes = Some(0); + + // TODO: We probably want to also add some cleanup after a while if unused. + self.prev_io.insert( + (disk.mount_point.clone(), checked_name.to_string()), + (io_device.read_bytes, io_device.write_bytes), + ); + } } - let summed_total_bytes = match (device.used_space, device.free_space) { + let summed_total_bytes = match (disk.used_space, disk.free_space) { (Some(used), Some(free)) => Some(used + free), _ => None, }; self.disk_harvest.push(DiskWidgetData { - name: device.name, - mount_point: device.mount_point, - free_bytes: device.free_space, - used_bytes: device.used_space, - total_bytes: device.total_space, + name: disk.name, + mount_point: disk.mount_point, + free_bytes: disk.free_space, + used_bytes: disk.used_space, + total_bytes: disk.total_space, summed_total_bytes, io_read_rate_bytes, io_write_rate_bytes,