Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions Action.c
Original file line number Diff line number Diff line change
Expand Up @@ -781,9 +781,12 @@ static Htop_Reaction actionHelp(State* st) {
addbartext(CRT_colors[CPU_GUEST], "/", "guest");
addbartext(CRT_colors[CPU_IOWAIT], "/", "io-wait");
addbartext(CRT_colors[BAR_SHADOW], " ", "used%");
} else if (CRT_colorScheme == COLORSCHEME_MONOCHROME) {
addbartext(CRT_colors[CPU_STEAL], "/-/-/", "virtualized");
addbartext(CRT_colors[BAR_SHADOW], " ", "used%");
} else {
addbartext(CRT_colors[CPU_GUEST], "/", "virt");
addbartext(CRT_colors[BAR_SHADOW], " ", "used%");
addbartext(CRT_colors[CPU_STEAL], "/", "virtualized");
addbartext(CRT_colors[BAR_SHADOW], " ", "used%");
}
addattrstr(CRT_colors[BAR_BORDER], "]");

Expand Down
12 changes: 0 additions & 12 deletions CPUMeter.c
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,6 @@ static const int CPUMeter_attributes[] = {
CPU_IOWAIT
};

static const int CPUMeter_attributes_summary[] = {
CPU_NICE,
CPU_NORMAL,
CPU_SYSTEM,
CPU_GUEST
};

