Introduce "single value" model for monochrome graph

[kjbracey2]

The text labels for CPU and memory go to the trouble of figuring out a subset of the values to sum, for a single-value display.

But the monochrome graph simply summed all curItems values. This is less sophisticated than the label behaviour.

Add a Meter::single_value member that can be used to output a meter's single-value sum, and make the graph display use it instead of its own "all-bars" sum where available.

Implement this in MemoryMeter, SwapMeter and CPUMeter.

This has the effect of making a memory graph not show cached memory, so will not be permanently at the top when the cache is heavily used, and the CPU graph will obey the account_guest_in_cpu_meter setting. In both cases, the graph becomes consistent with the text label of the bar.

This follows discussion on #913 - it's what I was thinking of as curSumItems there. If/when #913 is implemented, single_value would become values[MEMORY_USED] + values[MEMORY_SHARED].

There are possibly other meters and/or displays that could use single_value.

[kjbracey2]

Maybe better to omit this sanitisation? Not sure about it. Platforms that intentionally return NAN also set curItems to 0, so the graph should correctly show 0.

We must always update single_value though (if we ever do), so the alternative would be to always set this->single_value = percent; before the if.

[BenBE]

If unused I think NAN should be preferred.

[kjbracey2]

This way we indicate "we have a single value and it's zero", so it will be used.

The other way we put NAN in there and that indicates "we have no single value", in which case Meter will sum curItems entries, and that should be zero, so also producing zero.

[kjbracey2]

This is also justified by htop's FAQ:

