Add SampleMap and use it in SparseHistogram

base/metrics/sample_map.cc

Index: base/metrics/sample_map.cc
diff --git a/base/metrics/sample_map.cc b/base/metrics/sample_map.cc
new file mode 100644
index 0000000000000000000000000000000000000000..c96b9f9049c6ff21e1895e4e5f3418a8bdef8f78
--- /dev/null
+++ b/base/metrics/sample_map.cc
@@ -0,0 +1,92 @@
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/metrics/sample_map.h"
+
+#include "base/logging.h"
+
+using std::map;
+
+namespace base {
+
+typedef HistogramBase::Count Count;
+typedef HistogramBase::Sample Sample;
+
+SampleMap::SampleMap() {}
+
+SampleMap::~SampleMap() {}
+
+void SampleMap::Accumulate(Sample value, Count count) {
+  sample_counts_[value] += count;
+  IncreaseSum(count * value);
+  IncreaseRedundantCount(count);
+}
+
+Count SampleMap::GetCount(Sample value) const {
+  map<Sample, Count>::const_iterator it = sample_counts_.find(value);
+  if (it == sample_counts_.end())
+    return 0;
+  return it->second;
+}
+
+Count SampleMap::TotalCount() const {
+  Count count = 0;
+  for (map<Sample, Count>::const_iterator it = sample_counts_.begin();
+       it != sample_counts_.end();
+       ++it) {
+    count += it->second;
+  }
+  return count;
+}
+
+scoped_ptr<SampleCountIterator> SampleMap::Iterator() const {
+  return scoped_ptr<SampleCountIterator>(new SampleMapIterator(sample_counts_));
+}
+
+void SampleMap::ResetRedundantCount(Count count) {
+  IncreaseRedundantCount(- redundant_count());

[jar (doing other things), 2012/10/05 01:27:36]

nit: remove space after unary operator |-|

[kaiwang, 2012/10/05 03:16:17]

Done.

+  IncreaseRedundantCount(count);
+}

[Ilya Sherman, 2012/10/05 03:29:24]

This method feels a little odd as part of the API -- it sort of suggests that
this class shouldn't be managing its own redundant count at all, and that
Accumulate() should only ever be called as part of construction.  Put another
way, it suggests that the inheritance relationship with HistogramSamples isn't
quite right.

It's ok to leave as is, but it feels a little heavy handed.

[kaiwang, 2012/10/05 04:02:38]

Yes, it may feel a bit odd. But it's caused by different types of usage from
Histogram and SparseHistogram.
We do need a redundant count in HistogramSamples so I think it's still better
than having another redundant_count in Histogram.

[jar (doing other things), 2012/10/05 16:55:19]

It does seem like the right answer is to have this reset performed explicitly in
the base class, by adding this method.  I am a bit surprised that we need such a
method anywhere.

On 2012/10/05 04:02:38, kaiwang wrote:

+
+bool SampleMap::AddSubtractImpl(SampleCountIterator* iter,
+                                HistogramSamples::Instruction instruction) {
+  Sample min;
+  Sample max;
+  Count count;
+  for (; !iter->Done(); iter->Next()) {
+    iter->Get(&min, &max, &count);
+    if (min + 1 != max)
+      return false;  // SparseHistogram only supports bucket with size 1.
+    sample_counts_[min] +=
+        (instruction ==  HistogramSamples::ADD) ? count : -count;
+  }
+  return true;
+}
+
+SampleMapIterator::SampleMapIterator(const map<Sample, Count>& sample_counts)
+    : iter_(sample_counts.begin()),
+      end_(sample_counts.end()) {}
+
+SampleMapIterator::~SampleMapIterator() {}
+
+bool SampleMapIterator::Done() const {
+  return iter_ == end_;
+}
+
+void SampleMapIterator::Next() {
+  DCHECK(!Done());
+  iter_++;
+}
+
+void SampleMapIterator::Get(Sample* min, Sample* max, Count* count) const {
+  DCHECK(!Done());
+  if (min != NULL)
+    *min = iter_->first;
+  if (max != NULL)
+    *max = iter_->first + 1;
+  if (count != NULL)
+    *count = iter_->second;
+}
+
+}  // namespace base