htop icon indicating copy to clipboard operation
htop copied to clipboard

add grouped memory & cpu columns

Open 80avin opened this issue 1 year ago • 10 comments

Fixes https://github.com/htop-dev/htop/issues/301 , https://github.com/htop-dev/htop/issues/920 , https://github.com/htop-dev/htop/issues/1154

Add GCPU (Grouped CPU) and GMEM (Grouped Memory) columns which show sum of cpu/memory for the current process & its children.

Open Questions

  • Are keyboard shortcuts to sort these columns acceptable ?
    • Logically, they mirror existing shortcuts of M (memory) to X and P (CPU) to W. I didn't want to choose Q that's why chose 2nd column of keys.
  • Are names shown ( GCPU, GMEM ) acceptable ? Could've used keywords like tree, children to derive new names ( like TCPU, CPUT, or maybe something else. )
  • Tested only on Linux ( Linux 6.5.0-13-generic - Kubuntu 23.10 )

Screenshot

image

80avin avatar Nov 25 '23 15:11 80avin

I don't like this.

  1. The GCPU% and GMEM% are bad names. (The "G" can be confused with something else like GPU and graphics memory)
  2. I think a more ideal solution is not to introduce new columns for such things, but to adjust the CPU% and MEM% displays in Tree View so that they display aggregated data when the tree is collapsed.
  3. The sorting in Tree View by CPU% or MEM% might need to be adjusted, too.

Explorer09 avatar Nov 26 '23 08:11 Explorer09

I don't like this.

  1. The GCPU% and GMEM% are bad names. (The "G" can be confused with something else like GPU and graphics memory)

Names can be changed. And I'm not quite happy with them either …

  1. I think a more ideal solution is not to introduce new columns for such things, but to adjust the CPU% and MEM% displays in Tree View so that they display aggregated data when the tree is collapsed.

That's mostly a matter of taste, as you can argue both ways. Either you display always the true values and have aggregate columns to add things up, or you have a mixed column, where some values are aggregates and you end up having to mark, which values are aggregates and which are not.

I personally prefer the solution using a separate column, as it does not loose you any information, but to the contrary actually tells you, how much of the aggregate value is from child processes.

  1. The sorting in Tree View by CPU% or MEM% might need to be adjusted, too.

Can you elaborate on this item?

BenBE avatar Nov 26 '23 14:11 BenBE

@BenBE The idea of aggregated values of child processes works better when Tree View is in effect, otherwise you will end up seeing duplicated numbers of the parent processes listed first, and their child processes listed further down. When the CPU% numbers of all processes don't add up to (number of CPU cores × 100%), users would get confused more. That's one of the reasons I don't like a column of aggregated numbers.

The sorting in Tree View by CPU% or MEM% might need to be adjusted, too.

Can you elaborate on this item?

This is my personal opinion. I think the advantage of having a tree view is to have a quick idea which processes are part of a group and which process groups form a session. It's not just about tracking the parents of the processes. So I think there are rooms to improve the Tree View usability, especially with the goal of managing processes in groups and sessions.

When it comes to sorting, I mean the CPU% and MEM% in a tree branch should be ordered by the aggregated value of that branch. For one parent process, the children with the most CPU usage (in total, including all the descendant processes) would be listed first, followed by other children/branches. The similar applies to the MEM usage sorting.

Explorer09 avatar Nov 26 '23 15:11 Explorer09

@Explorer09 I agree with most of what you said.

I don't like this.

  1. The GCPU% and GMEM% are bad names.

I am still thinking about the right name and am not convinced with what current ones. I've mentioned same in PR body also, and will try to find better ones. If using non-alphabetical ASCII characters ( like %CPU┼ , %CPU± , %CPU┬ , %CPU╦ , %CPU» , %CPU* etc.) is fine, then we can have more combinations to try & explore. Not good examples, but I will give it more time.

  1. I think a more ideal solution is not to introduce new columns for such things but to adjust the CPU% and MEM% displays in Tree View so that they display aggregated data when the tree is collapsed.

Actually, I thought in this direction originally. My requirement was simple, to see the aggregate CPU/MEM%, irrespective of whether it is shown by a new setting or a new column. But then I wanted to see how many resources a tree is using vs. the parent & children nodes side-by-side. This could be achieved only by adding a new column type. We can also have more aggregate columns which aren't derived from current ones, like child process count, etc. However, I still see these as opinions and will accept whatever the majority votes for.

  1. The sorting in Tree View by CPU% or MEM% might need to be adjusted, too.

I also noticed the same. I'm thinking of making a separate PR for that as it will impact the existing columns also.

80avin avatar Nov 26 '23 18:11 80avin

Have you done some performance testing how much this additional column influences the overall performance?

A quick and dirty implementation shows that it takes <10 ms on my machine, running ~200tasks at the moment. I don't think this is good and I'll try to optimize while refactoring how/where the aggregate is calculated.

80avin avatar Nov 26 '23 18:11 80avin

  1. The GCPU% and GMEM% are bad names.

