-
Notifications
You must be signed in to change notification settings - Fork 255
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.
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
Conversation
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); |
There was a problem hiding this comment.
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.
@@ -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) { |
There was a problem hiding this comment.
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?
As the original requester in #424, this is amazing, thanks :) It seems like it only sums memory usage when it's displayed in percentages? When memory usage is displayed in MiB collapsing branches in the tree view shows only the top-level memory usage. |
Gah. Yeah, that's a bug. I forgot to update the string for that value even though I added all the values up... |
#473 should fix this issue. I'll probably deploy the patch in the next day or so. Thanks for the prompt report! |
Thanks so much :) |
Description
A description of the change and what it does. If relevant (such as any change that modifies the UI), please provide screenshots of the change:
For the process widget, we now sum the resource usage of the child processes on the parent entry when collapsing in tree mode.
For example - before collapsing:
After collapsing:
Note that if you search to filter, and collapse, it will not sum the pruned values (values that cannot be seen). This is partly because I'm a bit lazy, and partly because I think this behaviour makes sense.
For example, let's say I search for a process with 4 child processes "AA, AB, BA, BB", with CPU usage 0.1, 0.2, 0.3, 0.4 respectively. Assume the parent process has 0 usage.
I think this is fine because I'm treating this as summing any child that is still visible somehow. Summing unseen values would probably be weird as it would look like it's not adding up.
Further note that if you had, say, a child "CC" with a usage of, say, 2.0, and its parent of "AB", and you searched for CC in our above example, you would get a sum of 2.2. This is because AB is still visible by the fact that CC was the searched process, and AB must still exist (albeit faded out) in the tree hierarchy, and as such will still be displayed.
We can see this here:
Searching "btm":
Searching for "zsh" instead:
Issue
If applicable, what issue does this address?
Closes: #424
Type of change
Remove the irrelevant ones:
Test methodology
If relevant, please state how this was tested:
Furthermore, please tick which platforms this change was tested on:
If relevant, all of these platforms should be tested.
Checklist
If relevant, ensure the following have been met:
cargo test
check)cargo fmt
)Other information
Provide any other relevant information to this change: