update TimeseriesHistogram::rate to return a template type

Summary:
Update TimeseriesHistogram::rate(int level) to have a configurable return type,
similar to the rate(TimeType start, TimeType end) function, as well as the
avg() functions.  I believe it was simply an oversight initially that this
version of rate did not have a configurable return type.

Since rate() and avg() are template methods, their full definitions should
really be available in TimeseriesHistogram.h rather than
TimeseriesHistogram-defs.h.  This also fixes that problem.  Most of the logic
in these functions isn't actually dependent on the return type, so that was
split out into separate non-template helper functions that are still in
TimeseriesHistogram-defs.h

Reviewed By: yfeldblum

Differential Revision: D3776017

fbshipit-source-id: 7deebf5b9ea1be143b5d772a15246706cb0cae80

diff --git a/folly/stats/TimeseriesHistogram-defs.h b/folly/stats/TimeseriesHistogram-defs.h
index f546d8bcf3b3632ad1ceb981e724570a0b720ec2..f41b749e6510cd27c1ef462c2f140042c3ed3c3c 100644 (file)
--- a/folly/stats/TimeseriesHistogram-defs.h
+++ b/folly/stats/TimeseriesHistogram-defs.h
@@ -23,48 +23,6 @@
 
 namespace folly {
 
-template <class T, class TT, class C>
-template <typename ReturnType>
-ReturnType TimeseriesHistogram<T, TT, C>::avg(int level) const {
-  ValueType total = ValueType();
-  int64_t nsamples = 0;
-  for (unsigned int b = 0; b < buckets_.getNumBuckets(); ++b) {
-    const auto& levelObj = buckets_.getByIndex(b).getLevel(level);
-    total += levelObj.sum();
-    nsamples += levelObj.count();
-  }
-  return folly::detail::avgHelper<ReturnType>(total, nsamples);
-}
-
-template <class T, class TT, class C>
-template <typename ReturnType>
-ReturnType TimeseriesHistogram<T, TT, C>::avg(TimeType start,
-                                              TimeType end) const {
-  ValueType total = ValueType();
-  int64_t nsamples = 0;
-  for (unsigned int b = 0; b < buckets_.getNumBuckets(); ++b) {
-    const auto& levelObj = buckets_.getByIndex(b).getLevel(start, end);
-    total += levelObj.sum(start, end);
-    nsamples += levelObj.count(start, end);
-  }
-  return folly::detail::avgHelper<ReturnType>(total, nsamples);
-}
-
-template <class T, class TT, class C>
-template <typename ReturnType>
-ReturnType TimeseriesHistogram<T, TT, C>::rate(TimeType start,
-                                               TimeType end) const {
-  ValueType total = ValueType();
-  TimeType elapsed(0);
-  for (unsigned int b = 0; b < buckets_.getNumBuckets(); ++b) {
-    const auto& level = buckets_.getByIndex(b).getLevel(start);
-    total += level.sum(start, end);
-    elapsed = std::max(elapsed, level.elapsed(start, end));
-  }
-  return folly::detail::rateHelper<ReturnType, TimeType, TimeType>(
-      total, elapsed);
-}
-
 template <typename T, typename TT, typename C>
 TimeseriesHistogram<T, TT, C>::TimeseriesHistogram(ValueType bucketSize,
                                             ValueType min,
@@ -164,18 +122,6 @@ int TimeseriesHistogram<T, TT, C>::getPercentileBucketIdx(double pct,
                                          CountFromInterval(start, end));
 }
 
-template <typename T, typename TT, typename C>
-T TimeseriesHistogram<T, TT, C>::rate(int level) const {
-  ValueType total = ValueType();
-  TimeType elapsed(0);
-  for (unsigned int b = 0; b < buckets_.getNumBuckets(); ++b) {
-    const auto& levelObj = buckets_.getByIndex(b).getLevel(level);
-    total += levelObj.sum();
-    elapsed = std::max(elapsed, levelObj.elapsed());
-  }
-  return elapsed == TimeType(0) ? 0 : (total / elapsed.count());
-}
-
 template <typename T, typename TT, typename C>
 void TimeseriesHistogram<T, TT, C>::clear() {
   for (size_t i = 0; i < buckets_.getNumBuckets(); i++) {
@@ -225,4 +171,54 @@ std::string TimeseriesHistogram<T, TT, C>::getString(TimeType start,
   return result;
 }
 
+template <class T, class TT, class C>
+void TimeseriesHistogram<T, TT, C>::computeAvgData(
+    ValueType* total,
+    int64_t* nsamples,
+    int level) const {
+  for (unsigned int b = 0; b < buckets_.getNumBuckets(); ++b) {
+    const auto& levelObj = buckets_.getByIndex(b).getLevel(level);
+    *total += levelObj.sum();
+    *nsamples += levelObj.count();
+  }
+}
+
+template <class T, class TT, class C>
+void TimeseriesHistogram<T, TT, C>::computeAvgData(
+    ValueType* total,
+    int64_t* nsamples,
+    TimeType start,
+    TimeType end) const {
+  for (unsigned int b = 0; b < buckets_.getNumBuckets(); ++b) {
+    const auto& levelObj = buckets_.getByIndex(b).getLevel(start);
+    *total += levelObj.sum(start, end);
+    *nsamples += levelObj.count(start, end);
+  }
+}
+
+template <typename T, typename TT, typename C>
+void TimeseriesHistogram<T, TT, C>::computeRateData(
+    ValueType* total,
+    TimeType* elapsed,
+    int level) const {
+  for (unsigned int b = 0; b < buckets_.getNumBuckets(); ++b) {
+    const auto& levelObj = buckets_.getByIndex(b).getLevel(level);
+    *total += levelObj.sum();
+    *elapsed = std::max(*elapsed, levelObj.elapsed());
+  }
+}
+
+template <class T, class TT, class C>
+void TimeseriesHistogram<T, TT, C>::computeRateData(
+    ValueType* total,
+    TimeType* elapsed,
+    TimeType start,
+    TimeType end) const {
+  for (unsigned int b = 0; b < buckets_.getNumBuckets(); ++b) {
+    const auto& level = buckets_.getByIndex(b).getLevel(start);
+    *total += level.sum(start, end);
+    *elapsed = std::max(*elapsed, level.elapsed(start, end));
+  }
+}
+
 }  // namespace folly

diff --git a/folly/stats/TimeseriesHistogram.h b/folly/stats/TimeseriesHistogram.h
index 9aead54ea07dc55da0bffa317bd1c44e1407925f..d5ce089d75b1191e3f38b2fb339da6b55f5ef158 100644 (file)
--- a/folly/stats/TimeseriesHistogram.h
+++ b/folly/stats/TimeseriesHistogram.h
@@ -147,25 +147,48 @@ class TimeseriesHistogram {
   }
 
   /* Average of values at the given timeseries level (all buckets). */
-  template <typename ReturnType=double>
-  ReturnType avg(int level) const;
+  template <typename ReturnType = double>
+  ReturnType avg(int level) const {
+    auto total = ValueType();
+    int64_t nsamples = 0;
+    computeAvgData(&total, &nsamples, level);
+    return folly::detail::avgHelper<ReturnType>(total, nsamples);
+  }
 
   /* Average of values added during the given interval (all buckets). */
-  template <typename ReturnType=double>
-  ReturnType avg(TimeType start, TimeType end) const;
+  template <typename ReturnType = double>
+  ReturnType avg(TimeType start, TimeType end) const {
+    auto total = ValueType();
+    int64_t nsamples = 0;
+    computeAvgData(&total, &nsamples, start, end);
+    return folly::detail::avgHelper<ReturnType>(total, nsamples);
+  }
 
   /*
    * Rate at the given timeseries level (all buckets).
    * This is the sum of all values divided by the time interval (in seconds).
    */
-  ValueType rate(int level) const;
+  template <typename ReturnType = double>
+  ReturnType rate(int level) const {
+    auto total = ValueType();
+    TimeType elapsed(0);
+    computeRateData(&total, &elapsed, level);
+    return folly::detail::rateHelper<ReturnType, TimeType, TimeType>(
+        total, elapsed);
+  }
 
   /*
    * Rate for the given interval (all buckets).
    * This is the sum of all values divided by the time interval (in seconds).
    */
-  template <typename ReturnType=double>
-  ReturnType rate(TimeType start, TimeType end) const;
+  template <typename ReturnType = double>
+  ReturnType rate(TimeType start, TimeType end) const {
+    auto total = ValueType();
+    TimeType elapsed(0);
+    computeRateData(&total, &elapsed, start, end);
+    return folly::detail::rateHelper<ReturnType, TimeType, TimeType>(
+        total, elapsed);
+  }
 
   /*
    * Update every underlying timeseries object with the given timestamp. You
@@ -319,10 +342,22 @@ class TimeseriesHistogram {
    */
   void maybeHandleSingleUniqueValue(const ValueType& value);
 
+  void computeAvgData(ValueType* total, int64_t* nsamples, int level) const;
+  void computeAvgData(
+      ValueType* total,
+      int64_t* nsamples,
+      TimeType start,
+      TimeType end) const;
+  void computeRateData(ValueType* total, TimeType* elapsed, int level) const;
+  void computeRateData(
+      ValueType* total,
+      TimeType* elapsed,
+      TimeType start,
+      TimeType end) const;
+
   folly::detail::HistogramBuckets<ValueType, ContainerType> buckets_;
   bool haveNotSeenValue_;
   bool singleUniqueValue_;
   ValueType firstValue_;
 };
-
 }  // folly

diff --git a/folly/test/TimeseriesHistogramTest.cpp b/folly/test/TimeseriesHistogramTest.cpp
index 489c277d0db66a67c9c768570459b30ed383cd25..3cb5ce8bedb0c71617aaf40c7c3e0c1794d03fb7 100644 (file)
--- a/folly/test/TimeseriesHistogramTest.cpp
+++ b/folly/test/TimeseriesHistogramTest.cpp
@@ -220,6 +220,50 @@ TEST(TimeseriesHistogram, Basic) {
     EXPECT_EQ(0, hist.getBucket(0).count(IntMTMHTS::MINUTE));
     EXPECT_EQ(0, hist.getBucket(hist.getNumBuckets() - 1).count(
                 IntMTMHTS::MINUTE));
+
+    EXPECT_EQ(6000, hist.count(IntMTMHTS::MINUTE));
+    EXPECT_EQ(60000, hist.count(IntMTMHTS::TEN_MINUTE));
+    EXPECT_EQ(360000, hist.count(IntMTMHTS::HOUR));
+    EXPECT_EQ(360000, hist.count(IntMTMHTS::ALLTIME));
+
+    // Each second we added 4950 total over 100 data points
+    EXPECT_EQ(297000, hist.sum(IntMTMHTS::MINUTE));
+    EXPECT_EQ(2970000, hist.sum(IntMTMHTS::TEN_MINUTE));
+    EXPECT_EQ(17820000, hist.sum(IntMTMHTS::HOUR));
+    EXPECT_EQ(17820000, hist.sum(IntMTMHTS::ALLTIME));
+
+    EXPECT_EQ(49, hist.avg<int>(IntMTMHTS::MINUTE));
+    EXPECT_EQ(49, hist.avg<int>(IntMTMHTS::TEN_MINUTE));
+    EXPECT_EQ(49, hist.avg<int>(IntMTMHTS::HOUR));
+    EXPECT_EQ(49, hist.avg<int>(IntMTMHTS::ALLTIME));
+    EXPECT_EQ(49.5, hist.avg<double>(IntMTMHTS::MINUTE));
+    EXPECT_EQ(49.5, hist.avg<double>(IntMTMHTS::TEN_MINUTE));
+    EXPECT_EQ(49.5, hist.avg<double>(IntMTMHTS::HOUR));
+    EXPECT_EQ(49.5, hist.avg<double>(IntMTMHTS::ALLTIME));
+
+    EXPECT_EQ(4950, hist.rate<int>(IntMTMHTS::MINUTE));
+    EXPECT_EQ(4950, hist.rate<int>(IntMTMHTS::TEN_MINUTE));
+    EXPECT_EQ(4950, hist.rate<int>(IntMTMHTS::HOUR));
+    EXPECT_EQ(4950, hist.rate<int>(IntMTMHTS::ALLTIME));
+    EXPECT_EQ(4950, hist.rate<double>(IntMTMHTS::MINUTE));
+    EXPECT_EQ(4950, hist.rate<double>(IntMTMHTS::TEN_MINUTE));
+    EXPECT_EQ(4950, hist.rate<double>(IntMTMHTS::HOUR));
+    EXPECT_EQ(4950, hist.rate<double>(IntMTMHTS::ALLTIME));
+
+    EXPECT_EQ(1000, hist.count(seconds(10), seconds(20)));
+    EXPECT_EQ(49500, hist.sum(seconds(10), seconds(20)));
+    EXPECT_EQ(4950, hist.rate(seconds(10), seconds(20)));
+    EXPECT_EQ(49.5, hist.avg<double>(seconds(10), seconds(20)));
+
+    EXPECT_EQ(200, hist.count(seconds(3550), seconds(3552)));
+    EXPECT_EQ(9900, hist.sum(seconds(3550), seconds(3552)));
+    EXPECT_EQ(4950, hist.rate(seconds(3550), seconds(3552)));
+    EXPECT_EQ(49.5, hist.avg<double>(seconds(3550), seconds(3552)));
+
+    EXPECT_EQ(0, hist.count(seconds(4550), seconds(4552)));
+    EXPECT_EQ(0, hist.sum(seconds(4550), seconds(4552)));
+    EXPECT_EQ(0, hist.rate(seconds(4550), seconds(4552)));
+    EXPECT_EQ(0, hist.avg<double>(seconds(4550), seconds(4552)));
   }
 
   // -----------------