SampleSet -> HistogramSamples (will be reused by SparseHistogram)

net/disk_cache/stats_histogram.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 "net/disk_cache/stats_histogram.h"
 
 #include "base/debug/leak_annotations.h"
 #include "base/logging.h"
+#include "base/metrics/sample_vector.h"

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

nit: Alpha-sort

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

Done.

+#include "base/metrics/bucket_ranges.h"
+#include "base/metrics/histogram_base.h"
 #include "base/metrics/statistics_recorder.h"
 #include "net/disk_cache/stats.h"
 
 namespace disk_cache {
 
+using base::BucketRanges;
 using base::Histogram;
+using base::HistogramSamples;
+using base::SampleVector;
 using base::StatisticsRecorder;
 
-// Static.
-const Stats* StatsHistogram::stats_ = NULL;
+StatsHistogram::StatsHistogram(const std::string& name,
+                               Sample minimum,
+                               Sample maximum,
+                               size_t bucket_count,
+                               BucketRanges* ranges,
+                               const Stats* stats)
+    : Histogram(name, minimum, maximum, bucket_count, ranges),
+      stats_(stats) {}
 
-StatsHistogram::~StatsHistogram() {
-  // Only cleanup what we set.
-  if (init_)
-    stats_ = NULL;
+StatsHistogram::~StatsHistogram() {}
+
+void StatsHistogram::InitializeBucketRanges(const Stats* stats,

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

nit: Add a comment indicating that this is static (unless you tuck it into an
anonymous namespace per my suggestion in the header file).

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

Done.

+                                            BucketRanges* ranges) {
+  for (size_t i = 0; i < ranges->size(); i++) {
+    ranges->set_range(i, stats->GetBucketRange(i));
+  }
+  ranges->ResetChecksum();
 }
 
-StatsHistogram* StatsHistogram::FactoryGet(const std::string& name) {
+StatsHistogram* StatsHistogram::FactoryGet(const std::string& name,
+                                           const Stats* stats) {
   Sample minimum = 1;
   Sample maximum = disk_cache::Stats::kDataSizesLength - 1;
   size_t bucket_count = disk_cache::Stats::kDataSizesLength;
-
   Histogram* histogram = StatisticsRecorder::FindHistogram(name);
   if (!histogram) {
+    CHECK(stats);

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

nit: DCHECK

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

Done.

+
+    BucketRanges* ranges = new BucketRanges(bucket_count + 1);

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

Who owns this memory?  Please either write this into a scoped_ptr<> that you
then Pass() (preferred), or a comment describing the memory ownership.

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

Done.

+    InitializeBucketRanges(stats, ranges);
+
     // To avoid racy destruction at shutdown, the following will be leaked.
     StatsHistogram* stats_histogram =
-        new StatsHistogram(name, minimum, maximum, bucket_count);
+        new StatsHistogram(name, minimum, maximum, bucket_count, ranges, stats);
     stats_histogram->SetFlags(kUmaTargetedHistogramFlag);
     histogram = StatisticsRecorder::RegisterOrDeleteDuplicate(stats_histogram);
   }
 
   DCHECK(HISTOGRAM == histogram->histogram_type());
   DCHECK(histogram->HasConstructionArguments(minimum, maximum, bucket_count));
 
   // We're preparing for an otherwise unsafe upcast by ensuring we have the
   // proper class type.
   StatsHistogram* return_histogram = static_cast<StatsHistogram*>(histogram);
   return return_histogram;
 }
 
-bool StatsHistogram::Init(const Stats* stats) {
-  DCHECK(stats);
-  if (stats_)
-    return false;
-
-  // We support statistics report for only one cache.
-  init_ = true;
-  stats_ = stats;
-  return true;
-}
-
-Histogram::Sample StatsHistogram::ranges(size_t i) const {
-  DCHECK(stats_);
-  return stats_->GetBucketRange(i);
-}
-
-size_t StatsHistogram::bucket_count() const {
-  return disk_cache::Stats::kDataSizesLength;
-}
-
-void StatsHistogram::SnapshotSample(SampleSet* sample) const {
-  DCHECK(stats_);
-  StatsSamples my_sample;
-  stats_->Snapshot(&my_sample);
-
-  *sample = my_sample;
+scoped_ptr<SampleVector> StatsHistogram::SnapshotSamples() const {
+  scoped_ptr<SampleVector> samples(new SampleVector(bucket_ranges()));
+  stats_->Snapshot(samples.get());
 
   // Only report UMA data once.
   StatsHistogram* mutable_me = const_cast<StatsHistogram*>(this);
   mutable_me->ClearFlags(kUmaTargetedHistogramFlag);
+
+  return samples.Pass();
 }
 
 Histogram::Inconsistencies StatsHistogram::FindCorruption(
-    const SampleSet& snapshot) const {
+    const HistogramSamples& samples) const {
   return NO_INCONSISTENCIES;  // This class won't monitor inconsistencies.
 }
 
 }  // namespace disk_cache