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

Proper implementation of memory_unit and mem_precision #52

Merged
merged 3 commits into from Dec 12, 2022

Conversation

slackingfred
Copy link

@slackingfred slackingfred commented Dec 12, 2022

Description

I can't believe that hyfetch has been showing wrong (inaccurate at best) values for used/total memory for this long. And the memory precision feature seems broken from the beginning.

The problem: if we had 15,361 to 16,383 KiB of memory, hyfetch would show 15.00 MiB instead of the correct decimal value, because it represented memory values in integral MiB internally. If we had 8,192 MiB of memory, hyfetch would show 0.819 TiB (!) instead of 0.008 TiB (if you chose that unit), because the algorithm for digits after the decimal point was incorrect.

To fix the problem, we change to KiB internal representation (basically remove one division by 1024 from raw values), and we rewrite the assignment for final mem_used and mem_total strings. Still no awk involved!

Features

Issues

  • If bar_length were very large (>4000), it might overflow on 32-bit machines.
  • If mem_precision were large (>=10 on 32-bit or >=19 on 64-bit), it would overflow.

TODO

  1. Banker's rounding instead of rounding down
  2. Better handling for insane config values/command arguments (not a problem specific for this feature, though)

@hykilpikonna hykilpikonna merged commit 39be7aa into hykilpikonna:master Dec 12, 2022
@hykilpikonna
Copy link
Owner

Merged! Thanks for the fix

@slackingfred slackingfred deleted the memory_unit branch December 13, 2022 02:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants