Move variance calculation in SampleCounter to a new extension class

Variance calculation isn't currently used but overflow checks there may
cause unnecessary crash. Instead of completely deleting useful feature
it's now easy to disable it by choosing an appropriate Counter class.

Bug: None
Change-Id: Ifa8bbf2d023553504caa768e08e59ebccfb2fbb4
Reviewed-on: https://webrtc-review.googlesource.com/99561
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Commit-Queue: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#24699}

rtc_base/numerics/sample_counter.cc

@@ -26,11 +26,6 @@ void SampleCounter::Add(int sample) {
     RTC_DCHECK_GE(sample, std::numeric_limits<int64_t>::min() - sum_);
   }
   sum_ += sample;
-  // Prevent overflow in squaring.
-  RTC_DCHECK_GT(sample, std::numeric_limits<int32_t>::min());
-  RTC_DCHECK_LE(int64_t{sample} * sample,
-                std::numeric_limits<int64_t>::max() - sum_squared_);
-  sum_squared_ += int64_t{sample} * sample;
   ++num_samples_;
   if (!max_ || sample > *max_) {
     max_ = sample;
@@ -44,9 +39,6 @@ void SampleCounter::Add(const SampleCounter& other) {
     RTC_DCHECK_GE(other.sum_, std::numeric_limits<int64_t>::min() - sum_);
   }
   sum_ += other.sum_;
-  RTC_DCHECK_LE(other.sum_squared_,
-                std::numeric_limits<int64_t>::max() - sum_squared_);
-  sum_squared_ += other.sum_squared_;
   RTC_DCHECK_LE(other.num_samples_,
                 std::numeric_limits<int64_t>::max() - num_samples_);
   num_samples_ += other.num_samples_;
@@ -61,7 +53,18 @@ absl::optional<int> SampleCounter::Avg(int64_t min_required_samples) const {
   return rtc::dchecked_cast<int>(sum_ / num_samples_);
 }
 
-absl::optional<int64_t> SampleCounter::Variance(
+absl::optional<int> SampleCounter::Max() const {
+  return max_;
+}
+
+void SampleCounter::Reset() {
+  *this = {};
+}
+
+SampleCounterWithVariance::SampleCounterWithVariance() = default;
+SampleCounterWithVariance::~SampleCounterWithVariance() = default;
+
+absl::optional<int64_t> SampleCounterWithVariance::Variance(
     int64_t min_required_samples) const {
   RTC_DCHECK_GT(min_required_samples, 0);
   if (num_samples_ < min_required_samples)
@@ -71,11 +74,23 @@ absl::optional<int64_t> SampleCounter::Variance(
   return sum_squared_ / num_samples_ - mean * mean;
 }
 
-absl::optional<int> SampleCounter::Max() const {
-  return max_;
+void SampleCounterWithVariance::Add(int sample) {
+  SampleCounter::Add(sample);
+  // Prevent overflow in squaring.
+  RTC_DCHECK_GT(sample, std::numeric_limits<int32_t>::min());
+  RTC_DCHECK_LE(int64_t{sample} * sample,
+                std::numeric_limits<int64_t>::max() - sum_squared_);
+  sum_squared_ += int64_t{sample} * sample;
 }
 
-void SampleCounter::Reset() {
+void SampleCounterWithVariance::Add(const SampleCounterWithVariance& other) {
+  SampleCounter::Add(other);
+  RTC_DCHECK_LE(other.sum_squared_,
+                std::numeric_limits<int64_t>::max() - sum_squared_);
+  sum_squared_ += other.sum_squared_;
+}
+
+void SampleCounterWithVariance::Reset() {
   *this = {};
 }
 

rtc_base/numerics/sample_counter.h

@@ -23,19 +23,32 @@ class SampleCounter {
   ~SampleCounter();
   void Add(int sample);
   absl::optional<int> Avg(int64_t min_required_samples) const;
-  absl::optional<int64_t> Variance(int64_t min_required_samples) const;
   absl::optional<int> Max() const;
   void Reset();
   // Adds all the samples from the |other| SampleCounter as if they were all
   // individually added using |Add(int)| method.
   void Add(const SampleCounter& other);
 
- private:
+ protected:
   int64_t sum_ = 0;
-  int64_t sum_squared_ = 0;
   int64_t num_samples_ = 0;
   absl::optional<int> max_;
 };
 
+class SampleCounterWithVariance : public SampleCounter {
+ public:
+  SampleCounterWithVariance();
+  ~SampleCounterWithVariance();
+  void Add(int sample);
+  absl::optional<int64_t> Variance(int64_t min_required_samples) const;
+  void Reset();
+  // Adds all the samples from the |other| SampleCounter as if they were all
+  // individually added using |Add(int)| method.
+  void Add(const SampleCounterWithVariance& other);
+
+ private:
+  int64_t sum_squared_ = 0;
+};
+
 }  // namespace rtc
 #endif  // RTC_BASE_NUMERICS_SAMPLE_COUNTER_H_

rtc_base/numerics/sample_counter_unittest.cc

@@ -24,7 +24,6 @@ TEST(SampleCounterTest, ProcessesNoSamples) {
   constexpr int kMinSamples = 1;
   SampleCounter counter;
   EXPECT_THAT(counter.Avg(kMinSamples), Eq(absl::nullopt));
-  EXPECT_THAT(counter.Avg(kMinSamples), Eq(absl::nullopt));
   EXPECT_THAT(counter.Max(), Eq(absl::nullopt));
 }
 
@@ -35,7 +34,6 @@ TEST(SampleCounterTest, NotEnoughSamples) {
     counter.Add(value);
   }
   EXPECT_THAT(counter.Avg(kMinSamples), Eq(absl::nullopt));
-  EXPECT_THAT(counter.Avg(kMinSamples), Eq(absl::nullopt));
   EXPECT_THAT(counter.Max(), Eq(5));
 }
 
@@ -46,8 +44,36 @@ TEST(SampleCounterTest, EnoughSamples) {
     counter.Add(value);
   }
   EXPECT_THAT(counter.Avg(kMinSamples), Eq(3));
-  EXPECT_THAT(counter.Variance(kMinSamples), Eq(2));
   EXPECT_THAT(counter.Max(), Eq(5));
 }
 
+TEST(SampleCounterTest, ComputesVariance) {
+  constexpr int kMinSamples = 5;
+  SampleCounterWithVariance counter;
+  for (int value : {1, 2, 3, 4, 5}) {
+    counter.Add(value);
+  }
+  EXPECT_THAT(counter.Variance(kMinSamples), Eq(2));
+}
+
+TEST(SampleCounterTest, AggregatesTwoCounters) {
+  constexpr int kMinSamples = 5;
+  SampleCounterWithVariance counter1;
+  for (int value : {1, 2, 3}) {
+    counter1.Add(value);
+  }
+  SampleCounterWithVariance counter2;
+  for (int value : {4, 5}) {
+    counter2.Add(value);
+  }
+  // Before aggregation there is not enough samples.
+  EXPECT_THAT(counter1.Avg(kMinSamples), Eq(absl::nullopt));
+  EXPECT_THAT(counter1.Variance(kMinSamples), Eq(absl::nullopt));
+  // Aggregate counter2 in counter1.
+  counter1.Add(counter2);
+  EXPECT_THAT(counter1.Avg(kMinSamples), Eq(3));
+  EXPECT_THAT(counter1.Max(), Eq(5));
+  EXPECT_THAT(counter1.Variance(kMinSamples), Eq(2));
+}
+
 }  // namespace rtc