SampleSet -> HistogramSamples (will be reused by SparseHistogram)

chrome/common/metrics/metrics_log_base.cc

 // 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 "chrome/common/metrics/metrics_log_base.h"
 
 #include "base/base64.h"
 #include "base/basictypes.h"
 #include "base/md5.h"
+#include "base/metrics/histogram_base.h"
+#include "base/metrics/histogram_samples.h"
 #include "base/perftimer.h"
 #include "base/string_number_conversions.h"
 #include "base/sys_byteorder.h"
 #include "base/sys_info.h"
 #include "base/third_party/nspr/prtime.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/common/chrome_version_info.h"
 #include "chrome/common/logging_chrome.h"
 #include "chrome/common/metrics/proto/histogram_event.pb.h"
 #include "chrome/common/metrics/proto/system_profile.pb.h"
 #include "chrome/common/metrics/proto/user_action_event.pb.h"
 #include "libxml/xmlwriter.h"
 
 #define OPEN_ELEMENT_FOR_SCOPE(name) ScopedElement scoped_element(this, name)
 
 using base::Histogram;
+using base::HistogramBase;
+using base::HistogramSamples;
+using base::SampleCountIterator;
 using base::Time;
 using base::TimeDelta;
 using metrics::HistogramEventProto;
 using metrics::SystemProfileProto;
 using metrics::UserActionEventProto;
 
 namespace {
 
 // libxml take xmlChar*, which is unsigned char*
 inline const unsigned char* UnsignedChar(const char* input) {
@@ skipping 406 matching lines @@
   int result = xmlTextWriterEndElement(xml_wrapper_->writer());
   DCHECK_GE(result, 0);
 }
 
 // TODO(JAR): A The following should really be part of the histogram class.
 // Internal state is being needlessly exposed, and it would be hard to reuse
 // this code. If we moved this into the Histogram class, then we could use
 // the same infrastructure for logging StatsCounters, RatesCounters, etc.
 void MetricsLogBase::RecordHistogramDelta(
     const Histogram& histogram,
-    const Histogram::SampleSet& snapshot) {
+    const HistogramSamples& snapshot) {
   DCHECK(!locked_);
   DCHECK_NE(0, snapshot.TotalCount());
-  DCHECK_EQ(histogram.bucket_count(), snapshot.size());

[Ilya Sherman, 2012/09/12 03:20:58]

nit: Can this DCHECK be restored?

[kaiwang, 2012/09/20 22:54:59]

now it's always true (enforced by HistogramSamples, which is created based on
bucket_count)

[Ilya Sherman, 2012/09/21 08:52:12]

Sorry, I don't follow.  These are being passed in as separate arguments --
nothing guarantees that they are for the same histogram...  What am I missing?

[kaiwang, 2012/09/21 19:51:18]

Sorry I didn't make it clear.
HistogramSamples does not have size() member function.
For this function, it actually outputs information separately from |histogram|
and |snapshot|. It's not a problem when snapshot is not from histogram. If
snapshot is from histogram, other code in Histogram enforces bucket_count()
always equals size()

 
   // We will ignore the MAX_INT/infinite value in the last element of range[].
 
   OPEN_ELEMENT_FOR_SCOPE("histogram");
 
   std::string base64_name_hash;
   uint64 numeric_name_hash;
   CreateHashes(histogram.histogram_name(),
                &base64_name_hash,
                &numeric_name_hash);
 
   // Write the XML version.
   WriteAttribute("name", base64_name_hash);
 
   WriteInt64Attribute("sum", snapshot.sum());
   // TODO(jar): Remove sumsquares when protobuffer accepts this as optional.
   WriteInt64Attribute("sumsquares", 0);
 
-  for (size_t i = 0; i < histogram.bucket_count(); i++) {
-    if (snapshot.counts(i)) {
-      OPEN_ELEMENT_FOR_SCOPE("histogrambucket");

[Ilya Sherman, 2012/09/12 03:20:58]

What happened to this line?

[kaiwang, 2012/09/20 22:54:59]

Done.

-      WriteIntAttribute("min", histogram.ranges(i));
-      WriteIntAttribute("max", histogram.ranges(i + 1));
-      WriteIntAttribute("count", snapshot.counts(i));
-    }
+  for (scoped_ptr<SampleCountIterator> it = snapshot.Iterator();
+       !it->Done();
+       it->Next()) {
+    HistogramBase::Sample min;
+    HistogramBase::Sample max;
+    HistogramBase::Count count;
+    it->Get(&min, &max, &count);
+    WriteIntAttribute("min", min);
+    WriteIntAttribute("max", max);
+    WriteIntAttribute("count", count);
   }
 
   // Write the protobuf version.
   HistogramEventProto* histogram_proto = uma_proto_.add_histogram_event();
   histogram_proto->set_name_hash(numeric_name_hash);
   histogram_proto->set_sum(snapshot.sum());
 
-  for (size_t i = 0; i < histogram.bucket_count(); ++i) {
-    if (snapshot.counts(i)) {
-      HistogramEventProto::Bucket* bucket = histogram_proto->add_bucket();
-      bucket->set_min(histogram.ranges(i));
-      bucket->set_max(histogram.ranges(i + 1));
-      bucket->set_bucket_index(i);
-      bucket->set_count(snapshot.counts(i));
-    }
+  for (scoped_ptr<SampleCountIterator> it = snapshot.Iterator();
+       !it->Done();
+       it->Next()) {
+    HistogramBase::Sample min;
+    HistogramBase::Sample max;
+    HistogramBase::Count count;
+    it->Get(&min, &max, &count);
+    HistogramEventProto::Bucket* bucket = histogram_proto->add_bucket();
+    bucket->set_min(min);
+    bucket->set_max(max);
+    bucket->set_count(count);
+
+    size_t index;
+    if (it->GetBucketIndex(&index))
+      bucket->set_bucket_index(index);
   }
 }