From bd8228d9514fb717cb495b28de9974b447f08a45 Mon Sep 17 00:00:00 2001 From: Adam Simpkins Date: Wed, 31 Aug 2016 12:21:13 -0700 Subject: [PATCH] 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 --- folly/stats/TimeseriesHistogram-defs.h | 104 ++++++++++++------------- folly/stats/TimeseriesHistogram.h | 51 ++++++++++-- folly/test/TimeseriesHistogramTest.cpp | 44 +++++++++++ 3 files changed, 137 insertions(+), 62 deletions(-) diff --git a/folly/stats/TimeseriesHistogram-defs.h b/folly/stats/TimeseriesHistogram-defs.h index f546d8bc..f41b749e 100644 --- a/folly/stats/TimeseriesHistogram-defs.h +++ b/folly/stats/TimeseriesHistogram-defs.h @@ -23,48 +23,6 @@ namespace folly { -template -template -ReturnType TimeseriesHistogram::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(total, nsamples); -} - -template -template -ReturnType TimeseriesHistogram::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(total, nsamples); -} - -template -template -ReturnType TimeseriesHistogram::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( - total, elapsed); -} - template TimeseriesHistogram::TimeseriesHistogram(ValueType bucketSize, ValueType min, @@ -164,18 +122,6 @@ int TimeseriesHistogram::getPercentileBucketIdx(double pct, CountFromInterval(start, end)); } -template -T TimeseriesHistogram::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 void TimeseriesHistogram::clear() { for (size_t i = 0; i < buckets_.getNumBuckets(); i++) { @@ -225,4 +171,54 @@ std::string TimeseriesHistogram::getString(TimeType start, return result; } +template +void TimeseriesHistogram::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 +void TimeseriesHistogram::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 +void TimeseriesHistogram::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 +void TimeseriesHistogram::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 9aead54e..d5ce089d 100644 --- 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 - ReturnType avg(int level) const; + template + ReturnType avg(int level) const { + auto total = ValueType(); + int64_t nsamples = 0; + computeAvgData(&total, &nsamples, level); + return folly::detail::avgHelper(total, nsamples); + } /* Average of values added during the given interval (all buckets). */ - template - ReturnType avg(TimeType start, TimeType end) const; + template + ReturnType avg(TimeType start, TimeType end) const { + auto total = ValueType(); + int64_t nsamples = 0; + computeAvgData(&total, &nsamples, start, end); + return folly::detail::avgHelper(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 + ReturnType rate(int level) const { + auto total = ValueType(); + TimeType elapsed(0); + computeRateData(&total, &elapsed, level); + return folly::detail::rateHelper( + total, elapsed); + } /* * Rate for the given interval (all buckets). * This is the sum of all values divided by the time interval (in seconds). */ - template - ReturnType rate(TimeType start, TimeType end) const; + template + ReturnType rate(TimeType start, TimeType end) const { + auto total = ValueType(); + TimeType elapsed(0); + computeRateData(&total, &elapsed, start, end); + return folly::detail::rateHelper( + 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 buckets_; bool haveNotSeenValue_; bool singleUniqueValue_; ValueType firstValue_; }; - } // folly diff --git a/folly/test/TimeseriesHistogramTest.cpp b/folly/test/TimeseriesHistogramTest.cpp index 489c277d..3cb5ce8b 100644 --- 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(IntMTMHTS::MINUTE)); + EXPECT_EQ(49, hist.avg(IntMTMHTS::TEN_MINUTE)); + EXPECT_EQ(49, hist.avg(IntMTMHTS::HOUR)); + EXPECT_EQ(49, hist.avg(IntMTMHTS::ALLTIME)); + EXPECT_EQ(49.5, hist.avg(IntMTMHTS::MINUTE)); + EXPECT_EQ(49.5, hist.avg(IntMTMHTS::TEN_MINUTE)); + EXPECT_EQ(49.5, hist.avg(IntMTMHTS::HOUR)); + EXPECT_EQ(49.5, hist.avg(IntMTMHTS::ALLTIME)); + + EXPECT_EQ(4950, hist.rate(IntMTMHTS::MINUTE)); + EXPECT_EQ(4950, hist.rate(IntMTMHTS::TEN_MINUTE)); + EXPECT_EQ(4950, hist.rate(IntMTMHTS::HOUR)); + EXPECT_EQ(4950, hist.rate(IntMTMHTS::ALLTIME)); + EXPECT_EQ(4950, hist.rate(IntMTMHTS::MINUTE)); + EXPECT_EQ(4950, hist.rate(IntMTMHTS::TEN_MINUTE)); + EXPECT_EQ(4950, hist.rate(IntMTMHTS::HOUR)); + EXPECT_EQ(4950, hist.rate(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(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(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(seconds(4550), seconds(4552))); } // ----------------- -- 2.34.1