Commit 1a7befdb authored by Drew Hoskins's avatar Drew Hoskins Committed by facebook-github-bot-4

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
parent 116d93db
......@@ -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>();
}
......
......@@ -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);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment