You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Add configurable SI decimal vs IEC binary unit prefixes - #1970
Why do we need this feature? Also I have concerns about how the feature is implemented:
If we want to provide a configurable unit display (even for just the toggle between binary and deciml units), it should either affect globally, or allow a per-meter and per-table-column configuration. Limiting the setting to affect only DiskIO and NetworkIO meters doesn't make sense.
For the fixed units like KiB/s, what to do if the number displayed is very large that it overflows the displayable space? No safeguards against that.
What to do if the set of units is to be expanded? Say, TiB, PiB, EiB?
Why do we need this feature? Also I have concerns about how the feature is implemented:
I humbly believe that display of a transfer rate / bandwidth usage should be in SI decimal units (power of 10) and not IEC units (power of 2). In my personal use case output of htop network speed doesn't align with network equipment bandwidth usage measurement. Or for that matter with iostat for disk. Of course this is somewhat controversial. Rater than forcing some arbitrary standard users at least deserve choice. Hence configurable parameter.
If we want to provide a configurable unit display (even for just the toggle between binary and deciml units), it should either affect globally, or allow a per-meter and per-table-column configuration. Limiting the setting to affect only DiskIO and NetworkIO meters doesn't make sense.
Absolutely agree, however I did not see any other place where transfer rate is being displayed. Saying that I found and updated per process table rate. So this should be global now?
For the fixed units like KiB/s, what to do if the number displayed is very large that it overflows the displayable space? No safeguards against that.
Are you referring to visual misalignment? I think this is user choice. If someone wants to display something in small units, large numbers and misalignment are expected. Most people sticks with auto. Forcing specific unit like MB/s is rather rare and specific.
What to do if the set of units is to be expanded? Say, TiB, PiB, EiB?
Good question! I don't think these are achievable on a single machine at this time outside of L1/L2 cache speed. I have added TB/s and TiB/s for future use.
Until there's a clarification on the requirements I don't think it worth it for you to revise the patches.
I humbly believe that display of a transfer rate / bandwidth usage should be in SI decimal units (power of 10) and not IEC units (power of 2). In my personal use case output of htop network speed doesn't align with network equipment bandwidth usage measurement. Of course this is somewhat controversial. Rater than forcing some arbitrary standard users at least deserve choice. Hence configurable parameter.
I have no personal objections on the SI vs. IEC unit switch. My concern is on the other part of the requirements that's either unclear or not worth it.
If we want to provide a configurable unit display (even for just the toggle between binary and deciml units), it should either affect globally, or allow a per-meter and per-table-column configuration. Limiting the setting to affect only DiskIO and NetworkIO meters doesn't make sense.
Absolutely agree, however I did not see any other place where transfer rate is being displayed. Saying that I found and updated per process table rate. So this should be global now?
Grep the two keywords: Row_printRate and Row_printKBytes. The calls of these two utility functions are where the base-2 and base-10 unit switch are useful.
For the fixed units like KiB/s, what to do if the number displayed is very large that it overflows the displayable space? No safeguards against that.
Are you referring to visual misalignment? I think this is user choice. If someone wants to display something in small units, large numbers and misalignment are expected. Most people sticks with auto. Forcing specific unit like MB/s is rather rare and specific.
Meter_humanUnit intentionally constrains the number of characters displayed to 5 (including the K, M, G or T prefix itself). If a decimal point is present, only a maximum of 3 digits can be printed. The reason was simply that extra digits of precision don't seem worth it.
I have added TB/s and TiB/s for future use.
My suggestion was to remove the fixed unit options altogether, and leave only the IEC and SI unit options. That would simplify the UI.
It can be expected in the future that there would be 10-PiB/s, 11-PB/s, etc. as a natural extensions of these. I would rather not have such a potential for scope creep. As you mentioned very few people would use these fixed units. Just ditch them. The original Meter_humanUnit code can work with units up to QiB (quebibytes) thanks to the unitPrefixes array and a loop that flexibly convert the numbers.
I think a IEC / SI switch could be acceptable, probably best to make an issue to discuss this first. I know this topic is a bit controversial among IT engineers.
I think the bulk of the PR does not stand a chance of getting merged.
It adds a lot of complexity for taking the auto-scaling feature (which is probably more useful the most of our users) away.
I have no personal objections on the SI vs. IEC unit switch. My concern is on the other part of the requirements that's either unclear or not worth it.
I have removed the non-auto sizing.
Grep the two keywords: Row_printRate and Row_printKBytes. The calls of these two utility functions are where the base-2 and base-10 unit switch are useful.
Okay I think this is addressed now.
My suggestion was to remove the fixed unit options altogether, and leave only the IEC and SI unit options. That would simplify the UI.
Done! I have no problem with that. My only requirement is to have SI / IEC unit choice in order to align with other tools.
I think a IEC / SI switch could be acceptable, probably best to make an issue to discuss this first.
Awesome. This is really all thats needed.
I know this topic is a bit controversial among IT engineers.
Actually I don't think it's controversial among IT engineers. Whats controversial is how this whole IEC units came to light and how it was adopted in software. Making it configurable addresses the issue.
I think the bulk of the PR does not stand a chance of getting merged. It adds a lot of complexity for taking the auto-scaling feature (which is probably more useful the most of our users) away.
Just a note on organizing the enum: To be future-proof the most elegant solution would be to encode the LSb as IEC vs. SI, and use the other bits to denote the power to use, where 0 is "auto" and everything else is one step of the unit ladder.
But that aside, I see little benefit in this overall feature except to confuse people (for which it would work great). Historically we chose to use IEC throughout due to (visual) space constraints where we could not reliably tell the unit to the user otherwise. And given htop is resource monitor for telling you about memory consumption it makes most sense to use the IEC units as a basis.. This even holds when talking about transfer rates from and to disks, as the OS operates on sectors (IEC-based) and cache pages (also IEC-based). The only place where this historically broke down somewhat is with network related metrics as these use SI units mostly.
Historically we chose to use IEC throughout due to (visual) space constraints where we could not reliably tell the unit to the user otherwise. And given htop is resource monitor for telling you about memory consumption it makes most sense to use the IEC units as a basis.. This even holds when talking about transfer rates from and to disks, as the OS operates on sectors (IEC-based) and cache pages (also IEC-based). The only place where this historically broke down somewhat is with network related metrics as these use SI units mostly.
I agree that the IEC units should remain the default as the main thing htop users would want to monitor is the memory usage.
For hard disks and SSDs, despite the fact that sector sizes are usually engineered to be a power of two, disk sizes are instead marketed in base-10 units. (A 1 TB SSD is 1000 GB of space, not 1024 GiB.) Fortunately monitoring hard disks is not the primary use case of htop, thus the unit problem affects only a niche group of people. Just for your information.
It is a niche group of people, but the problem is that having only IEC units is sometimes incompatible with other monitoring tools, for example iostat even on latest Ubuntu or FreeBSD, or network monitoring software.
I would like to have option to switch to SI units to compare or correlate results from htop with network monitoring software which as @BenBE mentioned uses SI units, or iostat and related.
Perhaps instead of menu configurable option we can add flag --units or something like that?
It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.
Adds Settings.decimalUnits, with a default value of false, configuration read/write support, and a display-options checkbox. Adds Meter_ioRateUnit(...) for SI and IEC bytes-per-second formatting. Extends Row_printRate with a decimal flag. Updates disk and network meters and Linux and PCP process displays to use the configured unit system.
Small I/O rates in decimal mode can display as zero or be rounded up. The other previously flagged concerns (config parsing, history migration, pinned title width) remain open but are minor. The change is mergeable with owner awareness.
- [ ] Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Tip
🎯 Customize your risk assessments
Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.
✅ Automatic approvals, with your rules
Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
SI or IEC, rates now align, Settings select the scale by design, Meters show each unit clear, Rows render values without fear, Read and write follow the chosen sign.
@tenox7 This PR has not been forgotten. Just had little time the past days and also there's internally still an open question. Of the two commits I'm inclined to merge at least the first one (might even happen out-of-line); the second one still is under consideration.
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Adds a new user setting to choose between IEC binary (KiB/MiB/…) and SI decimal (KB/MB/…) scaling when displaying I/O rates, and wires that preference through the I/O rate formatting paths used by meters and process/dynamic columns.
Changes:
Introduce a persisted decimalUnits setting (read/write to config + UI toggle in Display Options).
Extend Row_printRate() to accept a decimal flag and scale using base 1000 vs 1024.
Add Meter_ioRateUnit() and update Disk/Network I/O meters to render the appropriate suffix (iB/s vs B/s).
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
File
Description
Settings.h
Adds decimalUnits to the settings struct.
Settings.c
Persists decimal_units in settings read/write; initializes default.
Row.h
Updates Row_printRate signature and clarifies units/scaling behavior.
Row.c
Implements base-1000 vs base-1024 scaling for rate display.
linux/LinuxProcess.c
Passes decimalUnits into Row_printRate for per-process I/O rate fields.
pcp/PCPProcess.c
Passes decimalUnits into Row_printRate for PCP per-process I/O rate fields.
pcp/PCPDynamicColumn.c
Applies decimalUnits when rendering dynamic rate-like columns.
Meter.h
Declares new Meter_ioRateUnit() helper for I/O rate formatting.
The reason will be displayed to describe this comment to others. Learn more.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
⚠️ Outside diff range comments (2)
Minor · Publish the migrated history file only after the copy completes. · Settings.c:963-965
Settings.c:963-965 Minor · Data Integrity & Integration · Quick win
Publish the migrated history file only after the copy completes.
Settings_migrateHistory() creates htop_history with O_EXCL before it copies anything. If the process is killed mid-copy, the cleanup at line 991 never runs. Later runs then fail O_EXCL and skip migration, so the intact legacy file is never loaded. Copy to a temporary file in the state directory, then publish it with link(), which fails if the target exists. Remove the temporary file afterward.
Minor · Measure pinned titles in display cells, not bytes. · Row.c:214-217
Row.c:214-217 Minor · Functional Correctness · Poor tradeoff
Measure pinned titles in display cells, not bytes.
RowField_pinnedWidth sums strlen of each pinned title. For a pinned dynamic column with a non-ASCII heading, that sum exceeds the header's rendered cell count. Panel.c uses the value as a screen column, so the header's scrolling part shifts relative to the rows. Compute the width in cells. Also pad dynamic headings by cells in alignedTitleDynamicColumn (Row.c:136).
Note
Follow-up findings are listed below so you can review them separately from inline comments.
Follow-up (1)
Minor (1)
Minor · Also migrate ~/.htop_history when the config has moved to XDG. · Settings.c:1036-1038
Settings.c:1036-1038 Minor · Functional Correctness · Poor tradeoff
Also migrate ~/.htop_history when the config has moved to XDG.
Settings_getHistoryFile derives the legacy history path from the active config path. After a ~/.htoprc migration, that path is the XDG config path, so the lookup checks ~/.config/htop/htop_history. A ~/.htop_history left by an older htop is never migrated.
Additional comments: 1
darwin/DarwinProcess.c-509-509: 1
509-509: Minor · Potential issue · Heavy lift
Unverified finding
We couldn't verify this from the code. It is shown for review, not as a verified issue.
The new 64-bit thread_id is passed to a process-table lookup after conversion to pid_t. IDs outside the pid_t range may truncate or become negative, causing lookup collisions or the subsequent assertion to fail.
A thread can be attributed to the wrong row, or a debug build can abort.
This can occur when: macOS supplies a thread identifier greater than PID_MAX or INT_MAX while userland threads are displayed.
Review info
Run configuration
Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Repository UI
Review profile: ASSERTIVE
Plan: Advanced
Run ID: d8e82e5d-a3a8-4704-b9ff-ad7096307ef2
Commits
Reviewing files that changed from the base of the PR and between 86a8472 and 74c63d4.
Files selected for processing: 9
DisplayOptionsPanel.c
Meter.h
Row.c
Row.h
Settings.c
Settings.h
darwin/DarwinProcess.c
linux/LinuxProcess.c
pcp/PCPProcess.c
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.
Adds a Display Options toggle (and decimal_units config key) to show I/O rates in SI decimal units (KB/s, MB/s) instead of IEC binary (KiB/s, MiB/s). Affects the disk/network I/O meters and the per-process IO rate columns; memory stays IEC.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Antoni Sawicki <as@tenoware.com>
Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Review info
Run configuration
Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Repository UI
Review profile: ASSERTIVE
Plan: Advanced
Run ID: e9a15aaa-dd4e-4285-b09c-dbc5c763ca33
Commits
Reviewing files that changed from the base of the PR and between 74c63d4 and df0dbd9.
Files selected for processing: 1
Meter.c
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
when displaying network or disk io meters allow to choose whether to use IEC or SI unit prefixes