Fix MultiLevelTimeseries::getRate()
authorDrew Hoskins <dhoskins@fb.com>
Thu, 11 Feb 2016 23:53:21 +0000 (15:53 -0800)
committerfacebook-github-bot-4 <folly-bot@fb.com>
Fri, 12 Feb 2016 00:20:29 +0000 (16:20 -0800)
Summary:I hit a landmine where rate() returned 0 for my tests where fewer than 60 items were added per minute.  This was because it was truncating it.  And yet, countRate() was working.
It doesn't make sense for a rate (value accumulated per time period) to be integral.
Folly: I changed rate() to return a double by default for folly, as was done by avg() and countRate().  Looked like a typo to me.
Common wrapper: I changed getRate() to allow you to override and make it double, as was done by getAvg().  Defaulting to int (which is usually the value type) is a bad call IMO but it's a riskier change to change it to double, and I want to be consistent with getAvg().

Reviewed By: tracelog

Differential Revision: D2921061

fb-gh-sync-id: 00875f2ab7963ef3ba2db475aedaf6ebd413b38f
shipit-source-id: 00875f2ab7963ef3ba2db475aedaf6ebd413b38f

folly/stats/MultiLevelTimeSeries.h
folly/test/TimeseriesTest.cpp

index 81990d0bfd68ff5ddefd80053d45e48d85393559..be75805bab1462bbb366b1ad3eacc43e6b6b5c0b 100644 (file)
@@ -159,7 +159,7 @@ class MultiLevelTimeSeries {
    * not been called recently.
    */
   template <typename ReturnType=double, typename Interval=TimeType>
-  ValueType rate(int level) const {
+  ReturnType rate(int level) const {
     return getLevel(level).template rate<ReturnType, Interval>();
   }
 
index e23d0b7a538934158a2482e737cfc96ded864d72..8078d04520ff6b7a8d6ddeb180b4deb4b33721af 100644 (file)
@@ -859,10 +859,12 @@ TEST(MinuteHourTimeSeries, Basic) {
   EXPECT_EQ(mhts.avg(IntMHTS::MINUTE), 100);
   EXPECT_EQ(mhts.avg(IntMHTS::HOUR), 100);
   EXPECT_EQ(mhts.avg(IntMHTS::ALLTIME), 32.5);
+  EXPECT_EQ(mhts.avg<int>(IntMHTS::ALLTIME), 32);
 
   EXPECT_EQ(mhts.rate(IntMHTS::MINUTE), 100);
   EXPECT_EQ(mhts.rate(IntMHTS::HOUR), 100);
-  EXPECT_EQ(mhts.rate(IntMHTS::ALLTIME), 32);
+  EXPECT_EQ(mhts.rate(IntMHTS::ALLTIME), 32.5);
+  EXPECT_EQ(mhts.rate<int>(IntMHTS::ALLTIME), 32);
 
   for (int i = 0; i < 1800; ++i) {
     mhts.addValue(cur_time++, 120);