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.
This commit is contained in:
Clement Tsang
2026-07-15 08:11:49 +00:00
committed by GitHub
parent d0053421b4
commit 8b2b3327b3
2 changed files with 85 additions and 37 deletions
+1
View File
@@ -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
+84 -37
View File
@@ -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<dyn PairKey + 'a> for (String, String) {
fn borrow(&self) -> &(dyn PairKey + 'a) {
self
}
}
impl Hash for dyn PairKey + '_ {
fn hash<H: Hasher>(&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<DiskWidgetData>,
pub temp_data: Vec<TempWidgetData>,
#[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<DiskHarvest>, 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,