Skip to content

Commit

Permalink
refactor: clean up some file structure, process code, and terminal cl…
Browse files Browse the repository at this point in the history
…eanup (#1676)

* move widgets

* reduce allocations needed

* ah

* more possible optimizations around reducing allocs

* some fixes

* I forgot to clear the buffer oops

* missing

* only run terminal cleanup after certain point
  • Loading branch information
ClementTsang authored Feb 15, 2025
1 parent 2b5441c commit d63ca07
Show file tree
Hide file tree
Showing 13 changed files with 207 additions and 93 deletions.
4 changes: 3 additions & 1 deletion src/app/data/time_series.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ pub type Values = ChunkedData<f64>;
#[derive(Clone, Debug, Default)]
pub struct TimeSeriesData {
/// Time values.
pub time: Vec<Instant>, // TODO: (points_rework_v1) should we not store instant, and just store the millisecond component? Write a compatible wrapper!
///
/// TODO: (points_rework_v1) Either store millisecond-level only or offsets only.
pub time: Vec<Instant>,

/// Network RX data.
pub rx: Values,
Expand Down
8 changes: 6 additions & 2 deletions src/bin/main.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
use bottom::{reset_stdout, start_bottom};

fn main() -> anyhow::Result<()> {
start_bottom().inspect_err(|_| {
reset_stdout();
let mut run_error_hook = false;

start_bottom(&mut run_error_hook).inspect_err(|_| {
if run_error_hook {
reset_stdout();
}
})
}
52 changes: 51 additions & 1 deletion src/collection/processes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
//! For Windows, macOS, FreeBSD, Android, and Linux, this is handled by sysinfo.
use cfg_if::cfg_if;
use sysinfo::ProcessStatus;

cfg_if! {
if #[cfg(target_os = "linux")] {
Expand Down Expand Up @@ -81,7 +82,7 @@ pub struct ProcessHarvest {
pub total_write_bytes: u64,

/// The current state of the process (e.g. zombie, asleep).
pub process_state: (String, char),
pub process_state: (&'static str, char),

/// Cumulative process uptime.
pub time: Duration,
Expand Down Expand Up @@ -150,3 +151,52 @@ impl DataCollector {
}
}
}

/// Pulled from [`ProcessStatus::to_string`] to avoid an alloc.
pub(super) fn process_status_str(status: ProcessStatus) -> &'static str {
cfg_if::cfg_if! {
if #[cfg(target_os = "linux")] {
match status {
ProcessStatus::Idle => "Idle",
ProcessStatus::Run => "Runnable",
ProcessStatus::Sleep => "Sleeping",
ProcessStatus::Stop => "Stopped",
ProcessStatus::Zombie => "Zombie",
ProcessStatus::Tracing => "Tracing",
ProcessStatus::Dead => "Dead",
ProcessStatus::Wakekill => "Wakekill",
ProcessStatus::Waking => "Waking",
ProcessStatus::Parked => "Parked",
ProcessStatus::UninterruptibleDiskSleep => "UninterruptibleDiskSleep",
_ => "Unknown",
}
} else if #[cfg(target_os = "windows")] {
match status {
ProcessStatus::Run => "Runnable",
_ => "Unknown",
}
} else if #[cfg(target_os = "macos")] {
match status {
ProcessStatus::Idle => "Idle",
ProcessStatus::Run => "Runnable",
ProcessStatus::Sleep => "Sleeping",
ProcessStatus::Stop => "Stopped",
ProcessStatus::Zombie => "Zombie",
_ => "Unknown",
}
} else if #[cfg(target_os = "freebsd")] {
match status {
ProcessStatus::Idle => "Idle",
ProcessStatus::Run => "Runnable",
ProcessStatus::Sleep => "Sleeping",
ProcessStatus::Stop => "Stopped",
ProcessStatus::Zombie => "Zombie",
ProcessStatus::Dead => "Dead",
ProcessStatus::LockBlocked => "LockBlocked",
_ => "Unknown",
}
} else {
"Unknown"
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@ use std::{
time::Duration,
};

use concat_string::concat_string;
use hashbrown::HashSet;
use process::*;
use sysinfo::ProcessStatus;

use super::{Pid, ProcessHarvest, UserTable};
use super::{process_status_str, Pid, ProcessHarvest, UserTable};
use crate::collection::{error::CollectionResult, DataCollector};

/// Maximum character length of a `/proc/<PID>/stat`` process name.
Expand Down Expand Up @@ -149,39 +150,9 @@ fn read_proc(
uptime,
} = args;

let (command, name) = {
let truncated_name = stat.comm.as_str();
if let Ok(cmdline) = cmdline {
if cmdline.is_empty() {
(format!("[{truncated_name}]"), truncated_name.to_string())
} else {
(
cmdline.join(" "),
if truncated_name.len() >= MAX_STAT_NAME_LEN {
if let Some(first_part) = cmdline.first() {
// We're only interested in the executable part... not the file path.
// That's for command.
first_part
.rsplit_once('/')
.map(|(_prefix, suffix)| suffix)
.unwrap_or(truncated_name)
.to_string()
} else {
truncated_name.to_string()
}
} else {
truncated_name.to_string()
},
)
}
} else {
(truncated_name.to_string(), truncated_name.to_string())
}
};

let process_state_char = stat.state;
let process_state = (
ProcessStatus::from(process_state_char).to_string(),
process_status_str(ProcessStatus::from(process_state_char)),
process_state_char,
);
let (cpu_usage_percent, new_process_times) = get_linux_cpu_usage(
Expand All @@ -197,7 +168,7 @@ fn read_proc(

// This can fail if permission is denied!
let (total_read_bytes, total_write_bytes, read_bytes_per_sec, write_bytes_per_sec) =
if let Ok(io) = io {
if let Some(io) = io {
let total_read_bytes = io.read_bytes;
let total_write_bytes = io.write_bytes;
let prev_total_read_bytes = prev_proc.total_read_bytes;
Expand Down Expand Up @@ -242,6 +213,37 @@ fn read_proc(
Duration::ZERO
};

let (command, name) = {
let truncated_name = stat.comm;
if let Some(cmdline) = cmdline {
if cmdline.is_empty() {
(concat_string!("[", truncated_name, "]"), truncated_name)
} else {
let name = if truncated_name.len() >= MAX_STAT_NAME_LEN {
let first_part = match cmdline.split_once(' ') {
Some((first, _)) => first,
None => &cmdline,
};

// We're only interested in the executable part, not the file path (part of command),
// so strip everything but the command name if needed.
let last_part = match first_part.rsplit_once('/') {
Some((_, last)) => last,
None => first_part,
};

last_part.to_string()
} else {
truncated_name
};

(cmdline, name)
}
} else {
(truncated_name.clone(), truncated_name)
}
};

Ok((
ProcessHarvest {
pid: process.pid,
Expand Down Expand Up @@ -354,9 +356,11 @@ pub(crate) fn linux_process_data(
uptime: sysinfo::System::uptime(),
};

let mut buffer = String::new();

let process_vector: Vec<ProcessHarvest> = pids
.filter_map(|pid_path| {
if let Ok(process) = Process::from_path(pid_path) {
if let Ok(process) = Process::from_path(pid_path, &mut buffer) {
let pid = process.pid;
let prev_proc_details = pid_mapping.entry(pid).or_default();

Expand Down
69 changes: 39 additions & 30 deletions src/collection/processes/linux/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,14 +58,13 @@ pub(crate) struct Stat {
}

impl Stat {
#[inline]
fn from_file(mut f: File, buffer: &mut String) -> anyhow::Result<Stat> {
// Since this is just one line, we can read it all at once. However, since it
// might have non-utf8 characters, we can't just use read_to_string.
// (technically) might have non-utf8 characters, we can't just use read_to_string.
f.read_to_end(unsafe { buffer.as_mut_vec() })?;

let line = buffer.to_string_lossy();
let line = line.trim();
// TODO: Is this needed?
let line = buffer.trim();

let (comm, rest) = {
let start_paren = line
Expand Down Expand Up @@ -204,8 +203,8 @@ pub(crate) struct Process {
pub pid: Pid,
pub uid: Option<uid_t>,
pub stat: Stat,
pub io: anyhow::Result<Io>,
pub cmdline: anyhow::Result<Vec<String>>,
pub io: Option<Io>,
pub cmdline: Option<String>,
}

#[inline]
Expand All @@ -223,8 +222,10 @@ impl Process {
/// methods. Therefore, this struct is only useful for either fields
/// that are unlikely to change, or are short-lived and
/// will be discarded quickly.
pub(crate) fn from_path(pid_path: PathBuf) -> anyhow::Result<Process> {
// TODO: Pass in a buffer vec/string to share?
///
/// This takes in a buffer to avoid allocs; this function will clear the buffer.
pub(crate) fn from_path(pid_path: PathBuf, buffer: &mut String) -> anyhow::Result<Process> {
buffer.clear();

let fd = rustix::fs::openat(
rustix::fs::CWD,
Expand Down Expand Up @@ -254,18 +255,26 @@ impl Process {
};

let mut root = pid_path;
let mut buffer = String::new();

// NB: Whenever you add a new stat, make sure to pop the root and clear the
// buffer!
let stat =
open_at(&mut root, "stat", &fd).and_then(|file| Stat::from_file(file, &mut buffer))?;
reset(&mut root, &mut buffer);

let cmdline = cmdline(&mut root, &fd, &mut buffer);
reset(&mut root, &mut buffer);
// Stat is pretty long, do this first to pre-allocate up-front.
let stat =
open_at(&mut root, "stat", &fd).and_then(|file| Stat::from_file(file, buffer))?;
reset(&mut root, buffer);

let cmdline = if cmdline(&mut root, &fd, buffer).is_ok() {
// The clone will give a string with the capacity of the length of buffer, don't worry.
Some(buffer.clone())
} else {
None
};
reset(&mut root, buffer);

let io = open_at(&mut root, "io", &fd).and_then(|file| Io::from_file(file, &mut buffer));
let io = open_at(&mut root, "io", &fd)
.and_then(|file| Io::from_file(file, buffer))
.ok();

Ok(Process {
pid,
Expand All @@ -278,22 +287,22 @@ impl Process {
}

#[inline]
fn cmdline(root: &mut PathBuf, fd: &OwnedFd, buffer: &mut String) -> anyhow::Result<Vec<String>> {
open_at(root, "cmdline", fd)
fn cmdline(root: &mut PathBuf, fd: &OwnedFd, buffer: &mut String) -> anyhow::Result<()> {
let _ = open_at(root, "cmdline", fd)
.map(|mut file| file.read_to_string(buffer))
.map(|_| {
buffer
.split('\0')
.filter_map(|s| {
if !s.is_empty() {
Some(s.to_string())
} else {
None
}
})
.collect::<Vec<_>>()
})
.map_err(Into::into)
.inspect(|_| {
// SAFETY: We are only replacing a single char (NUL) with another single char (space).
let buf_mut = unsafe { buffer.as_mut_vec() };

for byte in buf_mut {
if *byte == 0 {
const SPACE: u8 = ' '.to_ascii_lowercase() as u8;
*byte = SPACE;
}
}
})?;

Ok(())
}

/// Opens a path. Note that this function takes in a mutable root - this will
Expand Down
Loading

0 comments on commit d63ca07

Please sign in to comment.