MemoryMeter code refactoring & FreeBSD memory number fixes #1829

Explorer09 · 2025-11-30T19:21:08Z

Slightly refactor MemoryMeter code so that the "used" number and the "Show Cached Memory" option behavior can be tuned per platform.

Then, I modified and applied @PierreMarieBaty's patch so that the MemoryMeter in FreeBSD can display number closer to FreeBSD's top (it can never be 1:1 matching).

This PR supersedes #1824.
Resolves #1725.

(I would like @natoscott and @PierreMarieBaty to review this.)

Add a new output parameter 'usedNumber' to Platform_setMemoryValues() function. This allows different platform codes to have different rules about what memory categories count as 'used' memory in the bar display mode. Signed-off-by: Kang-Che Sung <[email protected]>

BenBE

The NULL checks apply to all platforms; I just noted them just once to stay DRY …

BenBE · 2025-11-30T20:16:22Z

MemoryMeter.c

+   double usedNumber = 0;
+   Platform_setMemoryValues(this, &usedNumber);
+
+   written = Meter_humanUnit(buffer, usedNumber, size);


I can't exactly say I love this change of the function signature here, but given the background behind this change, we'd probably have to expose far to much logic here otherwise. I'll leave this for the others as a note.

The expectation is Platform_setMemoryValues would be always inlined in MemoryMeter_updateValues, so that the Platform function signature won't be a big deal.

"usedNumber" doesn't offer explanatory value here - "totalUsed" would be more descriptive IMO.

This approach (function side-effect instead of results in the Meter structure) is used to solve a similar problem elsewhere, so I think its the best we can do for the text buffer case. Its cleaner in the way it handles platform-specific-code IMO, so +1 (with the name change) for this part.

@natoscott I have a concern that we have overloaded the meaning of "used" memory here. And I might be better if I rename the first memory item from MEMORY_METER_USED to MEMORY_METER_ACTIVE. There's one thing that someone needs to help me check out:

The Linux /proc/meminfo has a field named Active: and whether the Active memory number maps to the "used" memory in our MemoryMeter. (I'm primarily using macOS for now so I can't check the Linux platform with confidence.)

BenBE · 2025-11-30T20:19:13Z

MemoryMeter.c

+   /* 'buffers' memory field might be hidden by a setting in some OS
+      ports (e.g. FreeBSD). */
+   if (isNonnegative(this->values[MEMORY_METER_BUFFERS])) {
+      Meter_humanUnit(buffer, this->values[MEMORY_METER_BUFFERS], sizeof(buffer));
+      RichString_appendAscii(out, settings->showCachedMemory ? CRT_colors[METER_TEXT] : CRT_colors[METER_SHADOW], " buffers:");
+      RichString_appendAscii(out, settings->showCachedMemory ? CRT_colors[MEMORY_BUFFERS_TEXT] : CRT_colors[METER_SHADOW], buffer);
+   }

   Meter_humanUnit(buffer, this->values[MEMORY_METER_CACHE], sizeof(buffer));
   RichString_appendAscii(out, settings->showCachedMemory ? CRT_colors[METER_TEXT] : CRT_colors[METER_SHADOW], " cache:");
   RichString_appendAscii(out, settings->showCachedMemory ? CRT_colors[MEMORY_CACHE] : CRT_colors[METER_SHADOW], buffer);

   /* available memory is not supported on all platforms */
   if (isNonnegative(this->values[MEMORY_METER_AVAILABLE])) {
      Meter_humanUnit(buffer, this->values[MEMORY_METER_AVAILABLE], sizeof(buffer));
      RichString_appendAscii(out, CRT_colors[METER_TEXT], " available:");
      RichString_appendAscii(out, CRT_colors[METER_VALUE], buffer);
   }


If we already go for the isNonnegative check for some fields, wouldn't it be logical to just go one step further ad do it for every field? That way you could even hide the caches on platforms where determining these values was not feasible or too complicated or there was some option to hide those too …

Yes. We could have isNonnegative check for all memory fields, but I want to keep the changes small. If there is an OS when "cache" memory is not applicable, the porter can add that check in the future.

