Add implementation in metrics.h that uses atomic pointer.

webrtc/system_wrappers/include/metrics.h

Index: webrtc/system_wrappers/include/metrics.h
diff --git a/webrtc/system_wrappers/include/metrics.h b/webrtc/system_wrappers/include/metrics.h
index 5e8ca11e9184ef85c4b1777b7d9650c0a342c835..6886ec1e537b00532cc585a02ff86d5e9ea40d7f 100644
--- a/webrtc/system_wrappers/include/metrics.h
+++ b/webrtc/system_wrappers/include/metrics.h
@@ -13,6 +13,7 @@
 
 #include <string>
 
+#include "webrtc/base/atomicops.h"
 #include "webrtc/common_types.h"
 
 // Macros for allowing WebRTC clients (e.g. Chrome) to gather and aggregate
@@ -58,17 +59,29 @@
 
 
 // Macros for adding samples to a named histogram.
-//
-// NOTE: this is a temporary solution.
-// The aim is to mimic the behaviour in Chromium's src/base/metrics/histograms.h
-// However as atomics are not supported in webrtc, this is for now a modified
-// and temporary solution. Note that the histogram is constructed/found for
-// each call. Therefore, for now only use this implementation for metrics
-// that do not need to be updated frequently.
-// TODO(asapersson): Change implementation when atomics are supported.
-// Also consider changing string to const char* when switching to atomics.
 
-// Histogram for counters.
+// Histogram for counters (exponentially spaced buckets).
+#define RTC_HISTOGRAM_COUNTS_100(name, sample) \
+  RTC_HISTOGRAM_COUNTS(name, sample, 1, 100, 50)
+
+#define RTC_HISTOGRAM_COUNTS_200(name, sample) \
+  RTC_HISTOGRAM_COUNTS(name, sample, 1, 200, 50)
+
+#define RTC_HISTOGRAM_COUNTS_1000(name, sample) \
+  RTC_HISTOGRAM_COUNTS(name, sample, 1, 1000, 50)
+
+#define RTC_HISTOGRAM_COUNTS_10000(name, sample) \
+  RTC_HISTOGRAM_COUNTS(name, sample, 1, 10000, 50)
+
+#define RTC_HISTOGRAM_COUNTS_100000(name, sample) \
+  RTC_HISTOGRAM_COUNTS(name, sample, 1, 100000, 50)
+
+#define RTC_HISTOGRAM_COUNTS(name, sample, min, max, bucket_count) \
+  RTC_HISTOGRAM_COMMON_BLOCK(name, sample, \
+      webrtc::metrics::HistogramFactoryGetCounts(name, min, max, bucket_count))
+
+// Deprecated.
+// TODO(asapersson): Remove.
 #define RTC_HISTOGRAM_COUNTS_SPARSE_100(name, sample) \
   RTC_HISTOGRAM_COUNTS_SPARSE(name, sample, 1, 100, 50)
 
@@ -86,26 +99,55 @@
 
 #define RTC_HISTOGRAM_COUNTS_SPARSE(name, sample, min, max, bucket_count) \
   RTC_HISTOGRAM_COMMON_BLOCK_SLOW(name, sample, \
-      webrtc::metrics::HistogramFactoryGetCounts( \
-          name, min, max, bucket_count))
+      webrtc::metrics::HistogramFactoryGetCounts(name, min, max, bucket_count))
+
+// Histogram for percentage (evenly spaced buckets).
+#define RTC_HISTOGRAM_PERCENTAGE(name, sample) \
+  RTC_HISTOGRAM_ENUMERATION(name, sample, 101)
 
-// Histogram for percentage.
+// Deprecated.
+// TODO(asapersson): Remove.
 #define RTC_HISTOGRAM_PERCENTAGE_SPARSE(name, sample) \
   RTC_HISTOGRAM_ENUMERATION_SPARSE(name, sample, 101)
 
-// Histogram for enumerators.
+// Histogram for enumerators (evenly spaced buckets).
 // |boundary| should be above the max enumerator sample.
+#define RTC_HISTOGRAM_ENUMERATION(name, sample, boundary) \
+  RTC_HISTOGRAM_COMMON_BLOCK(name, sample, \
+      webrtc::metrics::HistogramFactoryGetEnumeration(name, boundary))
+
+// Deprecated.
+// TODO(asapersson): Remove.
 #define RTC_HISTOGRAM_ENUMERATION_SPARSE(name, sample, boundary) \
   RTC_HISTOGRAM_COMMON_BLOCK_SLOW(name, sample, \
       webrtc::metrics::HistogramFactoryGetEnumeration(name, boundary))
 
-#define RTC_HISTOGRAM_COMMON_BLOCK_SLOW(constant_name, sample, \
-                                        factory_get_invocation) \
+// The name of the histogram should not vary.
+// TODO(asapersson): Consider changing string to const char*.

[pbos-webrtc, 2015/12/21 13:44:42]

FTR: I think you should def. do this, const char* makes it harder to use
wrongly.

[åsapersson, 2015/12/21 16:45:32]

Ok, will look at that in a separate CL.

+#define RTC_HISTOGRAM_COMMON_BLOCK(constant_name, sample, \
+                                   factory_get_invocation) \
   do { \
-    webrtc::metrics::Histogram* histogram_pointer = factory_get_invocation; \
+    static webrtc::metrics::Histogram* atomic_histogram_pointer = nullptr; \
+    webrtc::metrics::Histogram* histogram_pointer = \
+        rtc::AtomicOps::AcquireLoadPtr(&atomic_histogram_pointer); \
+    if (!histogram_pointer) { \
+      histogram_pointer = factory_get_invocation; \
+      rtc::AtomicOps::CompareAndSwapPtr( \
+          &atomic_histogram_pointer, \
+          static_cast<webrtc::metrics::Histogram*>(nullptr), \
+          histogram_pointer); \

[pbos-webrtc, 2015/12/21 13:44:43]

Consider DCHECKing that this previous value was either nullptr or the value
you're setting it to (histogram_pointer).

[åsapersson, 2015/12/21 16:45:32]

Done.

+    } \
     webrtc::metrics::HistogramAdd(histogram_pointer, constant_name, sample); \
   } while (0)
 
+// The histogram is constructed/found for each call.
+// May be used for histograms with infrequent updates.
+#define RTC_HISTOGRAM_COMMON_BLOCK_SLOW(name, sample, factory_get_invocation) \

[pbos-webrtc, 2015/12/21 13:44:42]

Should we consider getting rid of this implementation as well?

[åsapersson, 2015/12/21 16:45:32]

Yes when all sparse are removed. Added deprecated comment.

+  do { \
+    webrtc::metrics::Histogram* histogram_pointer = factory_get_invocation; \
+    webrtc::metrics::HistogramAdd(histogram_pointer, name, sample); \
+  } while (0)
+
 namespace webrtc {
 namespace metrics {