From 1a7befdb10222e71db9da527ba13cbc8e236784b Mon Sep 17 00:00:00 2001 From: Drew Hoskins Date: Thu, 11 Feb 2016 15:53:21 -0800 Subject: [PATCH] Fix MultiLevelTimeseries::getRate() 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 | 2 +- folly/test/TimeseriesTest.cpp | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/folly/stats/MultiLevelTimeSeries.h b/folly/stats/MultiLevelTimeSeries.h index 81990d0b..be75805b 100644 --- a/folly/stats/MultiLevelTimeSeries.h +++ b/folly/stats/MultiLevelTimeSeries.h @@ -159,7 +159,7 @@ class MultiLevelTimeSeries { * not been called recently. */ template - ValueType rate(int level) const { + ReturnType rate(int level) const { return getLevel(level).template rate(); } diff --git a/folly/test/TimeseriesTest.cpp b/folly/test/TimeseriesTest.cpp index e23d0b7a..8078d045 100644 --- a/folly/test/TimeseriesTest.cpp +++ b/folly/test/TimeseriesTest.cpp @@ -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(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(IntMHTS::ALLTIME), 32); for (int i = 0; i < 1800; ++i) { mhts.addValue(cur_time++, 120); -- 2.34.1