@BenBE I agree.

And its not a great leap from that to solving the underlying problem here, which is not yet tackled by any of these changes, and that is that we still do not correctly represent FreeBSD memory categories. We should be listening more to what @PierreMarieBaty has said, and not trying to force FreeBSD into categories that don't properly fit.

If we're OK with having MEMORY_METER_COMPRESSED and MEMORY_METER_AVAILABLE (which are platform-specific) then we are also OK with having MEMORY_METER_WIRED, for example.

unsupported/Platform.c

BenBE · 2025-11-30T20:22:14Z

solaris/Platform.c

+         this->values[MEMORY_METER_BUFFERS] = 0;
+         this->values[MEMORY_METER_CACHE] = 0;


Shouldn't these be NAN instead?

Doesn't matter. In Bar and Graph meter modes both 0 and NAN would hide the value from display.

solaris/Platform.c

PierreMarieBaty · 2025-11-30T23:49:42Z

In freebsd/FreeBSDMachine.c, delete everything related to the shared memory, i.e.: delete the MIB_vm_vmtotal variable, delete the corresponding sysctl call, delete the sharedMem variable and don’t assign any value to super->sharedMem. As I said elsewhere, you cannot know which OS-specific memory classes this shared memory belongs to: active, wired or laundry. When it gets bigger than all 3 separately, you end up with a negative number in the htop category from which you subtract it, and this makes the meter incorrectly display a full bar. It happened to me yesterday. For this reason, and contrarily to what I thought first, setting any value in MEMORY_METER_SHARED in FreeBSD is wrong. On the other hand, it is licit to set a value into MEMORY_METER_BUFFERS because in FreeBSD the buffered memory is guaranteed to always belong to the OS-specific "wired" class, and thus can never be greater than it. I have no objection on the rest. Only regrets that I’ll have to maintain my own fork, but it doesn’t matter.

natoscott

In addition to the issues pointed out, I also take issue with the way you have stated co-authored-by on the first commit. This gives the impression that the commit was a collaboration, that Pierre-Marie agrees with it, and that you worked on it together. This is not the case at all.

Give the proper attribution from PR #1824 - since that is Pierre-Marie's work - and then add to it with your own commits.

natoscott · 2025-12-01T01:04:06Z

MemoryMeter.c

+   double usedNumber = 0;
+   Platform_setMemoryValues(this, &usedNumber);
+
+   written = Meter_humanUnit(buffer, usedNumber, size);


"usedNumber" doesn't offer explanatory value here - "totalUsed" would be more descriptive IMO.

This approach (function side-effect instead of results in the Meter structure) is used to solve a similar problem elsewhere, so I think its the best we can do for the text buffer case. Its cleaner in the way it handles platform-specific-code IMO, so +1 (with the name change) for this part.

natoscott · 2025-12-01T01:07:12Z

MemoryMeter.c

+   /* 'buffers' memory field might be hidden by a setting in some OS
+      ports (e.g. FreeBSD). */
+   if (isNonnegative(this->values[MEMORY_METER_BUFFERS])) {
+      Meter_humanUnit(buffer, this->values[MEMORY_METER_BUFFERS], sizeof(buffer));
+      RichString_appendAscii(out, settings->showCachedMemory ? CRT_colors[METER_TEXT] : CRT_colors[METER_SHADOW], " buffers:");
+      RichString_appendAscii(out, settings->showCachedMemory ? CRT_colors[MEMORY_BUFFERS_TEXT] : CRT_colors[METER_SHADOW], buffer);
+   }

   Meter_humanUnit(buffer, this->values[MEMORY_METER_CACHE], sizeof(buffer));
   RichString_appendAscii(out, settings->showCachedMemory ? CRT_colors[METER_TEXT] : CRT_colors[METER_SHADOW], " cache:");
   RichString_appendAscii(out, settings->showCachedMemory ? CRT_colors[MEMORY_CACHE] : CRT_colors[METER_SHADOW], buffer);

   /* available memory is not supported on all platforms */
   if (isNonnegative(this->values[MEMORY_METER_AVAILABLE])) {
      Meter_humanUnit(buffer, this->values[MEMORY_METER_AVAILABLE], sizeof(buffer));
      RichString_appendAscii(out, CRT_colors[METER_TEXT], " available:");
      RichString_appendAscii(out, CRT_colors[METER_VALUE], buffer);
   }