typedef struct CPUMeterData_ {
unsigned int cpus;
Meter** meters;
Expand Down Expand Up @@ -89,11 +82,6 @@ static void CPUMeter_updateValues(Meter* this) {

const Machine* host = this->host;
const Settings* settings = host->settings;
if (settings->detailedCPUTime) {
this->curAttributes = CPUMeter_attributes;
} else {
this->curAttributes = CPUMeter_attributes_summary;
}

unsigned int cpu = this->param;
if (cpu > host->existingCPUs) {
Expand Down
10 changes: 8 additions & 2 deletions linux/Platform.c
Original file line number Diff line number Diff line change
Expand Up @@ -371,8 +371,14 @@ double Platform_setCPUValues(Meter* this, unsigned int cpu) {
v[CPU_METER_IOWAIT] = cpuData->ioWaitPeriod / total * 100.0;
} else {
v[CPU_METER_KERNEL] = cpuData->systemAllPeriod / total * 100.0;
v[CPU_METER_IRQ] = (cpuData->stealPeriod + cpuData->guestPeriod) / total * 100.0;
this->curItems = 4;
this->curItems = 3;

v[CPU_METER_IRQ] = 0.0; // Accounted in 'kernel'
v[CPU_METER_SOFTIRQ] = 0.0; // Accounted in 'kernel'
v[CPU_METER_STEAL] = (cpuData->stealPeriod + cpuData->guestPeriod) / total * 100.0;
if (settings->accountGuestInCPUMeter) {
this->curItems = 6;
}
}

percent = sumPositiveValues(v, this->curItems);
Expand Down
10 changes: 2 additions & 8 deletions netbsd/Platform.c
Original file line number Diff line number Diff line change
Expand Up @@ -268,16 +268,10 @@ double Platform_setCPUValues(Meter* this, int cpu) {
if (host->settings->detailedCPUTime) {
v[CPU_METER_KERNEL] = cpuData->sysPeriod / total * 100.0;
v[CPU_METER_IRQ] = cpuData->intrPeriod / total * 100.0;
v[CPU_METER_SOFTIRQ] = 0.0;
v[CPU_METER_STEAL] = 0.0;
v[CPU_METER_GUEST] = 0.0;
v[CPU_METER_IOWAIT] = 0.0;
v[CPU_METER_FREQUENCY] = NAN;
this->curItems = 8;
this->curItems = 4;
} else {
v[CPU_METER_KERNEL] = cpuData->sysAllPeriod / total * 100.0;
v[CPU_METER_IRQ] = 0.0; // No steal nor guest on NetBSD
this->curItems = 4;
this->curItems = 3;
}
totalPercent = v[CPU_METER_NICE] + v[CPU_METER_NORMAL] + v[CPU_METER_KERNEL] + v[CPU_METER_IRQ];
totalPercent = CLAMP(totalPercent, 0.0, 100.0);
Expand Down
10 changes: 2 additions & 8 deletions openbsd/Platform.c
Original file line number Diff line number Diff line change
Expand Up @@ -219,16 +219,10 @@ double Platform_setCPUValues(Meter* this, unsigned int cpu) {
if (host->settings->detailedCPUTime) {
v[CPU_METER_KERNEL] = cpuData->sysPeriod / total * 100.0;
v[CPU_METER_IRQ] = cpuData->intrPeriod / total * 100.0;
v[CPU_METER_SOFTIRQ] = 0.0;
v[CPU_METER_STEAL] = 0.0;
v[CPU_METER_GUEST] = 0.0;
v[CPU_METER_IOWAIT] = 0.0;
v[CPU_METER_FREQUENCY] = NAN;
this->curItems = 8;
this->curItems = 4;
} else {
v[CPU_METER_KERNEL] = cpuData->sysAllPeriod / total * 100.0;
v[CPU_METER_IRQ] = 0.0; // No steal nor guest on OpenBSD
this->curItems = 4;
this->curItems = 3;
}
totalPercent = v[CPU_METER_NICE] + v[CPU_METER_NORMAL] + v[CPU_METER_KERNEL] + v[CPU_METER_IRQ];

Expand Down
10 changes: 8 additions & 2 deletions pcp/Platform.c
Original file line number Diff line number Diff line change
Expand Up @@ -560,9 +560,15 @@ static double Platform_setOneCPUValues(Meter* this, const Settings* settings, pm
v[CPU_METER_IOWAIT] = values[CPU_IOWAIT_PERIOD].ull / total * 100.0;
} else {
v[CPU_METER_KERNEL] = values[CPU_SYSTEM_ALL_PERIOD].ull / total * 100.0;
this->curItems = 3;

v[CPU_METER_IRQ] = 0.0;
v[CPU_METER_SOFTIRQ] = 0.0;
value = values[CPU_STEAL_PERIOD].ull + values[CPU_GUEST_PERIOD].ull;
v[CPU_METER_IRQ] = value / total * 100.0;
this->curItems = 4;
v[CPU_METER_STEAL] = value / total * 100.0;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"value" here is not "steal" time though - its the sum of steal+guest now. So this hasn't really solved things for your unmerged graphing PR. IOW, there's a problem switching on/off detailed CPU mode because the historical data in the steal slot transitions between sum/not-sum/sum/not-sum ... so the "steal" value is incorrect whenever detailed mode changes state.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't get it… What are you suggesting? I think the historical data having "sum/not-sum/sum/not-sum", will not present any visual display glitch.

Because "steal" and "guest" are presented as the same color in all color schemes of htop

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

| Because "steal" and "guest" are presented as the same color

Good point, it'll work by luck until someone wants to separate out those categories like in most tools.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, it'll work by luck until someone wants to separate out those categories like in most tools.

I won't call it working "by luck", because I can anticipate that the "steal" and "guest" colors be separate like this:

[CPU_STEAL] = A_BOLD | ColorPair(Cyan, Black),
[CPU_GUEST] = ColorPair(Cyan, Black),

But then, how is it wrong by merging the "steal" and "guest" into a single item in the non-detailed CPU meter mode?

Isn't that the whole point of the non-detailed mode? That the unnecessary details of CPU time items can be merged (just like "irq" and "sort-irq" merged with "kernel")?

I really don't get what you are suggesting.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

| I won't call it working "by luck",

It is just luck - we very rarely have two items sharing a color in a Meter.

| But then, how is it wrong by merging the "steal" and "guest" into a single item

Its not wrong - it just remains obfuscated - like the existing code has always been, overloading one CPU values slot with multiple values from another. The bug can be fixed by a trivial change compared to this proposal, so its a nack for me.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

| But then, how is it wrong by merging the "steal" and "guest" into a single item

Its not wrong - it just remains obfuscated - like the existing code has always been, overloading one CPU values slot with multiple values from another. The bug can be fixed by a trivial change compared to this proposal, so its a nack for me.

I would claim the use of IRQ slot for virtual CPU time is plain wrong despite your fix looks trivial. The CPU meter code has fixed ("fixing" for the "making things unmovable" meaning) that item/slot as CPU_METER_IRQ and it shouldn't be overloaded with another use such as "virtualized". It breaks forward compatibility - such as with the not-yet-merged #714.

#714 relies on an assumption that the meanings of item indices of a meter to remain stable. It's also so for the meter character allocation (|#*@$%&.) in the monochrome mode. It would be more of a UX surprise when the @ character in CPU meter suddenly means different things with the "detailed CPU time" toggle. I would rather have "virtual" CPU time permanently represented by % and & and document such in the manual.

if (settings->accountGuestInCPUMeter) {
this->curItems = 6;
}
}

percent = sumPositiveValues(v, this->curItems);
Expand Down
Loading