Use lock-free lazy initialization for static histogram references

Make all histogram macros thread safe, and fast by again
using statics to achieve performance.

...at the cost of:
Leak all histograms to avoid races at shutdown.

Also included leak suppression for valgrind.

r=rtenneti
BUG=78207
Review URL: http://codereview.chromium.org/6780035

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@80412 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 22 insertions, 24 deletions

diff --git a/net/disk_cache/histogram_macros.h b/net/disk_cache/histogram_macros.h
index e238599..0dc12c5 100644
--- a/net/disk_cache/histogram_macros.h
+++ b/net/disk_cache/histogram_macros.h
@@ -17,10 +17,12 @@
 // These histograms follow the definition of UMA_HISTOGRAMN_XXX except that
 // whenever the name changes (the experiment group changes), the histrogram
 // object is re-created.
+// Note: These macros are only run on one thread, so the declarations of
+// |counter| was made static (i.e., there will be no race for reinitialization).
 
 #define CACHE_HISTOGRAM_CUSTOM_COUNTS(name, sample, min, max, bucket_count) \
     do { \
-      scoped_refptr<base::Histogram> counter; \
+      static base::Histogram* counter(NULL); \
       if (!counter || name != counter->histogram_name()) \
         counter = base::Histogram::FactoryGet( \
             name, min, max, bucket_count, \
@@ -39,7 +41,7 @@
 
 #define CACHE_HISTOGRAM_CUSTOM_TIMES(name, sample, min, max, bucket_count) \
     do { \
-      scoped_refptr<base::Histogram> counter; \
+      static base::Histogram* counter(NULL); \
       if (!counter || name != counter->histogram_name()) \
         counter = base::Histogram::FactoryTimeGet( \
             name, min, max, bucket_count, \
@@ -52,7 +54,7 @@
     base::TimeDelta::FromSeconds(10), 50)
 
 #define CACHE_HISTOGRAM_ENUMERATION(name, sample, boundary_value) do { \
-    scoped_refptr<base::Histogram> counter; \
+    static base::Histogram* counter(NULL); \
     if (!counter || name != counter->histogram_name()) \
       counter = base::LinearHistogram::FactoryGet( \
                     name, 1, boundary_value, boundary_value + 1, \
diff --git a/net/disk_cache/stats.cc b/net/disk_cache/stats.cc
index d9a9d12..23991c0 100644
--- a/net/disk_cache/stats.cc
+++ b/net/disk_cache/stats.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -116,7 +116,7 @@ bool CreateStats(BackendImpl* backend, Addr* address, OnDiskStats* stats) {
   return StoreStats(backend, *address, stats);
 }
 
-Stats::Stats() : backend_(NULL) {
+Stats::Stats() : backend_(NULL), size_histogram_(NULL) {
 }
 
 Stats::~Stats() {
@@ -146,7 +146,7 @@ bool Stats::Init(BackendImpl* backend, uint32* storage_addr) {
   if (first_time) {
     first_time = false;
     // ShouldReportAgain() will re-enter this object.
-    if (!size_histogram_.get() && backend->cache_type() == net::DISK_CACHE &&
+    if (!size_histogram_ && backend->cache_type() == net::DISK_CACHE &&
         backend->ShouldReportAgain()) {
       // Stats may be reused when the cache is re-created, but we want only one
       // histogram at any given time.
diff --git a/net/disk_cache/stats.h b/net/disk_cache/stats.h
index ba0d7dd..1f5aa59 100644
--- a/net/disk_cache/stats.h
+++ b/net/disk_cache/stats.h
@@ -86,7 +86,7 @@ class Stats {
   uint32 storage_addr_;
   int data_sizes_[kDataSizesLength];
   int64 counters_[MAX_COUNTER];
-  scoped_refptr<StatsHistogram> size_histogram_;
+  StatsHistogram* size_histogram_;
 
   DISALLOW_COPY_AND_ASSIGN(Stats);
 };
diff --git a/net/disk_cache/stats_histogram.cc b/net/disk_cache/stats_histogram.cc
index e9d7696..6d3097a 100644
--- a/net/disk_cache/stats_histogram.cc
+++ b/net/disk_cache/stats_histogram.cc
@@ -21,23 +21,23 @@ StatsHistogram::~StatsHistogram() {
     stats_ = NULL;
 }
 
-scoped_refptr<StatsHistogram> StatsHistogram::StatsHistogramFactoryGet(
+StatsHistogram* StatsHistogram::StatsHistogramFactoryGet(
     const std::string& name) {
-  scoped_refptr<Histogram> histogram(NULL);
+  Histogram* histogram(NULL);
 
   Sample minimum = 1;
   Sample maximum = disk_cache::Stats::kDataSizesLength - 1;
   size_t bucket_count = disk_cache::Stats::kDataSizesLength;
 
   if (StatisticsRecorder::FindHistogram(name, &histogram)) {
-    DCHECK(histogram.get() != NULL);
+    DCHECK(histogram != NULL);
   } else {
-    StatsHistogram* stats_histogram = new StatsHistogram(name, minimum, maximum,
-                                                         bucket_count);
+    // To avoid racy destruction at shutdown, the following will be leaked.
+    StatsHistogram* stats_histogram =
+        new StatsHistogram(name, minimum, maximum, bucket_count);
     stats_histogram->InitializeBucketRange();
-    histogram = stats_histogram;
-    histogram->SetFlags(kUmaTargetedHistogramFlag);
-    StatisticsRecorder::RegisterOrDiscardDuplicate(&histogram);
+    stats_histogram->SetFlags(kUmaTargetedHistogramFlag);
+    histogram = StatisticsRecorder::RegisterOrDeleteDuplicate(stats_histogram);
   }
 
   DCHECK(HISTOGRAM == histogram->histogram_type());
@@ -45,13 +45,10 @@ scoped_refptr<StatsHistogram> StatsHistogram::StatsHistogramFactoryGet(
 
   // We're preparing for an otherwise unsafe upcast by ensuring we have the
   // proper class type.
-  Histogram* temp_histogram = histogram.get();
-  StatsHistogram* temp_stats_histogram =
-      static_cast<StatsHistogram*>(temp_histogram);
+  StatsHistogram* return_histogram = static_cast<StatsHistogram*>(histogram);
   // Validate upcast by seeing that we're probably providing the checksum.
-  CHECK_EQ(temp_stats_histogram->StatsHistogram::CalculateRangeChecksum(),
-           temp_stats_histogram->CalculateRangeChecksum());
-  scoped_refptr<StatsHistogram> return_histogram(temp_stats_histogram);
+  CHECK_EQ(return_histogram->StatsHistogram::CalculateRangeChecksum(),
+           return_histogram->CalculateRangeChecksum());
   return return_histogram;
 }
 
diff --git a/net/disk_cache/stats_histogram.h b/net/disk_cache/stats_histogram.h
index 249327c..83d359c 100644
--- a/net/disk_cache/stats_histogram.h
+++ b/net/disk_cache/stats_histogram.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -35,8 +35,7 @@ class StatsHistogram : public base::Histogram {
       : Histogram(name, minimum, maximum, bucket_count), init_(false) {}
   ~StatsHistogram();
 
-  static scoped_refptr<StatsHistogram>
-      StatsHistogramFactoryGet(const std::string& name);
+  static StatsHistogram* StatsHistogramFactoryGet(const std::string& name);
 
   // We'll be reporting data from the given set of cache stats.
   bool Init(const Stats* stats);