@BenBE I agree.

And its not a great leap from that to solving the underlying problem here, which is not yet tackled by any of these changes, and that is that we still do not correctly represent FreeBSD memory categories. We should be listening more to what @PierreMarieBaty has said, and not trying to force FreeBSD into categories that don't properly fit.

If we're OK with having MEMORY_METER_COMPRESSED and MEMORY_METER_AVAILABLE (which are platform-specific) then we are also OK with having MEMORY_METER_WIRED, for example.

PierreMarieBaty · 2025-12-01T01:39:56Z

@natoscott

"Give the proper attribution from PR #1824 - since that is Pierre-Marie's work - and then add to it with your own commits."

I totally don’t care. I’m not seeking reputation, and even if I was, having one single attribution on GitHub would be more damaging than having none 😅

"If we're OK with having MEMORY_METER_COMPRESSED and MEMORY_METER_AVAILABLE (which are platform-specific) then we are also OK with having MEMORY_METER_WIRED, for example."

…and although desirable that would be no less wrong, because even the fact that whether these categories overlap or not is OS-specific. Where would you put wired+compressed memory, for example ? All memory categories you define in htop must belong to the same total and non-overlapping representation.And that is impossible from the moment you decide htop is portable. I renounced to try and convince that there can hardly be something more OS-specific than memory management. It seems like only hands-on experience of system programming on various OSes can yield this conviction.

natoscott · 2025-12-01T01:49:45Z

Where would you put wired+compressed memory, for example ?

Hmm, I'm not sure what you're asking - that doesn't make sense as it doesn't exist on any platform. On platforms with 'wired' you'd get a value for wired and compressed wouldn't be represented at all (NaN in the Meter values array), same for 'available', same for 'compressed',... we don't have to make up a "fake" value just for the sake of portability, if that's what you're asking?

natoscott · 2025-12-01T01:55:11Z

[re attribution] I totally don’t care.

Heh, that's OK - we (the htop maintainers) do care. We'll be looking back at it for years to come, when we next have to tackle a bug or review a fix here, so reflecting the reality of the progression of changes is useful for us.

Oh, while I have you - there's a comment in your patch (couple of places actually) stating one value should be "deduced" from another. I think you meant "deducted" in those places - can you confirm? Thanks.

PierreMarieBaty · 2025-12-01T02:03:14Z

Where would you put wired+compressed memory, for example ?

Hmm, I'm not sure what you're asking - that doesn't make sense as it doesn't exist on any platform. On platforms with 'wired' you'd get a value for wired and compressed wouldn't be represented at all (NaN in the Meter values array), same for 'available', same for 'compressed',... we don't have to make up a "fake" value just for the sake of portability, if that's what you're asking?

That was a theoretical example. I should have picked "shared" + "wired", that would have been more meaningful. Whether pages can be swapped out determines whether they'd belong to the "wired" class or to the "active" class. If you implement a "wired" class to complement the "used/active" class, then you cannot have a class in the same representation that could accommodate either wired pages or active pages. Categories mustn't overlap - and whether they do or not is purely OS-specific because it depends on the meaning the OS gives to these labels. That's the reason why I asked to remove the MEMORY_METER_SHARED fillings from the FreeBSD code.

Oh, while I have you - there's a comment in your patch (couple of places actually) stating one value should be "deduced" from another. I think you meant "deducted" in those places - can you confirm? Thanks.

Sorry... English is not my mother tongue - in French the verb "déduire" means both (subtracting from something and figuring something out by logic). Depending on what the code below it does, please fix my broken English as appropriate.

natoscott · 2025-12-01T02:15:04Z

