src/utils_latency.c: Store "bin_width" as cdtime_t.

This solves the integer overflow when passing huge latency values to
latency_counter_add(). In addition to fixing the overflow the function
will now ignore values that are larger than LLONG_MAX, i.e. the longest
possible latency is 272 years. As a nice side-effect, the precission of
latency_counter_get_percentile() is improved.

Issue: #1131

diff --git a/src/utils_latency.c b/src/utils_latency.c
index 14fdf88..62f8171 100644 (file)
--- a/src/utils_latency.c
+++ b/src/utils_latency.c
@@ -30,12 +30,16 @@
 #include "common.h"
 
 #include <math.h>
+#include <limits.h>
 
 #ifndef HISTOGRAM_NUM_BINS
 # define HISTOGRAM_NUM_BINS 1000
 #endif
 
-static const int HISTOGRAM_DEFAULT_BIN_WIDTH = 1;
+#ifndef HISTOGRAM_DEFAULT_BIN_WIDTH
+/* 1048576 = 2^20 ^= 1/1024 s */
+# define HISTOGRAM_DEFAULT_BIN_WIDTH 1048576
+#endif
 
 struct latency_counter_s
 {
@@ -47,7 +51,7 @@ struct latency_counter_s
   cdtime_t min;
   cdtime_t max;
 
-  int bin_width;
+  cdtime_t bin_width;
   int histogram[HISTOGRAM_NUM_BINS];
 };
 
@@ -68,7 +72,7 @@ struct latency_counter_s
 * So, if the required bin width is 300, then new bin width will be 512 as it is
 * the next nearest power of 2.
 */
-void change_bin_width (latency_counter_t *lc, size_t val) /* {{{ */
+void change_bin_width (latency_counter_t *lc, cdtime_t latency) /* {{{ */
 {
   /* This function is called because the new value is above histogram's range.
    * First find the required bin width:
@@ -76,17 +80,15 @@ void change_bin_width (latency_counter_t *lc, size_t val) /* {{{ */
    * then get the next nearest power of 2
    *           newBinWidth = 2^(ceil(log2(requiredBinWidth)))
    */
-  double required_bin_width = (double)(val + 1) / HISTOGRAM_NUM_BINS;
-  double required_bin_width_logbase2 = log(required_bin_width) / log(2.0);
-  int new_bin_width = (int)(pow(2.0, ceil( required_bin_width_logbase2)));
-  int old_bin_width = lc->bin_width;
+  double required_bin_width = ((double) (latency + 1)) / ((double) HISTOGRAM_NUM_BINS);
+  double required_bin_width_logbase2 = log (required_bin_width) / log (2.0);
+  cdtime_t new_bin_width = (cdtime_t) (pow (2.0, ceil (required_bin_width_logbase2)) + .5);
+  cdtime_t old_bin_width = lc->bin_width;
 
   lc->bin_width = new_bin_width;
 
-  /*
-   * bin width has been increased, now iterate through all bins and move the
-   * old bin's count to new bin.
-   */
+  /* bin_width has been increased, now iterate through all bins and move the
+   * old bin's count to new bin. */
   if (lc->num > 0) // if the histogram has data then iterate else skip
   {
       double width_change_ratio = ((double) old_bin_width) / ((double) new_bin_width);
@@ -99,19 +101,16 @@ void change_bin_width (latency_counter_t *lc, size_t val) /* {{{ */
              continue;
          assert (new_bin < i);
 
-         if (lc->histogram[i] != 0) {
-           DEBUG ("utils_latency: moving %d from %zu to %zu.", lc->histogram[i], i, new_bin);
-         }
          lc->histogram[new_bin] += lc->histogram[i];
          lc->histogram[i] = 0;
       }
   }
 
-  DEBUG("utils_latency: change_bin_width: val-[%zu], oldBinWidth-[%d], "
-          "newBinWidth-[%d], required_bin_width-[%f], "
-          "required_bin_width_logbase2-[%f]",
-          val, old_bin_width, new_bin_width, required_bin_width,
-          required_bin_width_logbase2);
+  DEBUG("utils_latency: change_bin_width: latency = %.3f; "
+      "old_bin_width = %.3f; new_bin_width = %.3f;",
+      CDTIME_T_TO_DOUBLE (latency),
+      CDTIME_T_TO_DOUBLE (old_bin_width),
+      CDTIME_T_TO_DOUBLE (new_bin_width));
 } /* }}} void change_bin_width */
 
 latency_counter_t *latency_counter_create () /* {{{ */