I am still thinking about the right name and am not convinced with what current ones. I've mentioned same in PR body also, and will try to find better ones. If using non-alphabetical ASCII characters ( like %CPU┼ , %CPU± , %CPU┬ , %CPU╦ , %CPU» , %CPU* etc.) is fine, then we can have more combinations to try & explore. Not good examples, but I will give it more time.

Using Unicode is fine in htop, but we would need ASCII fallback as not every user would run htop with UTF-8 mode enabled.

  1. I think a more ideal solution is not to introduce new columns for such things but to adjust the CPU% and MEM% displays in Tree View so that they display aggregated data when the tree is collapsed.

Actually, I thought in this direction originally. My requirement was simple, to see the aggregate CPU/MEM%, irrespective of whether it is shown by a new setting or a new column. But then I wanted to see how many resources a tree is using vs. the parent & children nodes side-by-side. This could be achieved only by adding a new column type. We can also have more aggregate columns which aren't derived from current ones, like child process count, etc. However, I still see these as opinions and will accept whatever the majority votes for.

I knew your idea, but the problem is that there could be a lot of columns introduced just for aggregated stats after we pull in your patch as the first example.

I have a better vision for this: if the aggregated data for a process group or session is important and worth displaying separately from the parent process node, I would introduce separate rows for displaying the aggregated info. Such rows would be colored differently from the processes and threads (for a better visual distinction). As these row do not refer to individual processes or threads:

  • The PID would be a placeholder like *.
  • The "Command" field or summary would say something like [Process group 12345, 5 processes] or [Session 23455, 10 processes]
  • I think these rows for aggregated data would be provided for distinct process groups and sessions only. If you want aggregated data for multiple process groups under a single parent (e.g. a bash shell running several pipes of commands simultaneously), the user would need to add up the stats themselves or close the tree under that bash process.
  • These new rows for aggregated data may be turned off (i.e. hidden) in settings.

How is that?

Explorer09 avatar Nov 26 '23 21:11 Explorer09

  1. The GCPU% and GMEM% are bad names.

I am still thinking about the right name and am not convinced with what current ones. I've mentioned same in PR body also, and will try to find better ones. If using non-alphabetical ASCII characters ( like %CPU┼ , %CPU± , %CPU┬ , %CPU╦ , %CPU» , %CPU* etc.) is fine, then we can have more combinations to try & explore. Not good examples, but I will give it more time.

Using Unicode is fine in htop, but we would need ASCII fallback as not every user would run htop with UTF-8 mode enabled.

To be precise: Enabling Unicode support is an optional compile-time feature, thus columns should be able to be displayed with ASCII alone.

  1. I think a more ideal solution is not to introduce new columns for such things but to adjust the CPU% and MEM% displays in Tree View so that they display aggregated data when the tree is collapsed.

Actually, I thought in this direction originally. My requirement was simple, to see the aggregate CPU/MEM%, irrespective of whether it is shown by a new setting or a new column. But then I wanted to see how many resources a tree is using vs. the parent & children nodes side-by-side. This could be achieved only by adding a new column type. We can also have more aggregate columns which aren't derived from current ones, like child process count, etc. However, I still see these as opinions and will accept whatever the majority votes for.

I knew your idea, but the problem is that there could be a lot of columns introduced just for aggregated stats after we pull in your patch as the first example.

ACK. That's also why I'm not really fired-up about this yet. I see both sides and my initial review was from a technical PoV only.

I have a better vision for this: if the aggregated data for a process group or session is important and worth displaying separately from the parent process node, I would introduce separate rows for displaying the aggregated info. Such rows would be colored differently from the processes and threads (for a better visual distinction). As these row do not refer to individual processes or threads:

Not really a great option either, as this causes vertical screen estate to be used up for this.

An idea to explore may be implementing "multi-mode" columns, similar to the meter display mode, where the same column could be switched to display different values (in the configuration).

  • The PID would be a placeholder like *.
  • The "Command" field or summary would say something like [Process group 12345, 5 processes] or [Session 23455, 10 processes]
  • I think these rows for aggregated data would be provided for distinct process groups and sessions only. If you want aggregated data for multiple process groups under a single parent (e.g. a bash shell running several pipes of commands simultaneously), the user would need to add up the stats themselves or close the tree under that bash process.
  • These new rows for aggregated data may be turned off (i.e. hidden) in settings.

How is that?

That looks like #303 on first glance …

BenBE avatar Nov 27 '23 00:11 BenBE

I have a better vision for this: if the aggregated data for a process group or session is important and worth displaying separately from the parent process node, I would introduce separate rows for displaying the aggregated info. Such rows would be colored differently from the processes and threads (for a better visual distinction).

I think the idea needs to be refined more to be convincing. Not sure if it is only me but when I try to evaluate this idea based on the value it adds relative to the development effort, I don't find it much pleasing. Like, if we want to show aggregate values like number of child processes running for a tree, it won't fit into any existing column. Correct me if you find I'm missing anything.

An idea to explore may be implementing "multi-mode" columns