That was a theoretical example. I should have picked "shared" + "wired", that would have been more meaningful. Whether pages can be swapped out determines whether they'd belong to the "wired" class or to the "active" class. If you implement a "wired" class to complement the "used/active" class, then you cannot have a class in the same representation that could accommodate either wired pages or active pages. Categories mustn't overlap - and whether they do or not is purely OS-specific because it depends on the meaning the OS gives to these labels. That's the reason why I asked to remove the MEMORY_METER_SHARED fillings from the FreeBSD code.

Understood and agreed - for any given platform, the categories mustn't overlap and need to sum up to 100% of total memory.

Oh, while I have you - there's a comment in your patch (couple of places actually) stating one value should be "deduced" from another. I think you meant "deducted" in those places - can you confirm? Thanks.

Sorry... English is not my mother tongue -

No problem, no apology needed.

in French the verb "déduire" means both (subtracting from something and figuring something out by logic).

Interesting!

Depending on what the code below it does, please fix my broken English as appropriate.

OK, will do.

PierreMarieBaty · 2025-12-01T02:24:34Z

Understood and agreed - for any given platform, the categories mustn't overlap and need to sum up to 100% of total memory.

And that's impossible if generic categories are used for all platforms. Cf. the debate in #1824. It's unfortunate I failed to convince.

Explorer09 · 2025-12-01T03:14:12Z

In addition to the issues pointed out, I also take issue with the way you have stated co-authored-by on the first commit. This gives the impression that the commit was a collaboration, that Pierre-Marie agrees with it, and that you worked on it together. This is not the case at all.

Give the proper attribution from PR #1824 - since that is Pierre-Marie's work - and then add to it with your own commits.

What did I miss? In the last commit the main author is Pierre-Marie and I didn't change it. Am I supposed to remove my name from the "co-authored-by" line or what? I'm fine with waiving the credit of my modification, by the way.

Explorer09 · 2025-12-01T03:40:54Z

That was a theoretical example. I should have picked "shared" + "wired", that would have been more meaningful. Whether pages can be swapped out determines whether they'd belong to the "wired" class or to the "active" class. If you implement a "wired" class to complement the "used/active" class, then you cannot have a class in the same representation that could accommodate either wired pages or active pages. Categories mustn't overlap - and whether they do or not is purely OS-specific because it depends on the meaning the OS gives to these labels. That's the reason why I asked to remove the MEMORY_METER_SHARED fillings from the FreeBSD code.

Understood and agreed - for any given platform, the categories mustn't overlap and need to sum up to 100% of total memory.

