Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: Collapsing in tree mode sums usage to parent #445

Merged
merged 4 commits into from
Apr 5, 2021
Merged

feature: Collapsing in tree mode sums usage to parent #445

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7b0f177
feature: Collapsing in tree mode sums usage to parent
ClementTsang Apr 5, 2021
e613107
Add changelog entry
ClementTsang Apr 5, 2021
a2a81e2
Fix rounding flaw causing summation to look like it wasn't adding up
ClementTsang Apr 5, 2021
a119675
Add todo
ClementTsang Apr 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- [#437](https://github.com/ClementTsang/bottom/pull/437): Redo dynamic network y-axis, add linear scaling, unit type, and prefix options.

- [#445](https://github.com/ClementTsang/bottom/pull/445): Collapsing in tree mode sums usage to parent.

## Changes

- [#372](https://github.com/ClementTsang/bottom/pull/372): Hides the SWAP graph and legend in normal mode if SWAP is 0.
Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ crossterm = "0.18.2"
ctrlc = { version = "3.1", features = ["termination"] }
clap = "2.33"
dirs-next = "2.0.0"
fxhash = "0.2.1"
futures = "0.3.12"
fxhash = "0.2.1"
indexmap = "~1.6"
itertools = "0.10.0"
once_cell = "1.5.2"
Expand Down
128 changes: 121 additions & 7 deletions src/data_conversion.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1019,6 +1019,62 @@ pub fn tree_process_data(
(explored_pids, lines)
}

/// Returns the total sum of CPU, MEM%, MEM, R/s, W/s, Total Read, and Total Write via DFS traversal.
fn get_usage_of_all_children(
parent_pid: Pid, parent_child_mapping: &HashMap<Pid, IndexSet<Pid, FxBuildHasher>>,
pid_process_mapping: &HashMap<Pid, &ConvertedProcessData>,
) -> (f64, f64, u64, f64, f64, f64, f64) {
if let Some(&converted_process_data) = pid_process_mapping.get(&parent_pid) {
let (
mut cpu,
mut mem_percent,
mut mem,
mut rps,
mut wps,
mut total_read,
mut total_write,
) = (
(converted_process_data.cpu_percent_usage * 10.0).round() / 10.0,
(converted_process_data.mem_percent_usage * 10.0).round() / 10.0,
converted_process_data.mem_usage_bytes,
(converted_process_data.rps_f64 * 10.0).round() / 10.0,
(converted_process_data.wps_f64 * 10.0).round() / 10.0,
(converted_process_data.tr_f64 * 10.0).round() / 10.0,
(converted_process_data.tw_f64 * 10.0).round() / 10.0,
);

if let Some(children) = parent_child_mapping.get(&parent_pid) {
for &child_pid in children {
let (
child_cpu,
child_mem_percent,
child_mem,
child_rps,
child_wps,
child_total_read,
child_total_write,
) = get_usage_of_all_children(
child_pid,
parent_child_mapping,
pid_process_mapping,
);

cpu += child_cpu;
mem_percent += child_mem_percent;
mem += child_mem;
rps += child_rps;
wps += child_wps;
total_read += child_total_read;
total_write += child_total_write;
}
}

(cpu, mem_percent, mem, rps, wps, total_read, total_write)
} else {
(0.0_f64, 0.0_f64, 0, 0.0_f64, 0.0_f64, 0.0_f64, 0.0_f64)
}
}

let mut to_sort_vec = Vec::new();
for pid in pids_to_explore {
if let Some(process) = pid_process_mapping.get(&pid) {
Expand Down Expand Up @@ -1051,9 +1107,9 @@ pub fn tree_process_data(
explored_pids
.iter()
.zip(lines)
.filter_map(|(pid, prefix)| match pid_process_mapping.remove(pid) {
.filter_map(|(pid, prefix)| match pid_process_mapping.get(pid) {
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is fine?

Some(process) => {
let mut p = process.clone();
let mut p = (*process).clone();
p.process_description_prefix = Some(format!(
"{}{}{}",
prefix,
Expand All @@ -1065,6 +1121,62 @@ pub fn tree_process_data(
}
));

// As part of https://github.com/ClementTsang/bottom/issues/424, also append their statistics to the parent if
// collapsed.
//
// Note that this will technically be "missing" entries, it collapses + sums based on what is visible
// since this runs *after* pruning steps.
if p.is_collapsed_entry {
if let Some(children) = parent_child_mapping.get(&p.pid) {
// Do some rounding.
p.cpu_percent_usage = (p.cpu_percent_usage * 10.0).round() / 10.0;
p.mem_percent_usage = (p.mem_percent_usage * 10.0).round() / 10.0;
p.rps_f64 = (p.rps_f64 * 10.0).round() / 10.0;
p.wps_f64 = (p.wps_f64 * 10.0).round() / 10.0;
p.tr_f64 = (p.tr_f64 * 10.0).round() / 10.0;
p.tw_f64 = (p.tw_f64 * 10.0).round() / 10.0;

for &child_pid in children {
// Let's just do a simple DFS traversal...
let (
child_cpu,
child_mem_percent,
child_mem,
child_rps,
child_wps,
child_total_read,
child_total_write,
) = get_usage_of_all_children(
child_pid,
&parent_child_mapping,
&pid_process_mapping,
);

p.cpu_percent_usage += child_cpu;
p.mem_percent_usage += child_mem_percent;
p.mem_usage_bytes += child_mem;
p.rps_f64 += child_rps;
p.wps_f64 += child_wps;
p.tr_f64 += child_total_read;
p.tw_f64 += child_total_write;
}

let converted_rps = get_decimal_bytes(p.rps_f64 as u64);
let converted_wps = get_decimal_bytes(p.wps_f64 as u64);
let converted_total_read = get_decimal_bytes(p.tr_f64 as u64);
let converted_total_write = get_decimal_bytes(p.tw_f64 as u64);

p.read_per_sec = format!("{:.*}{}/s", 0, converted_rps.0, converted_rps.1);
p.write_per_sec = format!("{:.*}{}/s", 0, converted_wps.0, converted_wps.1);
p.total_read =
format!("{:.*}{}", 0, converted_total_read.0, converted_total_read.1);
p.total_write = format!(
"{:.*}{}",
0, converted_total_write.0, converted_total_write.1
);
}
}

Some(p)
}
None => None,
Expand Down Expand Up @@ -1186,10 +1298,12 @@ pub fn group_process_data(
.iter()
.map(|(identifier, process_details)| {
let p = process_details.clone();
let converted_rps = get_binary_bytes(p.read_per_sec as u64);
let converted_wps = get_binary_bytes(p.write_per_sec as u64);
let converted_total_read = get_binary_bytes(p.total_read as u64);
let converted_total_write = get_binary_bytes(p.total_write as u64);

// FIXME: Unify this step in the three locations it is used to one function.
let converted_rps = get_decimal_bytes(p.read_per_sec as u64);
let converted_wps = get_decimal_bytes(p.write_per_sec as u64);
let converted_total_read = get_decimal_bytes(p.total_read as u64);
let converted_total_write = get_decimal_bytes(p.total_write as u64);
Comment on lines +1303 to +1306
Copy link
Owner Author

@ClementTsang ClementTsang Apr 5, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional fix to keep process resource usage and disk usage consistent. This should have been done in #443 but was overlooked; ideally we keep this all consistent in one function in the future to avoid this.


let read_per_sec = format!("{:.*}{}/s", 0, converted_rps.0, converted_rps.1);
let write_per_sec = format!("{:.*}{}/s", 0, converted_wps.0, converted_wps.1);
Expand All @@ -1208,7 +1322,7 @@ pub fn group_process_data(
cpu_percent_usage: p.cpu_percent_usage,
mem_percent_usage: p.mem_percent_usage,
mem_usage_bytes: p.mem_usage_bytes,
mem_usage_str: get_binary_bytes(p.mem_usage_bytes),
mem_usage_str: get_decimal_bytes(p.mem_usage_bytes),
group_pids: p.group_pids,
read_per_sec,
write_per_sec,
Expand Down