From 98182cb9987f5ab773a89271b7720692f5cbe21d Mon Sep 17 00:00:00 2001 From: Explorer09 Date: Wed, 26 Jul 2023 19:52:35 +0800 Subject: [PATCH] Replace isnan() with better comparisons (isgreater(), etc.) (This commit is work in progress) The standard isnan() function is defined to never throw FP exceptions even when the argument is a "signaling" NaN. This makes isnan() more expensive than (x != x) expression unless the compiler flag '-fno-signaling-nans' is given. Introduce inline functions isNaN(), isNonnegative() and isPositive(), and replace isnan() in htop's codebase with the new inline functions. These functions utilize isgreater() and isgreaterequal() comparisons, which do not throw FP exceptions on "quiet" NaNs, which htop uses extensively. --- BatteryMeter.c | 3 ++- CPUMeter.c | 26 +++++++++++++------------- FileDescriptorMeter.c | 11 ++++++----- Macros.h | 20 ++++++++++++++++++++ MemoryMeter.c | 10 +++++----- Meter.c | 6 ++++-- Process.c | 2 +- SwapMeter.c | 5 +++-- configure.ac | 24 ++++++++++++++++-------- generic/fdstat_sysctl.c | 3 --- linux/LibSensors.c | 20 +++++++++----------- 11 files changed, 79 insertions(+), 51 deletions(-) diff --git a/BatteryMeter.c b/BatteryMeter.c index 33d17b734..24b2e6825 100644 --- a/BatteryMeter.c +++ b/BatteryMeter.c @@ -12,6 +12,7 @@ This meter written by Ian P. Hands (iphands@gmail.com, ihands@redhat.com). #include #include "CRT.h" +#include "Macros.h" #include "Object.h" #include "Platform.h" #include "XUtils.h" @@ -27,7 +28,7 @@ static void BatteryMeter_updateValues(Meter* this) { Platform_getBattery(&percent, &isOnAC); - if (isnan(percent)) { + if (!isNonnegative(percent)) { this->values[0] = NAN; xSnprintf(this->txtBuffer, sizeof(this->txtBuffer), "N/A"); return; diff --git a/CPUMeter.c b/CPUMeter.c index a946aa7d7..c12bb72c6 100644 --- a/CPUMeter.c +++ b/CPUMeter.c @@ -9,11 +9,11 @@ in the source distribution for its full text. #include "CPUMeter.h" -#include #include #include #include "CRT.h" +#include "Macros.h" #include "Object.h" #include "Platform.h" #include "ProcessList.h" @@ -71,7 +71,7 @@ static void CPUMeter_updateValues(Meter* this) { } double percent = Platform_setCPUValues(this, cpu); - if (isnan(percent)) { + if (!isNonnegative(percent)) { xSnprintf(this->txtBuffer, sizeof(this->txtBuffer), "offline"); return; } @@ -86,17 +86,17 @@ static void CPUMeter_updateValues(Meter* this) { if (settings->showCPUFrequency) { double cpuFrequency = this->values[CPU_METER_FREQUENCY]; - if (isnan(cpuFrequency)) { - xSnprintf(cpuFrequencyBuffer, sizeof(cpuFrequencyBuffer), "N/A"); - } else { + if (isNonnegative(cpuFrequency)) { xSnprintf(cpuFrequencyBuffer, sizeof(cpuFrequencyBuffer), "%4uMHz", (unsigned)cpuFrequency); + } else { + xSnprintf(cpuFrequencyBuffer, sizeof(cpuFrequencyBuffer), "N/A"); } } #ifdef BUILD_WITH_CPU_TEMP if (settings->showCPUTemperature) { double cpuTemperature = this->values[CPU_METER_TEMPERATURE]; - if (isnan(cpuTemperature)) + if (isNaN(cpuTemperature)) xSnprintf(cpuTemperatureBuffer, sizeof(cpuTemperatureBuffer), "N/A"); else if (settings->degreeFahrenheit) xSnprintf(cpuTemperatureBuffer, sizeof(cpuTemperatureBuffer), "%3d%sF", (int)(cpuTemperature * 9 / 5 + 32), CRT_degreeSign); @@ -146,12 +146,12 @@ static void CPUMeter_display(const Object* cast, RichString* out) { len = xSnprintf(buffer, sizeof(buffer), "%5.1f%% ", this->values[CPU_METER_SOFTIRQ]); RichString_appendAscii(out, CRT_colors[METER_TEXT], "si:"); RichString_appendnAscii(out, CRT_colors[CPU_SOFTIRQ], buffer, len); - if (!isnan(this->values[CPU_METER_STEAL])) { + if (isNonnegative(this->values[CPU_METER_STEAL])) { len = xSnprintf(buffer, sizeof(buffer), "%5.1f%% ", this->values[CPU_METER_STEAL]); RichString_appendAscii(out, CRT_colors[METER_TEXT], "st:"); RichString_appendnAscii(out, CRT_colors[CPU_STEAL], buffer, len); } - if (!isnan(this->values[CPU_METER_GUEST])) { + if (isNonnegative(this->values[CPU_METER_GUEST])) { len = xSnprintf(buffer, sizeof(buffer), "%5.1f%% ", this->values[CPU_METER_GUEST]); RichString_appendAscii(out, CRT_colors[METER_TEXT], "gu:"); RichString_appendnAscii(out, CRT_colors[CPU_GUEST], buffer, len); @@ -166,7 +166,7 @@ static void CPUMeter_display(const Object* cast, RichString* out) { len = xSnprintf(buffer, sizeof(buffer), "%5.1f%% ", this->values[CPU_METER_NICE]); RichString_appendAscii(out, CRT_colors[METER_TEXT], "low:"); RichString_appendnAscii(out, CRT_colors[CPU_NICE_TEXT], buffer, len); - if (!isnan(this->values[CPU_METER_IRQ])) { + if (isNonnegative(this->values[CPU_METER_IRQ])) { len = xSnprintf(buffer, sizeof(buffer), "%5.1f%% ", this->values[CPU_METER_IRQ]); RichString_appendAscii(out, CRT_colors[METER_TEXT], "vir:"); RichString_appendnAscii(out, CRT_colors[CPU_GUEST], buffer, len); @@ -176,10 +176,10 @@ static void CPUMeter_display(const Object* cast, RichString* out) { if (settings->showCPUFrequency) { char cpuFrequencyBuffer[10]; double cpuFrequency = this->values[CPU_METER_FREQUENCY]; - if (isnan(cpuFrequency)) { - len = xSnprintf(cpuFrequencyBuffer, sizeof(cpuFrequencyBuffer), "N/A "); - } else { + if (isNonnegative(cpuFrequency)) { len = xSnprintf(cpuFrequencyBuffer, sizeof(cpuFrequencyBuffer), "%4uMHz ", (unsigned)cpuFrequency); + } else { + len = xSnprintf(cpuFrequencyBuffer, sizeof(cpuFrequencyBuffer), "N/A "); } RichString_appendAscii(out, CRT_colors[METER_TEXT], "freq: "); RichString_appendnWide(out, CRT_colors[METER_VALUE], cpuFrequencyBuffer, len); @@ -189,7 +189,7 @@ static void CPUMeter_display(const Object* cast, RichString* out) { if (settings->showCPUTemperature) { char cpuTemperatureBuffer[10]; double cpuTemperature = this->values[CPU_METER_TEMPERATURE]; - if (isnan(cpuTemperature)) { + if (isNaN(cpuTemperature)) { len = xSnprintf(cpuTemperatureBuffer, sizeof(cpuTemperatureBuffer), "N/A"); } else if (settings->degreeFahrenheit) { len = xSnprintf(cpuTemperatureBuffer, sizeof(cpuTemperatureBuffer), "%5.1f%sF", cpuTemperature * 9 / 5 + 32, CRT_degreeSign); diff --git a/FileDescriptorMeter.c b/FileDescriptorMeter.c index 2d939d66d..92dc99ae3 100644 --- a/FileDescriptorMeter.c +++ b/FileDescriptorMeter.c @@ -12,6 +12,7 @@ in the source distribution for its full text. #include #include "CRT.h" +#include "Macros.h" #include "Meter.h" #include "Object.h" #include "Platform.h" @@ -19,7 +20,7 @@ in the source distribution for its full text. #include "XUtils.h" -#define FD_EFFECTIVE_UNLIMITED(x) ((x) > (1<<30)) +#define FD_EFFECTIVE_UNLIMITED(x) (!isgreaterequal((double)(1<<30), (x))) static const int FileDescriptorMeter_attributes[] = { FILE_DESCRIPTOR_USED, @@ -67,9 +68,9 @@ static void FileDescriptorMeter_updateValues(Meter* this) { } } - if (isnan(this->values[0])) { + if (!isNonnegative(this->values[0])) { xSnprintf(this->txtBuffer, sizeof(this->txtBuffer), "unknown/unknown"); - } else if (isnan(this->values[1]) || FD_EFFECTIVE_UNLIMITED(this->values[1])) { + } else if (FD_EFFECTIVE_UNLIMITED(this->values[1])) { xSnprintf(this->txtBuffer, sizeof(this->txtBuffer), "%.0lf/unlimited", this->values[0]); } else { xSnprintf(this->txtBuffer, sizeof(this->txtBuffer), "%.0lf/%.0lf", this->values[0], this->values[1]); @@ -81,7 +82,7 @@ static void FileDescriptorMeter_display(const Object* cast, RichString* out) { char buffer[50]; int len; - if (isnan(this->values[0])) { + if (!isNonnegative(this->values[0])) { RichString_appendAscii(out, CRT_colors[METER_TEXT], "unknown"); return; } @@ -91,7 +92,7 @@ static void FileDescriptorMeter_display(const Object* cast, RichString* out) { RichString_appendnAscii(out, CRT_colors[FILE_DESCRIPTOR_USED], buffer, len); RichString_appendAscii(out, CRT_colors[METER_TEXT], " max: "); - if (isnan(this->values[1]) || FD_EFFECTIVE_UNLIMITED(this->values[1])) { + if (FD_EFFECTIVE_UNLIMITED(this->values[1])) { RichString_appendAscii(out, CRT_colors[FILE_DESCRIPTOR_MAX], "unlimited"); } else { len = xSnprintf(buffer, sizeof(buffer), "%.0lf", this->values[1]); diff --git a/Macros.h b/Macros.h index 0f95347b7..429d5436d 100644 --- a/Macros.h +++ b/Macros.h @@ -2,6 +2,8 @@ #define HEADER_Macros #include // IWYU pragma: keep +#include +#include #ifndef MINIMUM #define MINIMUM(a, b) ((a) < (b) ? (a) : (b)) @@ -98,6 +100,24 @@ #define IGNORE_WCASTQUAL_END #endif +/* Checks if x is positive. Returns false if x is NaN. */ +static inline bool isPositive(double x) { + return isgreater(x, 0.0); +} + +/* Checks if x is nonnegative. Returns false if x is NaN. */ +static inline bool isNonnegative(double x) { + return isgreaterequal(x, 0.0); +} + +/* Cheaper function for checking NaNs. Unlike the standard isnan(), this may + throw an FP exception on a "signaling" NaN. + (ISO/IEC TS 18661-1 and the C23 standard added that isnan() throws no + exceptions even with a "signaling" NaN) */ +static inline bool isNaN(double x) { + return !isgreaterequal(x, x); +} + /* This subtraction is used by Linux / NetBSD / OpenBSD for calculation of CPU usage items. */ static inline unsigned long long saturatingSub(unsigned long long a, unsigned long long b) { return a > b ? a - b : 0; diff --git a/MemoryMeter.c b/MemoryMeter.c index 28c0be277..c7d99f885 100644 --- a/MemoryMeter.c +++ b/MemoryMeter.c @@ -11,6 +11,7 @@ in the source distribution for its full text. #include #include "CRT.h" +#include "Macros.h" #include "Object.h" #include "Platform.h" #include "RichString.h" @@ -42,9 +43,8 @@ static void MemoryMeter_updateValues(Meter* this) { /* we actually want to show "used + compressed" */ double used = this->values[MEMORY_METER_USED]; - if (!isnan(this->values[MEMORY_METER_COMPRESSED])) { + if (isPositive(this->values[MEMORY_METER_COMPRESSED])) used += this->values[MEMORY_METER_COMPRESSED]; - } written = Meter_humanUnit(buffer, used, size); METER_BUFFER_CHECK(buffer, size, written); @@ -71,14 +71,14 @@ static void MemoryMeter_display(const Object* cast, RichString* out) { RichString_appendAscii(out, CRT_colors[MEMORY_BUFFERS_TEXT], buffer); /* shared memory is not supported on all platforms */ - if (!isnan(this->values[MEMORY_METER_SHARED])) { + if (isNonnegative(this->values[MEMORY_METER_SHARED])) { Meter_humanUnit(buffer, this->values[MEMORY_METER_SHARED], sizeof(buffer)); RichString_appendAscii(out, CRT_colors[METER_TEXT], " shared:"); RichString_appendAscii(out, CRT_colors[MEMORY_SHARED], buffer); } /* compressed memory is not supported on all platforms */ - if (!isnan(this->values[MEMORY_METER_COMPRESSED])) { + if (isNonnegative(this->values[MEMORY_METER_COMPRESSED])) { Meter_humanUnit(buffer, this->values[MEMORY_METER_COMPRESSED], sizeof(buffer)); RichString_appendAscii(out, CRT_colors[METER_TEXT], " compressed:"); RichString_appendAscii(out, CRT_colors[MEMORY_COMPRESSED], buffer); @@ -89,7 +89,7 @@ static void MemoryMeter_display(const Object* cast, RichString* out) { RichString_appendAscii(out, CRT_colors[MEMORY_CACHE], buffer); /* available memory is not supported on all platforms */ - if (!isnan(this->values[MEMORY_METER_AVAILABLE])) { + if (isNonnegative(this->values[MEMORY_METER_AVAILABLE])) { Meter_humanUnit(buffer, this->values[MEMORY_METER_AVAILABLE], sizeof(buffer)); RichString_appendAscii(out, CRT_colors[METER_TEXT], " available:"); RichString_appendAscii(out, CRT_colors[METER_VALUE], buffer); diff --git a/Meter.c b/Meter.c index cf0fe36ac..6396d05a8 100644 --- a/Meter.c +++ b/Meter.c @@ -334,8 +334,10 @@ static void GraphMeterMode_draw(Meter* this, int x, int y, int w) { data->values[nValues - 1] = (this->curItems > 0) ? this->values[this->curItems - 1] : 0.0; } else { double value = 0.0; - for (uint8_t i = 0; i < this->curItems; i++) - value += !isnan(this->values[i]) ? this->values[i] : 0; + for (uint8_t i = 0; i < this->curItems; i++) { + if (isPositive(this->values[i])) + value += this->values[i]; + } data->values[nValues - 1] = value; } } diff --git a/Process.c b/Process.c index 1424d6ef1..8e2e1dd2c 100644 --- a/Process.c +++ b/Process.c @@ -754,7 +754,7 @@ void Process_printRate(RichString* str, double rate, bool coloring) { processMegabytesColor = CRT_colors[PROCESS]; } - if (isnan(rate)) { + 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); diff --git a/SwapMeter.c b/SwapMeter.c index 84e58a26e..1055a6e70 100644 --- a/SwapMeter.c +++ b/SwapMeter.c @@ -13,6 +13,7 @@ in the source distribution for its full text. #include #include "CRT.h" +#include "Macros.h" #include "Object.h" #include "Platform.h" #include "RichString.h" @@ -51,13 +52,13 @@ static void SwapMeter_display(const Object* cast, RichString* out) { RichString_appendAscii(out, CRT_colors[METER_TEXT], " used:"); RichString_appendAscii(out, CRT_colors[METER_VALUE], buffer); - if (!isnan(this->values[SWAP_METER_CACHE])) { + if (isNonnegative(this->values[SWAP_METER_CACHE])) { Meter_humanUnit(buffer, this->values[SWAP_METER_CACHE], sizeof(buffer)); RichString_appendAscii(out, CRT_colors[METER_TEXT], " cache:"); RichString_appendAscii(out, CRT_colors[SWAP_CACHE], buffer); } - if (!isnan(this->values[SWAP_METER_FRONTSWAP])) { + if (isNonnegative(this->values[SWAP_METER_FRONTSWAP])) { Meter_humanUnit(buffer, this->values[SWAP_METER_FRONTSWAP], sizeof(buffer)); RichString_appendAscii(out, CRT_colors[METER_TEXT], " frontswap:"); RichString_appendAscii(out, CRT_colors[SWAP_FRONTSWAP], buffer); diff --git a/configure.ac b/configure.ac index a3cc572f3..c44685fe4 100644 --- a/configure.ac +++ b/configure.ac @@ -203,21 +203,29 @@ AC_COMPILE_IFELSE([ CFLAGS="$old_CFLAGS" AC_MSG_CHECKING(for NaN support) -AC_RUN_IFELSE([ +dnl Note: AC_RUN_IFELSE does not try compiling the program at all when +dnl $cross_compiling is 'yes'. +AC_LINK_IFELSE([ AC_LANG_PROGRAM( [[ - #include +#include ]], [[ - double x = NAN; return !isnan(x); + double x = NAN; + /* Both should evaluate to false -> 0 (exit success) */ + return isgreater(x, x) || isgreaterequal(x, x); ]] )], - [AC_MSG_RESULT(yes)], - [ + [if test "$cross_compiling" = yes; then + AC_MSG_RESULT([yes (cross)]) + elif ./conftest$EXEEXT; then + AC_MSG_RESULT(yes) + else AC_MSG_RESULT(no) - AC_MSG_WARN([Compiler does not respect NaN, some functionality might break; consider using '-fno-finite-math-only']) - ], - [AC_MSG_RESULT(skipped)]) + AC_MSG_WARN([runtime behavior with NaN is not compliant - some functionality might break; consider using '-fno-finite-math-only']) + fi], + [AC_MSG_RESULT(no) + AC_MSG_ERROR([can not find required macros: NAN and isgreater()])]) # ---------------------------------------------------------------------- diff --git a/generic/fdstat_sysctl.c b/generic/fdstat_sysctl.c index 49e8e362a..432114c20 100644 --- a/generic/fdstat_sysctl.c +++ b/generic/fdstat_sysctl.c @@ -43,9 +43,6 @@ static void Generic_getFileDescriptors_sysctl_internal( len = sizeof(open_fd); if (sysctlname_numfiles && sysctlbyname(sysctlname_numfiles, &open_fd, &len, NULL, 0) == 0) { *used = open_fd; - } - - if (!isnan(*used)) { return; } diff --git a/linux/LibSensors.c b/linux/LibSensors.c index ff084b648..cbacc7ed4 100644 --- a/linux/LibSensors.c +++ b/linux/LibSensors.c @@ -200,7 +200,7 @@ void LibSensors_getCPUTemperatures(CPUData* cpus, unsigned int existingCPUs, uns continue; /* If already set, e.g. Ryzen reporting platform temperature for each die, use the bigger one */ - if (isnan(data[tempID])) { + if (isNaN(data[tempID])) { data[tempID] = temp; if (tempID > 0) coreTempCount++; @@ -220,7 +220,7 @@ void LibSensors_getCPUTemperatures(CPUData* cpus, unsigned int existingCPUs, uns } /* Only package temperature - copy to all cores */ - if (coreTempCount == 0 && !isnan(data[0])) { + if (coreTempCount == 0 && !isNaN(data[0])) { for (unsigned int i = 1; i <= existingCPUs; i++) data[i] = data[0]; @@ -229,22 +229,20 @@ void LibSensors_getCPUTemperatures(CPUData* cpus, unsigned int existingCPUs, uns } /* No package temperature - set to max core temperature */ - if (isnan(data[0]) && coreTempCount != 0) { - double maxTemp = NAN; + if (coreTempCount > 0 && isNaN(data[0])) { + double maxTemp = -HUGE_VAL; for (unsigned int i = 1; i <= existingCPUs; i++) { - if (isnan(data[i])) - continue; - - maxTemp = MAXIMUM(maxTemp, data[i]); + if (isgreater(data[i], maxTemp)) { + maxTemp = data[i]; + data[0] = data[i]; + } } - data[0] = maxTemp; - /* Check for further adjustments */ } /* Only temperature for core 0, maybe Ryzen - copy to all other cores */ - if (coreTempCount == 1 && !isnan(data[1])) { + if (coreTempCount == 1 && !isNaN(data[1])) { for (unsigned int i = 2; i <= existingCPUs; i++) data[i] = data[1];