It seems that Pierre-Marie finally understood the constraints that htop has with the memory meter that I have argued (even though I didn't explain it clear enough). If the regions were overlapping they cannot be plotted onto the bar.

I have no problem with removing the "shared" from MemoryMeter for FreeBSD. But I can't tell whether the "shared" memory in Linux has the same issue.

PierreMarieBaty · 2025-12-01T04:02:01Z

It seems that Pierre-Marie finally understood the constraints that htop has with the memory meter that I have argued (even though I didn't explain it clear enough). If the regions were overlapping they cannot be plotted onto the bar.

Either you’re trolling, or you consistently and thoroughly fail at identifying the root cause of the problem. The problem is that you insist at sticking to arbitrary constraints (generic memory classes) that jeopardize htop’s portability (because the fact that your memory classes overlap or not is totally dependent on the OS that feeds the numbers you want to see there). This is a double-bind. Generic classes are a logical impossibility. Consequently memory representation must be OS-specific. That’s what I’m saying from the start and now you pretend that I misunderstood the constraint of the very fallacy I denounce? You are either an intellectually dishonest person or a very poor entertainer, sir.

I have no problem with removing the "shared" from MemoryMeter for FreeBSD. But I can't tell whether the "shared" memory in Linux has the same issue

If you’re not able to tell whether it’s the case on the very OS htop was initially developed for, you’d better stay away from the part of the code that deals with memory representation and let more knowledgeable persons maintain it. End of discussion. Bye.

Explorer09 · 2025-12-01T04:21:20Z

@PierreMarieBaty

It seems that Pierre-Marie finally understood the constraints that htop has with the memory meter that I have argued (even though I didn't explain it clear enough). If the regions were overlapping they cannot be plotted onto the bar.
Either you’re trolling, or you consistently and thoroughly fail at identifying the root cause of the problem. The problem is that you insist at sticking to arbitrary constraints (generic memory classes) that jeopardize htop’s portability (because the fact that your memory classes overlap or not is totally dependent on the OS that feeds the numbers you want to see there).

I'm not trolling. And yet I did fail at identifying the root causes. But no, the problem wasn't on whether the memory regions overlap.

This is a double-bind. Generic classes are a logical impossibility. Consequently memory representation must be OS-specific. That’s what I’m saying from the start and now you pretend that I misunderstood the constraint of the very fallacy I denounce? You are either an intellectually dishonest person or a very poor entertainer, sir.

I disagree. OS-specific classes for all memory will become a maintenance hell for a cross-platform app like htop. The key of maintaining htop's "portability" (the "portability" term I use might not be what you defined "portability" as) is to find common things across all platforms and make abstract inferfaces over them. So, for example, the "free" memory has to be common. Making OS-specific subcategories for what is considered "free" is acceptable, but the general categories cannot go away.

And I forgot to mention that the color schemes, which is also a factor that forces us to make common categories and not be able to tune everything for every OS.

natoscott · 2025-12-01T05:48:39Z

It's unfortunate I failed to convince.

@PierreMarieBaty FWIW, I was convinced some time ago and agree there's a major flaw with the current approach. I'm sure if we all work together we'll be able to come up with a good solution for all platforms.

"Pierre-Marie finally understood" [...]

Either you’re trolling, [...]

I'm not trolling.

@Explorer09 maybe you don't realise it, but it is trolling - when you state its taken so long to understand in this way (finally), the implication that most people will see is that the person is slow.

It's disappointing to see a new contributor being treated poorly, again. I want to reiterate and reinforce the message that was sent earlier today over in #1829. Please stop behaving in these ways. Take your time and be more thoughtful in how you respond to messages, please, we are hoping and expecting to see some changes. Always consider the perspective of the people reading your message and whether it might be hurtful, or disrespectful, or demeaning to someone - if so, do not send it.

If you don't understand what is being said here, or cannot change this behaviour for some reason, send BenBE, DLange and/or myself some private mail to discuss further. Alternatively, let's schedule a call to consider ways we can help with further understanding the damage being caused and possible remediation.

Thanks!

Explorer09 · 2025-12-01T06:05:36Z

FWIW, I was convinced some time ago and agree there's a major flaw with the current approach. I'm sure if we all work together we'll be able to come up with a good solution for all platforms.

I'd like to see how you would explain the situation.

"Pierre-Marie finally understood" [...]

Either you’re trolling, [...]

I'm not trolling.

@Explorer09 maybe you don't realise it, but it is trolling - when you state its taken so long to understand in this way (finally), the implication that most people will see is that the person is slow.

It's disappointing to see a new contributor being treated poorly, again. I want to reiterate and reinforce the message that was sent earlier today over in #1828. Please stop behaving in these ways.

I have had long debates with other people before, but, honestly I didn't see the trolling aspects here. I just wished a better code quality for htop, and avoid destructive code changes where they don't make sense.

If you understood the technical issues here, it would be good to explain to me those parts, because I really don't believe Pierre-Marie's proposed solution is viable in the long run.

I know the need for OS-specific memory value calculations, and that's why I made this PR to refactor the related code (and it took me a night for doing it).

If you don't understand what is being said here, or cannot change this behaviour for some reason, send BenBE, DLange and/or myself some private mail to discuss further. Alternatively, let's schedule a call to consider ways we can help with further understanding the damage being caused and possible remediation.

Thanks!

I accept the discussion of the talking behavior thing in a private email.

natoscott · 2025-12-01T07:02:01Z

I accept the discussion of the talking behavior thing in a private email.

Great, thanks for being open to it - I'm out of time today, will send a note through tomorrow.

Add a new output parameter 'totalUsed' to Platform_setMemoryValues() function. This allows different platform codes to have different rules about what memory categories count as 'used' memory in the bar display mode. Signed-off-by: Kang-Che Sung <[email protected]>

No behavior changes in this commit. But this allows platform code to fine tune the Memory meter behavior in the future. Signed-off-by: Kang-Che Sung <[email protected]>

This change is to make the code slightly more robust. The 'available' memory is never drawn in bar or graph, so set the value to 0 when the meter mode is Bar or Graph. Signed-off-by: Kang-Che Sung <[email protected]>

Resolves htop-dev#1725

Explorer09 · 2025-12-01T17:32:16Z

I attributed the last commit to Pierre-Marie Baty as the author, but GitHub somehow failed to associate the email address to his account. (If it was successful then the commit should have shown his avatar picture.) The commit shows only my avatar picture but this is not my fault.

natoscott

IMO the freshly reworked #1824 addresses the core underlying problem better here, so its a NAK from me on this PR.

BenBE reviewed Nov 30, 2025

View reviewed changes

natoscott reviewed Dec 1, 2025

View reviewed changes

Explorer09 added 3 commits December 2, 2025 00:50

MemoryMeter: Handle "show cached memory" in platform code

d4f9029

No behavior changes in this commit. But this allows platform code to fine tune the Memory meter behavior in the future. Signed-off-by: Kang-Che Sung <[email protected]>

Linux & PCP: Don't set 'available' memory value in bar mode

c21814e

This change is to make the code slightly more robust. The 'available' memory is never drawn in bar or graph, so set the value to 0 when the meter mode is Bar or Graph. Signed-off-by: Kang-Che Sung <[email protected]>

Explorer09 force-pushed the freebsd-memory branch 2 times, most recently from 0f26b98 to 99c0bbf Compare December 1, 2025 17:23

FreeBSD: corrections to the used memory calculations

b53997f

Resolves htop-dev#1725

Explorer09 force-pushed the freebsd-memory branch from 99c0bbf to b53997f Compare December 1, 2025 17:28

Explorer09 mentioned this pull request Dec 2, 2025

Refactor and move the Compat code to assist portability #1832

Merged

natoscott requested changes Dec 6, 2025

View reviewed changes

Explorer09 marked this pull request as draft December 6, 2025 17:20

Explorer09 mentioned this pull request Dec 6, 2025

Platform-specific memory meter design discussion #1834

Open

		this->values[MEMORY_METER_BUFFERS] = 0;
		this->values[MEMORY_METER_CACHE] = 0;

Uh oh!

MemoryMeter code refactoring & FreeBSD memory number fixes #1829

Are you sure you want to change the base?

MemoryMeter code refactoring & FreeBSD memory number fixes #1829

Uh oh!

Conversation

Explorer09 commented Nov 30, 2025

Uh oh!

BenBE left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

PierreMarieBaty commented Nov 30, 2025 via email

Uh oh!

natoscott left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PierreMarieBaty commented Dec 1, 2025 via email • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

natoscott commented Dec 1, 2025

Uh oh!

natoscott commented Dec 1, 2025

Uh oh!

PierreMarieBaty commented Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

natoscott commented Dec 1, 2025

Uh oh!

PierreMarieBaty commented Dec 1, 2025

Uh oh!

Explorer09 commented Dec 1, 2025

Uh oh!

Explorer09 commented Dec 1, 2025

Uh oh!

PierreMarieBaty commented Dec 1, 2025 via email

Uh oh!

Explorer09 commented Dec 1, 2025

Uh oh!

natoscott commented Dec 1, 2025

Uh oh!

Explorer09 commented Dec 1, 2025

Uh oh!

natoscott commented Dec 1, 2025

Uh oh!

Explorer09 commented Dec 1, 2025

Uh oh!

natoscott left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

PierreMarieBaty commented Dec 1, 2025 via email •

edited

Loading

PierreMarieBaty commented Dec 1, 2025 •

edited

Loading