Incorporate shared memory in bar text

[kjbracey2]

Shared memory is claimed, and as significant a part of the memory load as the "used" memory. Since "shared" was separated from the "used" value, the basic "used/total" display in the bar text has become less meaningful for Linux, as it only reflects a subset of the claimed memory. The difference often isn't huge, but it can become so if tmpfs is heavily used.

Improve the situation by adding used and shared for that text, making it "claimed/total".

In accordance with that logic, move the shared value leftwards next to the used value, so that we're summing the two leftmost values - a contiguous region.

Fixes #906.

(This could be separated into two PRs, but they would conflict - doing it as two patches).

[kjbracey2]

Actually, when shared was separated (3.1.0), I think the bar text was actually displaying (total - available)/total, which included tmpfs, but #838 (3.1.1) changed it to used/total, losing tmpfs.

I think this is good compromise - tmpfs is back in there, and it's still based it on the displayed bars.

[BenBE]

I'm not really happy with changing the order in which values are displayed as this will likely interfere with platforms other than Linux.

Apart from this I don't like introducing so much noise for just changing the values included in the calculation.

Why not used+buffers+shared instead?

[kjbracey2]

changing the order in which values are displayed as this will likely interfere with platforms other than Linux.

My belief was that it wouldn't affect other systems at all - they don't have a shared, so it's a no-op for them. Internally, htop just skips value 1 instead of skipping value 2. This basically repeats #566, moving shared one more space.

Why not used+buffers+shared instead?

Good question. As written, it's simply because buffers has never been part of used. I was thinking of only consequences of the used->used+shared split.

Research seems to confirm that the buffers are potentially reclaimable in Linux - they're part of the cache that /proc/meminfo chooses to separate out. Raw disk blocks or other filing system metadata that don't correspond to file contents, but still potentially as droppable.

Source: https://unix.stackexchange.com/questions/440558/30-of-ram-is-buffers-what-is-it

So I don't think it should be included. And it also changes the answer on other platforms. Including only shared doesn't, as only Linux has that.

[kjbracey2]

There's a related point here with respect to the graph display - that basically shows full all the time on Linux, because it's summing curItems, so incorporates cache.

curItems being 4 is appropriate for the bar, which is showing a stack in different colours, so you can distinguish the reclaimable cache+buffers, but it's not good for a simple combined total like the graph.

I would suggest there should be a separate curSumItems set to 2 (used + shared) used to get the plotted graph value, and that then corresponds to the bar text value, and does assume the reordering.

[Explorer09]

@kjbracey

1. You should also swap the shared/buffer entries in the Memory Meter help text. I mean the actionHelp() function in Action.c.
2. I have implemented a color graph display in a separate PR (Dynamic scaling & Graph meter coloring (new) #714), I think your adjustment of curItems in MemoryMeter would be unneeded and would potentially break the graph display after my PR is applied.

[kjbracey2]

Thanks for feedback - I like the graph colouring - that addresses the problem in a different way.

My proposal wouldn't break it. I was suggesting a separate curSumItems to indicate how many to sum for a simple singular total (like a monochrome graph, or the text). A stacked display showing regions like the bars or your coloured graph could continue to use curItems.

[Explorer09]

@kjbracey The curItems is useful for hiding certain items from bar and graph display. IIRC, the CPU Meters use it for hiding CPU temperatures as they are not use percentages and the temperatures are only shown in text.
I don't know what your curSumItems proposal is meant to do.

[kjbracey2]

Sounds like I may need to review more carefully the meaning of curItems, but if that's a "how many entries to display in a stacked graph", mine is "how many entries to add up to get a summary value". Those are potentially different.

Memory has 5 entries, of which you want 4 in a stacked graph. But given that the cache naturally fills all unused space, it's not that useful to just add the 4 together to get a total if you can't see how much is cache. Excluding the cache gives a more useful number. And that's what the text on the bar already does.

curSumItems is API for that concept - MemoryMeter would set it to 2 (if including shared as per this PR), or 1 (reflecting current behavior). A monochrome graph (or simple bar?) or a generic equivalent of the bar label would use curSumItems. A stacking thing like the current bar or your coloured graph would use curItems. (They should be the same by default - I don't know if there's anything other than MemoryMeter that would benefit from them being different).

Names could be clarified. Maybe curItems should be curStackItems. It only limits stacked graphical displays, right?

[Explorer09]

@kjbracey No, the cache doesn't technically fill all unused space of the memory, in Linux at least.
You only need to change the txtBuffer (display text) of the Memory Meter to achieve what you want (add shared to the total "used" memory for summary display). I don't see a new curSumItems can be any useful for now. Let's keep the changes small.

[kjbracey2]

No, the cache doesn't technically fill all unused space of the memory, in Linux at least.

Indeed, but it tends to fill it up meaning that a simple total including it does not reasonably reflect current memory demands, so is generally less useful than it would be without including it.

And that's why the bar text has never included it. And it's why the monochrome graph isn't very useful, and why your coloured changes are a significant benefit compared to the monochrome, by making the cache perceivable.

I'm not doing the curSumItems in this patch - I'm just describing it as a concept to help justify the reordering, making the summed items contiguous. It would be useful to make the monochrome graph work better, making it graph the useful value shown in the bar label. It would have no impact on your graph display, except that that apparently totally replaces monochrome mode, so it would become redundant.

[kjbracey2]

I've rethought my curSumItems thing I talked about above, and submitted it as #928. (This new form does not depend on summing contiguous values, so if the bars don't get reordered, it would still work).

[BenBE]

@kjbracey2 Please rebase this branch and resolve the issues in the discussion as you go. If there's further input you need, feel free to ask.

[kjbracey2]

Rebased - not sure what to address, aside from the isnan - check that.

Note that #1153 had left the help inconsistent with the actual display - this corrects that as it goes, which maybe makes it less clear. Maybe I should add an initial commit fixing that.

[kjbracey2]

Okay, separated out the help correction.

Haven't reworded the commit message, but the effect of the second commit is now actually more to move "buffers" - the thing about "summing a discontiguous region" was now already happening with used+compressed and buffers in between.

[BenBE]

LGTM. Mostly the isnan vs isgreater change left.

[BenBE]

Maybe use isgreater as suggested in the other discussion with @Explorer09 …

[kjbracey2]

Awaiting clarification on the isgreater - @Explorer09 seems to be referring to code that isn't on main.

[Explorer09]

Awaiting clarification on the isgreater - @Explorer09 seems to be referring to code that isn't on main.

I stand corrected. The code surely isn't on main. When I was working with the color graph code in my personal branch, I have been using isgreater instead of isnan in many places, and I thought some of the isgreater uses have been merged back to main and so I was confused.

Here is my suggestion: start using isgreater for new code now when it makes sense. If the values are assumed to never go negative, then check using isgreater, not just isnan - it would make the code a lot safer.

[BenBE]

@Explorer09 Can you check which PR the changes for isgreater are in and try to separate them to their own commit somehow, such that they can be pulled onto main? Though only necessary if the PR is somewhat larger otherwise.

[intelfx]

Is there something that can be done to get this merged?

I believe the isPositive() rework has since landed in main, and #906 is a visible and user-affecting issue -- I just had to explain this exact situation to a very confused associate :-)

[intelfx]

I like the ordering changes, +1 on that.

However, I disagree with the wording changes. "Claimed" is completely new terminology that is never used anywhere else, so I don't think it would be helpful to replace an established (even if ambiguous) term with a completely new and never-before-used word. More importantly, "used" memory is how coreutils' free(1) calls it. Thus I am of opinion that htop should just match that terminology.

(I have rebased the PR's branch on top of main, dropping wording changes, here.)

[BenBE]

I like the ordering changes, +1 on that.

PR LGTM.

However, I disagree with the wording changes. "Claimed" is completely new terminology that is never used anywhere else, so I don't think it would be helpful to replace an established (even if ambiguous) term with a completely new and never-before-used word. More importantly, "used" memory is how coreutils' free(1) calls it. Thus I am of opinion that htop should just match that terminology.

ACK.

(I have rebased the PR's branch on top of main, dropping wording changes, here.)

I pulled in your changes. Thx.

Is there something that can be done to get this merged?

AFAICS there's some open discussions from @Explorer09 . If those could be marked resolved if no longer applicable we are GTG.

I believe the isPositive() rework has since landed in main, and #906 is a visible and user-affecting issue -- I just had to explain this exact situation to a very confused associate :-)

ACK.

[Explorer09]

@BenBE What open discussion did I have now?

Regarding isnan vs isgreater etc., the code has been rebased onto the new main which incorporated #1271, and thus it could be considered resolved.
The other discussion I saw is about using the "summary value" instead of the naïve sum of the meter. AFAIK, this is discussed separately in #928 (this PR doesn't include the "summary value" part in order to keep the changes small).

What else did I miss?

[BenBE]

That was the only one. GTG …

[kjbracey2]

However, I disagree with the wording changes. "Claimed" is completely new terminology that is never used anywhere else, so I don't think it would be helpful to replace an established (even if ambiguous) term with a completely new and never-before-used word. More importantly, "used" memory is how coreutils' free(1) calls it. Thus I am of opinion that htop should just match that terminology.

The corollary then is that the other usage of "used" should be changed, removing the ambiguity within htop, and avoiding using that coreutils term for something different.