"multi-mode" columns also sound like a great idea. If we're having that, then we can also allow the user to add duplicate columns (same columns with different modes) to his screen. But another perspective on the multi-mode column is that

  1. They will only reduce some clutter from Screen settings, as "Available Columns" will have one entry for process CPU & tree CPU. Otherwise, they'll be working the same as current PR from the user's perspective.
    Is this difference enough of value addition that we implement multi-mode feature for columns ?
  2. Different modes will still require different column names, so this "multi-mode" is just a way to pack multiple column definitions into one in the Settings only.

80avin avatar Nov 27 '23 16:11 80avin

I think the idea needs to be refined more to be convincing. Not sure if it is only me but when I try to evaluate this idea based on the value it adds relative to the development effort, I don't find it much pleasing.

Well, the development effort is not something I would evaluate as htop is an open source project without any definite deadline or goal (in terms of features). Maintenance effort is what we would evaluate instead for every new feature added.

Like, if we want to show aggregate values like number of child processes running for a tree, it won't fit into any existing column. Correct me if you find I'm missing anything.

Do you mean the number of child process in the Running (R) state, or the number of child processes that have started and not dead? The latter includes processes in the Sleep (S) state and I/O waiting (D) state. You know, processes you could see in htop are usually in S state -- only a few of them would be R and consume CPU time.

An idea to explore may be implementing "multi-mode" columns

"multi-mode" columns also sound like a great idea. If we're having that, then we can also allow the user to add duplicate columns (same columns with different modes) to his screen.

I will let @BenBE correct me if I get this wrong, but my vision is that the columns' mode would be global, and you won't see per-process CPU% and aggregated CPU% side by side. Only one will be displayed at a time, but a hotkey would be available to quickly switch between modes -- that is, between per-process data and aggregated data.

Explorer09 avatar Nov 27 '23 19:11 Explorer09

@80avin:

I have a better vision for this: if the aggregated data for a process group or session is important and worth displaying separately from the parent process node, I would introduce separate rows for displaying the aggregated info. Such rows would be colored differently from the processes and threads (for a better visual distinction).

I think the idea needs to be refined more to be convincing. Not sure if it is only me but when I try to evaluate this idea based on the value it adds relative to the development effort, I don't find it much pleasing. Like, if we want to show aggregate values like number of child processes running for a tree, it won't fit into any existing column. Correct me if you find I'm missing anything.

Currently there's no real column for this. If you stretch definitions you could re-use the # of LWP column for this.

An idea to explore may be implementing "multi-mode" columns

"multi-mode" columns also sound like a great idea. If we're having that, then we can also allow the user to add duplicate columns (same columns with different modes) to his screen. But another perspective on the multi-mode column is that

  1. They will only reduce some clutter from Screen settings, as "Available Columns" will have one entry for process CPU & tree CPU. Otherwise, they'll be working the same as current PR from the user's perspective. Is this difference enough of value addition that we implement multi-mode feature for columns ?
  2. Different modes will still require different column names, so this "multi-mode" is just a way to pack multiple column definitions into one in the Settings only.

ACK.

@Explorer09:

I think the idea needs to be refined more to be convincing. Not sure if it is only me but when I try to evaluate this idea based on the value it adds relative to the development effort, I don't find it much pleasing.

Well, the development effort is not something I would evaluate as htop is an open source project without any definite deadline or goal (in terms of features). Maintenance effort is what we would evaluate instead for every new feature added.

Correct. And one of the reasons #303 didn't get anywhere yet is the vague definition for what constitutes an application and what not. But that's something to discuss over there.

Like, if we want to show aggregate values like number of child processes running for a tree, it won't fit into any existing column. Correct me if you find I'm missing anything.

Do you mean the number of child process in the Running (R) state, or the number of child processes that have started and not dead? The latter includes processes in the Sleep (S) state and I/O waiting (D) state. You know, processes you could see in htop are usually in S state -- only a few of them would be R and consume CPU time.

That would be either multiple columns, or with multi-mode columns this could be done as one mode:

  • All processes in subtree
  • All active
  • All pending/ready
  • All idle

An idea to explore may be implementing "multi-mode" columns

"multi-mode" columns also sound like a great idea. If we're having that, then we can also allow the user to add duplicate columns (same columns with different modes) to his screen.

I will let @BenBE correct me if I get this wrong, but my vision is that the columns' mode would be global, and you won't see per-process CPU% and aggregated CPU% side by side. Only one will be displayed at a time, but a hotkey would be available to quickly switch between modes -- that is, between per-process data and aggregated data.

Both a "global column mode" and a "per-column mode" are possible options. It's basically a question of which direction we decide to go with this. But I'm more in the "per-column mode" camp with that idea, as that is the most flexible and is the closest to what this PR already tries to do.

That being said: We should keep discussions for this PR on topic and split the "multi-mode columns" and "row grouping" (e.g. applications) to separate discussions to keep this PR manageable. Those topics would need to be addressed in separate PRs anyway.

Finally, while from a technical PoV this PR LGTM (sans notes) I'm not completely happy with the overall feature (despite generally being in favour for it). The issue I think we need to address before continuing is the general discussion about whether we'd potentially want to introduce quite a few (similar) such columns, or if there's a better way to go ahead with this.

BenBE avatar Nov 27 '23 21:11 BenBE