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 functions isNaN(), isNonnegative(), isPositive() and compareRealNumbers(), and replace isnan() in htop's codebase with the new functions. These functions utilize isgreater() and isgreaterequal() comparisons, which do not throw FP exceptions on "quiet" NaNs, which htop uses extensively.
htop-dev · Jul 28, 2023 · e9b064a · e9b064a
1 parent 9bd767a
commit e9b064a
Show file tree

Hide file tree

Showing 16 changed files with 137 additions and 105 deletions.
diff --git a/BatteryMeter.c b/BatteryMeter.c
@@ -12,6 +12,7 @@ This meter written by Ian P. Hands ([email protected], [email protected]).
 #include <math.h>
 
 #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
@@ -9,11 +9,11 @@ in the source distribution for its full text.
 
 #include "CPUMeter.h"
 
-#include <math.h>
 #include <stdlib.h>
 #include <string.h>
 
 #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
@@ -12,14 +12,15 @@ in the source distribution for its full text.
 #include <string.h>
 
 #include "CRT.h"
+#include "Macros.h"
 #include "Meter.h"
 #include "Object.h"
 #include "Platform.h"
 #include "RichString.h"
 #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
@@ -2,6 +2,8 @@
 #define HEADER_Macros
 
 #include <assert.h> // IWYU pragma: keep
+#include <math.h>
+#include <stdbool.h>
 
 #ifndef MINIMUM
 #define MINIMUM(a, b)                  ((a) < (b) ? (a) : (b))
@@ -98,6 +100,24 @@
 #define IGNORE_WCASTQUAL_END
 #endif
 
+/* 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 stated that isnan() throws no
+   exceptions even with a "signaling" NaN) */
+static inline bool isNaN(double x) {
+   return !isgreaterequal(x, x);
+}
+
+/* Checks if x is nonnegative. Returns false if x is NaN. */
+static inline bool isNonnegative(double x) {
+   return isgreaterequal(x, 0.0);
+}
+
+/* Checks if x is positive. Returns false if x is NaN. */
+static inline bool isPositive(double x) {
+   return isgreater(x, 0.0);
+}
+
 /* 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
@@ -11,6 +11,7 @@ in the source distribution for its full text.
 #include <stddef.h>
 
 #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
@@ -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
@@ -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
@@ -13,6 +13,7 @@ in the source distribution for its full text.
 #include <stddef.h>
 
 #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
@@ -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 <math.h>
+#include <math.h>
       ]],
       [[
-       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 >&AS_MESSAGE_LOG_FD; 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, isgreater() and isgreaterequal()])])
 
 # ----------------------------------------------------------------------
 

diff --git a/freebsd/FreeBSDMachine.c b/freebsd/FreeBSDMachine.c
@@ -281,24 +281,21 @@ static inline void FreeBSDMachine_scanCPU(Machine* super) {
    // propagate frequency to all cores if only supplied for CPU 0
    if (cpus > 1) {
       if (super->settings->showCPUTemperature) {
-         double maxTemp = NAN;
+         double maxTemp = -HUGE_VAL;
          for (unsigned int i = 1; i < maxcpu; i++) {
-            const double coreTemp = this->cpus[i].temperature;
-            if (isnan(coreTemp))
-               continue;
-
-            maxTemp = MAXIMUM(maxTemp, coreTemp);
+            if (isgreater(this->cpus[i].temperature, maxTemp)) {
+               maxTemp = this->cpus[i].temperature;
+               this->cpus[0].temperature = maxTemp;
+            }
          }
-
-         this->cpus[0].temperature = maxTemp;
       }
 
       if (super->settings->showCPUFrequency) {
          const double coreZeroFreq = this->cpus[1].frequency;
          double freqSum = coreZeroFreq;
-         if (!isnan(coreZeroFreq)) {
+         if (isNonnegative(coreZeroFreq)) {
             for (unsigned int i = 2; i < maxcpu; i++) {
-               if (isnan(this->cpus[i].frequency))
+               if (!isNonnegative(this->cpus[i].frequency))
                   this->cpus[i].frequency = coreZeroFreq;
 
                freqSum += this->cpus[i].frequency;

diff --git 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/HugePageMeter.c b/linux/HugePageMeter.c
@@ -80,7 +80,7 @@ static void HugePageMeter_display(const Object* cast, RichString* out) {
    RichString_appendAscii(out, CRT_colors[METER_VALUE], buffer);
 
    for (unsigned i = 0; i < ARRAYSIZE(HugePageMeter_active_labels); i++) {
-      if (isnan(this->values[i])) {
+      if (!HugePageMeter_active_labels[i]) {
          break;
       }
       RichString_appendAscii(out, CRT_colors[METER_TEXT], HugePageMeter_active_labels[i]);