@@ -135,9 +134,9 @@ void latency_counter_destroy (latency_counter_t *lc) /* {{{ */
 
 void latency_counter_add (latency_counter_t *lc, cdtime_t latency) /* {{{ */
 {
-  size_t latency_ms;
+  cdtime_t bin;
 
-  if ((lc == NULL) || (latency == 0))
+  if ((lc == NULL) || (latency == 0) || (latency > ((cdtime_t) LLONG_MAX)))
     return;
 
   lc->sum += latency;
@@ -153,16 +152,14 @@ void latency_counter_add (latency_counter_t *lc, cdtime_t latency) /* {{{ */
   /* A latency of _exactly_ 1.0 ms should be stored in the buffer 0, so
    * subtract one from the cdtime_t value so that exactly 1.0 ms get sorted
    * accordingly. */
-  latency_ms = (size_t) CDTIME_T_TO_MS (latency - 1);
-
-  int bin = (int)(latency_ms / lc->bin_width);
+  bin = (latency - 1) / lc->bin_width;
   if (bin >= HISTOGRAM_NUM_BINS)
   {
-      change_bin_width(lc, latency_ms);
-      bin = (int)(latency_ms / lc->bin_width);
+      change_bin_width (lc, latency);
+      bin = (latency - 1) / lc->bin_width;
       if (bin >= HISTOGRAM_NUM_BINS)
       {
-          ERROR("utils_latency: latency_counter_add: Invalid bin %d", bin);
+          ERROR ("utils_latency: latency_counter_add: Invalid bin %lu", bin);
           return;
       }
   }
@@ -174,7 +171,7 @@ void latency_counter_reset (latency_counter_t *lc) /* {{{ */
   if (lc == NULL)
     return;
 
-  int bin_width = lc->bin_width;
+  cdtime_t bin_width = lc->bin_width;
   memset (lc, 0, sizeof (*lc));
 
   /* preserve bin width */
@@ -221,14 +218,14 @@ cdtime_t latency_counter_get_average (latency_counter_t *lc) /* {{{ */
   return (DOUBLE_TO_CDTIME_T (average));
 } /* }}} cdtime_t latency_counter_get_average */
 
-cdtime_t latency_counter_get_percentile (latency_counter_t *lc,
+cdtime_t latency_counter_get_percentile (latency_counter_t *lc, /* {{{ */
     double percent)
 {
   double percent_upper;
   double percent_lower;
-  double ms_upper;
-  double ms_lower;
-  double ms_interpolated;
+  double p;
+  cdtime_t latency_lower;
+  cdtime_t latency_interpolated;
   int sum;
   size_t i;
 
@@ -258,16 +255,18 @@ cdtime_t latency_counter_get_percentile (latency_counter_t *lc,
   assert (percent_upper >= percent);
   assert (percent_lower < percent);
 
-  ms_upper = (double) ( (i + 1) * lc->bin_width );
-  ms_lower = (double) ( i * lc->bin_width );
   if (i == 0)
-    return (MS_TO_CDTIME_T (ms_upper));
+    return (lc->bin_width);
+
+  latency_lower = ((cdtime_t) i) * lc->bin_width;
+  p = (percent - percent_lower) / (percent_upper - percent_lower);
 
-  ms_interpolated = (((percent_upper - percent) * ms_lower)
-      + ((percent - percent_lower) * ms_upper))
-    / (percent_upper - percent_lower);
+  latency_interpolated = latency_lower
+    + DOUBLE_TO_CDTIME_T (p * CDTIME_T_TO_DOUBLE (lc->bin_width));
 
-  return (MS_TO_CDTIME_T (ms_interpolated));
+  DEBUG ("latency_counter_get_percentile: latency_interpolated = %.3f",
+      CDTIME_T_TO_DOUBLE (latency_interpolated));
+  return (latency_interpolated);
 } /* }}} cdtime_t latency_counter_get_percentile */
 
 /* vim: set sw=2 sts=2 et fdm=marker : */