Add implementation in metrics.h that uses atomic pointer.

webrtc/system_wrappers/include/metrics.h

 //
 // Copyright (c) 2014 The WebRTC project authors. All Rights Reserved.
 //
 // Use of this source code is governed by a BSD-style license
 // that can be found in the LICENSE file in the root of the source
 // tree. An additional intellectual property rights grant can be found
 // in the file PATENTS.  All contributing project authors may
 // be found in the AUTHORS file in the root of the source tree.
 //
 
 #ifndef WEBRTC_SYSTEM_WRAPPERS_INCLUDE_METRICS_H_
 #define WEBRTC_SYSTEM_WRAPPERS_INCLUDE_METRICS_H_
 
 #include <string>
 
+#include "webrtc/base/atomicops.h"
 #include "webrtc/common_types.h"
 
 // Macros for allowing WebRTC clients (e.g. Chrome) to gather and aggregate
 // statistics.
 //
 // Histogram for counters.
 // RTC_HISTOGRAM_COUNTS(name, sample, min, max, bucket_count);
 //
 // Histogram for enumerators.
 // The boundary should be above the max enumerator sample.
@@ skipping 25 matching lines @@
 // enum Types {
 //   kTypeX,
 //   kTypeY,
 //   kBoundary,
 // };
 //
 // RTC_HISTOGRAM_ENUMERATION("WebRTC.Types", kTypeX, kBoundary);
 
 
 // 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)
 
 #define RTC_HISTOGRAM_COUNTS_SPARSE_200(name, sample) \
   RTC_HISTOGRAM_COUNTS_SPARSE(name, sample, 1, 200, 50)
 
 #define RTC_HISTOGRAM_COUNTS_SPARSE_1000(name, sample) \
   RTC_HISTOGRAM_COUNTS_SPARSE(name, sample, 1, 1000, 50)
 
 #define RTC_HISTOGRAM_COUNTS_SPARSE_10000(name, sample) \
   RTC_HISTOGRAM_COUNTS_SPARSE(name, sample, 1, 10000, 50)
 
 #define RTC_HISTOGRAM_COUNTS_SPARSE_100000(name, sample) \
   RTC_HISTOGRAM_COUNTS_SPARSE(name, sample, 1, 100000, 50)
 
 #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.
+// Histogram for percentage (evenly spaced buckets).
+#define RTC_HISTOGRAM_PERCENTAGE(name, sample) \
+  RTC_HISTOGRAM_ENUMERATION(name, sample, 101)
+
+// 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 { \
+    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, constant_name, sample); \
+    webrtc::metrics::HistogramAdd(histogram_pointer, name, sample); \
   } while (0)
 
 namespace webrtc {
 namespace metrics {
 
 // Time that should have elapsed for stats that are gathered once per call.
 enum { kMinRunTimeInSeconds = 10 };
 
 class Histogram;
 
@@ skipping 12 matching lines @@
 // Function for adding a |sample| to a histogram.
 // |name| can be used to verify that it matches the histogram name.
 void HistogramAdd(
     Histogram* histogram_pointer, const std::string& name, int sample);
 
 }  // namespace metrics
 }  // namespace webrtc
 
 #endif  // WEBRTC_SYSTEM_WRAPPERS_INCLUDE_METRICS_H_