Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhance Readability of DISK READ/DISK WRITE Columns by Shortening Labels and Values #1557

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
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
57 changes: 38 additions & 19 deletions Row.c
Original file line number Diff line number Diff line change
Expand Up @@ -468,28 +468,47 @@ void Row_printRate(RichString* str, double rate, bool coloring) {
}

if (!isNonnegative(rate)) {
RichString_appendAscii(str, shadowColor, " N/A ");
} else if (rate < 0.005) {
int len = snprintf(buffer, sizeof(buffer), "%7.2f B/s ", rate);
RichString_appendnAscii(str, shadowColor, buffer, len);
RichString_appendAscii(str, shadowColor, " N/A ");
} else if (rate < ONE_K) {
int len = snprintf(buffer, sizeof(buffer), "%7.2f B/s ", rate);
RichString_appendnAscii(str, baseColor, buffer, len);
} else if (rate < ONE_M) {
int len = snprintf(buffer, sizeof(buffer), "%7.2f K/s ", rate / ONE_K);
int len = snprintf(buffer, sizeof(buffer), "%4.0f", rate);
RichString_appendnAscii(str, baseColor, buffer, len);
} else if (rate < ONE_G) {
int len = snprintf(buffer, sizeof(buffer), "%7.2f M/s ", rate / ONE_M);
RichString_appendnAscii(str, megabytesColor, buffer, len);
} else if (rate < ONE_T) {
int len = snprintf(buffer, sizeof(buffer), "%7.2f G/s ", rate / ONE_G);
RichString_appendnAscii(str, largeNumberColor, buffer, len);
} else if (rate < ONE_P) {
int len = snprintf(buffer, sizeof(buffer), "%7.2f T/s ", rate / ONE_T);
RichString_appendnAscii(str, largeNumberColor, buffer, len);
len = snprintf(buffer, sizeof(buffer), " B/s ");
RichString_appendnAscii(str, shadowColor, buffer, len);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why using shadowColor? Ideally nonzero values should not use that text color -- the visual of that would let users skip that entry.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used it because it was mentioned in the Issue.
#991 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

Well. I would personally disagree with @BenBE unless these small values would practically make a lot of visual noise. But I am not the one to decide on this (I just put my suggestion).

Copy link
Member

Choose a reason for hiding this comment

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

IIRC, the current code shadows values below 1KiB/s. Fine either way IMO, but the note from @Explorer09 regarding accidentally visually skipping small values is valid.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As of now, according to the code in the main branch, only values below 0.005 B/s are in shadowColor.

However, in this pull request, I have made shadowColor include all values below 1 KiB/s. This change may cause small values to be accidentally skipped visually, especially for existing users. Therefore, I will change it back to the original settings.

} else {
int len = snprintf(buffer, sizeof(buffer), "%7.2f P/s ", rate / ONE_P);
RichString_appendnAscii(str, largeNumberColor, buffer, len);
size_t unitPrefixIndex = 0;
do {
if (unitPrefixIndex > ARRAYSIZE(unitPrefixes)-1) {
int len = snprintf(buffer, sizeof(buffer), "infinity ");
RichString_appendnAscii(str, largeNumberColor, buffer, len);
return;
}
unitPrefixIndex++;
rate /= ONE_K;
} while (rate >= ONE_K);

unitPrefixIndex--; // unitPrefixes starts from K

int precision = 0;
if (rate <= 99.9) {
if (rate <= 9.99)
precision = 2;
else
precision = 1;
}

if (precision < 2) {
double upper_limit = (precision == 1) ? 10 : 100;
if (rate < upper_limit) {
rate = upper_limit;
}
}

int len = snprintf(buffer, sizeof(buffer), "%4.*f", precision, rate);
int rateDisplayColor = (unitPrefixIndex < 2) ? ((!unitPrefixIndex) ? baseColor : megabytesColor) : largeNumberColor;
RichString_appendnAscii(str, rateDisplayColor, buffer, len);
len = snprintf(buffer, sizeof(buffer), " %c/s ", unitPrefixes[unitPrefixIndex]);
RichString_appendnAscii(str, shadowColor, buffer, len);

}
}

