Add option for hyperthread visibility in CPU meter labels#1902
Add option for hyperthread visibility in CPU meter labels#1902tjk213 wants to merge 3 commits intohtop-dev:mainfrom
Conversation
|
I have a concern about using the abbreviation "SMT" as I don't know what it refers to. Maybe you could just say "label CPU by physical cores". Your idea should be about grouping CPU meter numbers by their physical cores, right? Whether the threads would be labelled as a, b, c, etc. is second. |
|
SMT is a well-known acronym in industry, its perfectly fine to use here - it's short and precise, people who might use this feature know what it means. Looking at the linked wikipedia page also suggests it is a good term to use too - there's clearly only one possible meaning listed there that could be relevant in the context of htop. |
I would just say "multithread" or "hyperthread" rather than throw in an industry acronym that users need to look up for what it means. The users of htop could be very new to computers and if we have to explain an acronym in the UI, it fails at being user-friendly. Note that in the linked Wikipedia page SMT could also refer to surface-mount technology (although I've heard of SMD being used more often than SMT in that context). At first glance I was confused with "smt" being an ugly shorthand for "smart", but anyway I would vote for spelling out the word "multithread" (or "multithreading"). |
|
I would agree that SMT is a standard term & that there is really only one possible fit on that wiki page. With that said, happy to rename the flag if there is consensus on a better name. Note that the description in the UI uses the term HyperThreads as well, for users who find that more familiar: |
BenBE
left a comment
There was a problem hiding this comment.
I would agree that SMT is a standard term & that there is really only one possible fit on that wiki page. With that said, happy to rename the flag if there is consensus on a better name.
You can keep SMT. No need for using Intel's proprietary name. Also multithreading can be mistaken for running multiple threads (LWP) of a process, thus also something you'd better avoid.
Note that the description in the UI uses the term HyperThreads as well, for users who find that more familiar:
Show SMT/HyperThread siblings (e.g. 0a, 0b) instead of CPU number
I'd suggest to hint to the CPU topology in addition to SMT, which should make the intent of this setting more obvious even to people who don't know what SMT is.
CPUMeter.c
Outdated
| char threadLetter = 'a' + (char)threadIndex; | ||
| xSnprintf(caption, sizeof(caption), "%2d%c", coreID, threadLetter); |
There was a problem hiding this comment.
Properly handle more than 26 threads per core …
I know this is rare, but … ;-)
When I googled the term "SMT", the first few results I got was pages about surface mount technology. It suggested me that the "SMT" is not that universally used as you would think. Also, I don't think user would confuse CPU threads with process threads. Those are different things.
EDIT: Read this comment for my updated opinion. I no longer object the term SMT but I would like clarification through surrounding text. |
| if (host->settings->showCPUSMTLabels) { | ||
| int coreID = Machine_getCPUPhysicalCoreID(host, cpu - 1); | ||
| int threadIndex = Machine_getCPUThreadIndex(host, cpu - 1); | ||
| char threadLetter = 'a' + (char)(threadIndex % 26); |
There was a problem hiding this comment.
@BenBE I added a wrap so any systems with more than 26 threads per core will just start reusing letters. does that work for you, or would you prefer something else?
There was a problem hiding this comment.
Most I've ever heard of is 8 per core, so this seems a sensible approach. Consider adding an assert() so that some hapless developer someday will hit this if ever that limit is reached.
There was a problem hiding this comment.
If we distinguish letter cases (a-z and A-Z) then the number of addressable threads can be bumped up to 52. However, I am not sure what's the use case. Since we took a character in the CPU meter label to number the thread, then the maxinum addtressable physical cores went down to 99, and that's another limit worth concerning (and discussion).
EDIT: By the way, would there be users who prefer labeling the cores by letters and threads by numbers?
There was a problem hiding this comment.
While wrapping around (and throwing an assert) mitigates the immediate problem, I think a proper fallback like %d:%d might be a more sustainable way to go.
Two options available: Use single letter for first like 26 (or 16) threads, numbers for anything above, or always use numbers if there's some core with more than 26 (or 16) threads. Doing the latter is more complexity to implement, but gives a cleaner UI/UX.
There was a problem hiding this comment.
@BenBE There are only three characters of space for a label in a meter, so the thread numbering has to constrain to one or two characters. The extra character like a colon : is not an option.
I think a much easier, temporary solution is to mark threads greater than 26 as * instead of a letter. Yes, that would create duplicate labels, but as long as the threads are grouped together, I don't see that would create a big problem.
There was a problem hiding this comment.
if there is a clear consensus on what you folks would like to see I'm happy to implement any of these suggestions.
+1 to @Explorer09's point that expanding beyond 3 characters creates more problems than it solves.
My take is that we should leave this as-is, with just a wrap around the lower-case letters and no assert. The main intent here is just to serve as a reminder that "not all cores are created equal" and make it clear how the cores are paired. with the default setting, one might naively assume that hyperthreads are co-located (even/odd) rather than the more common hi/lo pattern. This intent is still accomplished even if some letters are repeated. at SMT-26, you'd have many columns & rows of meters, the repeated letters would be so far apart from each other that you'd hardly notice.
And of course the setting is off by default, so if there is some system out there with SMT-26 and the user doesn't like the repeated letters, they can just not use the setting.
There was a problem hiding this comment.
I am against repeating letters, by the way. Imagine when you communicate with someone and point out that threads "16a" to "16d" are always 100% busy, and your friend asks you which "16a" thread you are referring to.
An asterisk would be better if we run out of letters.
|
@tjk213 Please rebase and squish your commits to show the necessary changes instead of the path taken to get there. Doing like 3 commits (one introducing the setting, one introducing the data collection required, and one implementing the display) fully suffices. No need to spam like 14 commits at this change. ;-) |
d6e3aad to
cbc1265
Compare
This PR adds a
show_smt_cpu_labelssetting which, when enabled, labels cpu meters with aX{a,b,c,d}convention whereXis the physical core ID anda/b/c/dindicates which hyperthread on the core the labeled meter is watching. For example, 8 cores with SMT-2 renders as follows:The setting is off by default and currently only implemented for Linux.