Repository navigation
Reducing impact htop on CPU monitoring and lowering RAM consumption #2135
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
base: main
Are you sure you want to change the base?
Changes from all commits
09e7dde
5e81fd7
21934e2
27bc5ef
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1263,6 +1263,7 @@ IGNORE_WCASTQUAL_BEGIN | |
| } | ||
| define_key("\033[I", KEY_FOCUS_IN); | ||
| define_key("\033[O", KEY_FOCUS_OUT); | ||
| full_write_str(STDOUT_FILENO, "\033[?1004h"); | ||
|
Comment on lines
1264
to
+1266
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Benefit of forcing this active? |
||
| #ifdef HTOP_NETBSD | ||
| IGNORE_WCASTQUAL_END | ||
| #undef define_key | ||
|
|
@@ -1318,6 +1319,11 @@ void CRT_done(void) { | |
| curs_set(1); | ||
| endwin(); | ||
|
|
||
| const char* termType = getenv("TERM"); | ||
| if (terminalSupportsDefinedKeys(termType)) { | ||
| full_write_str(STDOUT_FILENO, "\033[?1004l"); | ||
| } | ||
|
|
||
| dumpStderr(); | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,7 +25,7 @@ in the source distribution for its full text. | |
|
|
||
| typedef struct HashtableItem_ { | ||
| ht_key_t key; | ||
| size_t probe; | ||
| unsigned int probe; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Contradicts one intended direction we want to go with the Hashtable code for quite some time. Using |
||
| void* value; | ||
| } HashtableItem; | ||
|
|
||
|
|
@@ -48,7 +48,7 @@ static void Hashtable_dump(const Hashtable* this) { | |
|
|
||
| size_t items = 0; | ||
| for (size_t i = 0; i < this->size; i++) { | ||
| fprintf(stderr, " item %5zu: key = %5u probe = %2zu value = %p\n", | ||
| fprintf(stderr, " item %5zu: key = %5u probe = %2u value = %p\n", | ||
| i, | ||
| this->buckets[i].key, | ||
| this->buckets[i].probe, | ||
|
|
@@ -151,7 +151,7 @@ void Hashtable_clear(Hashtable* this) { | |
|
|
||
| static void insert(Hashtable* this, ht_key_t key, void* value) { | ||
| size_t index = key % this->size; | ||
| size_t probe = 0; | ||
| unsigned int probe = 0; | ||
| #ifndef NDEBUG | ||
| size_t origIndex = index; | ||
| #endif | ||
|
|
@@ -299,9 +299,24 @@ void* Hashtable_remove(Hashtable* this, ht_key_t key) { | |
| return res; | ||
| } | ||
|
|
||
| void Hashtable_shrink(Hashtable* this) { | ||
| assert(Hashtable_isConsistent(this)); | ||
|
|
||
| /* shrink on load-factor < 0.4 if size can be reduced */ | ||
| if (this->size > 61 && 10 * this->items < 4 * this->size) { | ||
| size_t targetSize = this->items ? (this->items * 2) : 13; | ||
| size_t newSize = nextPrime(targetSize); | ||
| if (newSize < this->size) { | ||
| Hashtable_setSize(this, newSize); | ||
| } | ||
| } | ||
|
|
||
| assert(Hashtable_isConsistent(this)); | ||
| } | ||
|
|
||
|
Comment on lines
+302
to
+316
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's related PRs to this. Don't mix concerns between PRs. |
||
| void* Hashtable_get(Hashtable* this, ht_key_t key) { | ||
| size_t index = key % this->size; | ||
| size_t probe = 0; | ||
| unsigned int probe = 0; | ||
| void* res = NULL; | ||
| #ifndef NDEBUG | ||
| size_t origIndex = index; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,7 +19,7 @@ in the source distribution for its full text. | |
| #include "Object.h" | ||
|
|
||
|
|
||
| #define METER_TXTBUFFER_LEN 256 | ||
| #define METER_TXTBUFFER_LEN 127 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: #!/bin/bash
set -euo pipefail
# Inspect all txtBuffer references and their surrounding code.
rg -n -C 4 'txtBuffer' --glob '*.[ch]' .Repository: htop-dev/htop Length of output: 22576 🏁 Script executed: set -euo pipefail
printf '%s\n' '--- diff for Meter.h ---'
git diff 44a59cf1b407df3a1853e9f4f86c3f366808c9e0 cd27007d45adf30209f0fdcf007238ab2c638b2c -- Meter.h
printf '%s\n' '--- buffer helper definitions ---'
rg -n -C 8 -F -e 'METER_BUFFER_CHECK' -e 'METER_BUFFER_APPEND_CHR' -e 'Meter_humanUnit' --glob '*.[ch]' .
printf '%s\n' '--- network writer and input declarations ---'
sed -n '1,155p' NetworkIOMeter.c
rg -n -C 5 -F -e 'cached_rxb_diff_str' -e 'cached_txb_diff_str' --glob '*.[ch]' .
printf '%s\n' '--- pressure writer and file source ---'
sed -n '1,90p' linux/PressureStallMeter.c
rg -n -C 6 -F -e 'PressureStallMeter_updateValues' -e 'const char* file' --glob '*.[ch]' linux .
printf '%s\n' '--- PCP writer block ---'
sed -n '280,370p' pcp/PCPDynamicMeter.c
printf '%s\n' '--- other variable string writers ---'
sed -n '1,215p' linux/ZswapMeter.c
sed -n '160,205p' linux/OpenRCMeter.c
sed -n '295,322p' linux/SystemdMeter.c
sed -n '1,40p' SysArchMeter.cRepository: htop-dev/htop Length of output: 42613 🏁 Script executed: set -euo pipefail
printf '%s\n' '--- complete PCP update function ---'
sed -n '315,430p' pcp/PCPDynamicMeter.c
printf '%s\n' '--- PCP dynamic meter declarations and parser ---'
rg -n -C 8 -F -e 'totalMetrics' -e 'PCPDynamicMetric' -e 'PCPDynamicMeter_scan' -e 'pmsprintf' --glob '*.[ch]' pcp
printf '%s\n' '--- bounded helper definitions ---'
rg -n -C 10 -F -e 'xSnprintf' -e 'String_safeStrncpy' --glob '*.[ch]' XUtils.* String.* pcp | head -260Repository: htop-dev/htop Length of output: 41596 🌐 Web query:
💡 Result: 🏁 Script executed: set -euo pipefail
sed -n '470,530p' Meter.c
sed -n '285,310p' XUtils.cRepository: htop-dev/htop Length of output: 2045 🏁 Script executed: set -euo pipefail
rg -n -C 8 -F -e 'void fail' -e 'fail(void)' -e 'fail();' --glob '*.[ch]' .Repository: htop-dev/htop Length of output: 5995 Guard
Check the remaining capacity before formatting each value, then roll back or stop when the value cannot fit.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This one should be left alone in general, as these buffer sizes are used throughout the code … There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @BenBE Thanks for the guidance. I won’t press this recommendation, given the shared use of these buffer sizes. ✏️ Learnings added
You are interacting with an AI system. |
||
| #define MAX_METER_GRAPHDATA_VALUES 32768 | ||
|
|
||
| #define METER_BUFFER_CHECK(buffer, size, written) \ | ||
|
|
@@ -118,17 +118,18 @@ struct Meter_ { | |
| const Machine* host; | ||
|
|
||
| char* caption; | ||
| const int* curAttributes; | ||
| double* values; | ||
| double total; | ||
| void* meterData; | ||
| GraphData drawData; | ||
|
|
||
| MeterModeId mode; | ||
| unsigned int param; | ||
| GraphData drawData; | ||
| int h; | ||
| int columnWidthCount; /**< only used internally by the Header */ | ||
| uint8_t curItems; | ||
| const int* curAttributes; | ||
| char txtBuffer[METER_TXTBUFFER_LEN]; | ||
| double* values; | ||
| double total; | ||
| void* meterData; | ||
|
Comment on lines
+121
to
-131
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's also some PR for this kind of changes. Please don't mix concerns between PRs. |
||
| }; | ||
|
|
||
| typedef enum { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't make much sense, as the loop will be skipped regardless. So you are reading more memory, just to do something that a non-broken compiler has been doing since the 1980s … Zero-exec loops were in the standard from the beginning and for maintainability we'll not be catering to broken compilers. Not to mention, that this optimization will likely cause more trouble than it benefits performance, as the likely case with this loop in particular is at least one execution …