Expand Down
6 changes: 3 additions & 3 deletions htop.1.in
Original file line number Diff line number Diff line change
Expand Up @@ -500,13 +500,13 @@ Bytes of write(2) I/O for the process.
.B CNCLWB (IO_CANCEL)
Bytes of cancelled write(2) I/O.
.TP
.B IO_READ_RATE (DISK READ)
.B IO_READ_RATE (DIOR)
The I/O rate of read(2) in bytes per second, for the process.
.TP
.B IO_WRITE_RATE (DISK WRITE)
.B IO_WRITE_RATE (DIOW)
The I/O rate of write(2) in bytes per second, for the process.
.TP
.B IO_RATE (DISK R/W)
.B IO_RATE (DIORW)
The I/O rate, IO_READ_RATE + IO_WRITE_RATE (see above).
.TP
.B CGROUP
Expand Down
6 changes: 3 additions & 3 deletions linux/LinuxProcess.c
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,9 @@ const ProcessFieldData Process_fields[LAST_PROCESSFIELD] = {
[RBYTES] = { .name = "RBYTES", .title = " IO_R ", .description = "Bytes of read(2) I/O for the process", .flags = PROCESS_FLAG_IO, .defaultSortDesc = true, },
[WBYTES] = { .name = "WBYTES", .title = " IO_W ", .description = "Bytes of write(2) I/O for the process", .flags = PROCESS_FLAG_IO, .defaultSortDesc = true, },
[CNCLWB] = { .name = "CNCLWB", .title = " IO_C ", .description = "Bytes of cancelled write(2) I/O", .flags = PROCESS_FLAG_IO, .defaultSortDesc = true, },
[IO_READ_RATE] = { .name = "IO_READ_RATE", .title = " DISK READ ", .description = "The I/O rate of read(2) in bytes per second for the process", .flags = PROCESS_FLAG_IO, .defaultSortDesc = true, },
[IO_WRITE_RATE] = { .name = "IO_WRITE_RATE", .title = " DISK WRITE ", .description = "The I/O rate of write(2) in bytes per second for the process", .flags = PROCESS_FLAG_IO, .defaultSortDesc = true, },
[IO_RATE] = { .name = "IO_RATE", .title = " DISK R/W ", .description = "Total I/O rate in bytes per second", .flags = PROCESS_FLAG_IO, .defaultSortDesc = true, },
[IO_READ_RATE] = { .name = "IO_READ_RATE", .title = " DIOR ", .description = "The I/O rate of read(2) in bytes per second for the process", .flags = PROCESS_FLAG_IO, .defaultSortDesc = true, },
[IO_WRITE_RATE] = { .name = "IO_WRITE_RATE", .title = " DIOW ", .description = "The I/O rate of write(2) in bytes per second for the process", .flags = PROCESS_FLAG_IO, .defaultSortDesc = true, },
[IO_RATE] = { .name = "IO_RATE", .title = " DIORW ", .description = "Total I/O rate in bytes per second", .flags = PROCESS_FLAG_IO, .defaultSortDesc = true, },
[CGROUP] = { .name = "CGROUP", .title = "CGROUP (raw)", .description = "Which cgroup the process is in", .flags = PROCESS_FLAG_LINUX_CGROUP, .autoWidth = true, },
[CCGROUP] = { .name = "CCGROUP", .title = "CGROUP (compressed)", .description = "Which cgroup the process is in (condensed to essentials)", .flags = PROCESS_FLAG_LINUX_CGROUP, .autoWidth = true, },
[CONTAINER] = { .name = "CONTAINER", .title = "CONTAINER", .description = "Name of the container the process is in (guessed by heuristics)", .flags = PROCESS_FLAG_LINUX_CGROUP, .autoWidth = true, },
Expand Down
2 changes: 1 addition & 1 deletion pcp/PCPDynamicColumn.c
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ static void PCPDynamicColumn_setupWidth(ATTR_UNUSED ht_key_t key, void* value, A

pmUnits units = desc->units;
if (units.dimSpace && units.dimTime)
column->super.width = 11; // Row_printRate
column->super.width = 8; // Row_printRate
else if (units.dimSpace)
column->super.width = 5; // Row_printBytes
else if (units.dimCount && units.dimTime)
Expand Down
6 changes: 3 additions & 3 deletions pcp/PCPProcess.c
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,9 @@ const ProcessFieldData Process_fields[] = {
[RBYTES] = { .name = "RBYTES", .title = " IO_R ", .description = "Bytes of read(2) I/O for the process", .flags = PROCESS_FLAG_IO, .defaultSortDesc = true, },
[WBYTES] = { .name = "WBYTES", .title = " IO_W ", .description = "Bytes of write(2) I/O for the process", .flags = PROCESS_FLAG_IO, .defaultSortDesc = true, },
[CNCLWB] = { .name = "CNCLWB", .title = " IO_C ", .description = "Bytes of cancelled write(2) I/O", .flags = PROCESS_FLAG_IO, .defaultSortDesc = true, },
[IO_READ_RATE] = { .name = "IO_READ_RATE", .title = " DISK READ ", .description = "The I/O rate of read(2) in bytes per second for the process", .flags = PROCESS_FLAG_IO, .defaultSortDesc = true, },
[IO_WRITE_RATE] = { .name = "IO_WRITE_RATE", .title = " DISK WRITE ", .description = "The I/O rate of write(2) in bytes per second for the process", .flags = PROCESS_FLAG_IO, .defaultSortDesc = true, },
[IO_RATE] = { .name = "IO_RATE", .title = " DISK R/W ", .description = "Total I/O rate in bytes per second", .flags = PROCESS_FLAG_IO, .defaultSortDesc = true, },
[IO_READ_RATE] = { .name = "IO_READ_RATE", .title = " DIOR ", .description = "The I/O rate of read(2) in bytes per second for the process", .flags = PROCESS_FLAG_IO, .defaultSortDesc = true, },
[IO_WRITE_RATE] = { .name = "IO_WRITE_RATE", .title = " DIOW ", .description = "The I/O rate of write(2) in bytes per second for the process", .flags = PROCESS_FLAG_IO, .defaultSortDesc = true, },
[IO_RATE] = { .name = "IO_RATE", .title = " DIORW ", .description = "Total I/O rate in bytes per second", .flags = PROCESS_FLAG_IO, .defaultSortDesc = true, },
[CGROUP] = { .name = "CGROUP", .title = "CGROUP (raw) ", .description = "Which cgroup the process is in", .flags = PROCESS_FLAG_LINUX_CGROUP, },
[CCGROUP] = { .name = "CCGROUP", .title = "CGROUP (compressed) ", .description = "Which cgroup the process is in (condensed to essentials)", .flags = PROCESS_FLAG_LINUX_CGROUP, },
[CONTAINER] = { .name = "CONTAINER", .title = "CONTAINER ", .description = "Name of the container the process is in (guessed by heuristics)", .flags = PROCESS_FLAG_LINUX_CGROUP, },
Expand Down
Loading