The memory meter in htop says a low number, such as 9%, when top shows something like 90%! (Or: the MEM% number is low, but the bar looks almost full. What's going on?)

The number showed by the memory meter is the total memory used by processes. The additional available memory is used by the Linux kernel for buffering and disk cache, so in total almost the entire memory is in use by the kernel. I believe the number displayed by htop is a more meaningful metric of resources used: the number corresponds to the green bars; the blue and brown bars correspond to buffers and cache, respectively (as explained in the Help screen accessible through the F1 key).

The monochrome graph should be graphing the same "more meaningful metric", not just a wall of 90%.

(Although htop is now showing something less meaningful than it was when that FAQ was written, due to the separation of shared memory, but that's being addressed in #913).

If/when there is a coloured time graph as in #714, that would be graphing the stack of curItems, which would render this change redundant, unless there was a colour/mono (stacked/simple?) graph mode setting. This change is only relevant to things wanting single metrics.

[Explorer09]

I have no idea what you are doing here. It looks like single_value you've introduced here only applies to some meter and not others. How much benefit would this bring so as to justify adding a common, Meter class member?

I would prefer these implemented
as tweakable meter options (in Display Options menu) instead.

[kjbracey2]

I have no idea what you are doing here.

I thought I'd managed to explain it to you in the previous discussion!

Once more for luck - as the htop FAQ says, simply adding together all the memory regions does not give a very useful number. On any machine with limited RAM you just tend to see usage stuck up at 90% continuously because of the cache - only dropping when you quit a program, then it just climbs back up again. You just get a graph of "when did I quit something" dips. Increases are often just cache inflation, not increased usage. Showing just "used" (and "shared") is a more useful metric.

It looks like single_value you've introduced here only applies to some meter and not others. How much benefit would this bring so as to justify adding a common, Meter class member? I would prefer these implemented as tweakable meter options (in Display Options menu) instead.

"Instead"? Certainly it could be a tweakable option - graph either the sum of all bars, or the more meaningful metric as shown in the bar label - but you'd still need to pass the value from the Meters to the displays. And there's other way to pass a value other than in the Meter class. That's where value outputs go from updateValues methods.

The other alternative I considered (and discarded rapidly) was to add a return value to the updateValues methods, similar to the return value from Platform_setCpuValues that does the same job. But that's a much bigger structural change - you'd still have to store it somewhere. CPUMeter_updateValues gets to use that return value immediately.

I haven't reviewed which other meters would benefit from using it. I think Linux swap could, at least - its "cached" is similarly not really in use.

Edit: SwapMeter now uses it to exclude cached from the graph.

[Explorer09]

@kjbracey

I thought I'd managed to explain it to you in the previous discussion!
You just get a graph of "when did I quit something" dips. Increases are often just cache inflation, not increased usage. Showing just "used" (and "shared") is a more useful metric.

That's why I proposed the color graph (#714) in the first place. Even for cache size changes, it's still worth plotting in the graph when they are colored. And I've intended the color graph be a generic solution to all meters. How your solution would benefit after the color graph solution is my main question to ask.

[kjbracey2]

And I've intended the color graph be a generic solution to all meters.

I don't understand why you seem to think this isn't generic. It's totally generic. It's just that the default behaviour is to use the sum of the values - only meters that want something different need to override it. Any meter can override it if they want. This patch makes the meters more uniform in that the bar text label of all 3 updated meters is now displaying single_value specifically - they're outputting their text display in API for the graph.

I guess we could update all meters to provide single_value and ditch the default fallback?

The coloured graph is indeed a solution to the same problem, but it's putting the onus on the user, getting them to interpret a different visualisation to interpret richer data. I'd expect it to be a tweakable mode, either per-meter or globally, so there would still be simple graphs, and they would continue to use the single_value output. The coloured graph mode wouldn't use it. If there's no tweakable setting and simple graphs go, this becomes redundant.

My previous suggestion, of an alternate item count, would offer the possibility of a third graph mode where it's coloured but omits caches, but this version doesn't. It's assuming at most two modes: "simple-omitting-caches" or "stacked-including-caches".

And those two modes are consistent with the original (or at least long-term) design of htop's bars - this combination of coloured stacked info for detailed inspection, and the simple "meaningful metric" on the text label. To have only one of those graphable is an omission. Adding the coloured graph is strongly consistent with the design. To remove the simple graph would be to regress again. There should be history graphs of both things that are displayed instantaneously in the bar.

[Explorer09]

I don't understand why you seem to think this isn't generic. It's totally generic. It's just that the default behaviour is to use the sum of the values - only meters that want something different need to override it. Any meter can override it if they want.

And I consider this a bad OO (object-oriented) design practice. Introducing a class member where not all instances would specialize its value. It should be a sub-class instead, so that polymorphism and dynamic dispatch can work.

There are meters in htop that never provide numeric values: HostnameMeter, SysArchMeter and (most importantly) BlankMeter.
You can see in those three meters, the maxItems properties are zero.

This patch makes the meters more uniform in that the bar text label of all 3 updated meters is now displaying single_value specifically - they're outputting their text display in API for the graph.

You still need to format the single_value for text display. Because the value, as a floating point, is not suitable for printing directly. You would at least format it and apply a unit (percentage unit or K, M, G, T, etc.)

I guess we could update all meters to provide single_value and ditch the default fallback?

That would make things more complicated than necessary. For meters that have one value only (Load Average, Uptime), introducing single_value means more code and more work for them.

For what I've seen. the only benefit for single_value as a class member so far would be to easily plot it in the graph display. This would be redundant once the color graph is used.

[kjbracey2]

For what I've seen. the only benefit for single_value as a class member so far would be to easily plot it in the graph display.

Yes, just like the values members and the txtBuffer member, and the total member. Meter is the class where values are stored by updateValues(Meter *) and passed to the renderers, so that's where this value goes too.

If you want to complain about the class hierarchy of htop, I'm not the person to be talking to, and this is not the PR for it.

This would be redundant once the color graph is used.

The colour graph is not currently part of the codebase. And I'd very much not want simple graphs to be removed as an option. (I think your proposed patch does, but if it went in as is, I'd re-add a per-meter or global option for them).

[kjbracey2]

Rebased.

[Explorer09]

I have one more comment on this PR. I'm not sure if single_value is a good name for the Meter class member. Maybe nominalValue would be a better name for it. How do you guys think?

[kjbracey2]

Yes, I don't like single_value much either. It is indeed a "single" value, but what single value? (And seems to have a not_camel_case failure if nothing else.)

I'd be okay with nominalValue, but still doesn't feel perfect. How about summaryValue?

That even ties in the words "summary" and "sum". A summary could well be the sum, and that's what we do by default, but it can be something else, specified in summaryValue.

[kjbracey2]

Here's a version with summaryValue. Definitely better than single_value. Commit message reworded from version still in the original PR.