histogram_macros: Added some documentation explaining constant rule.

base/metrics/histogram_macros.h

Index: base/metrics/histogram_macros.h
diff --git a/base/metrics/histogram_macros.h b/base/metrics/histogram_macros.h
index fa23bea720ce3f825a32551c14f2062891cd94bf..adca4f2bdf4097a1ebde8bd4396f7ca4eeedb667 100644
--- a/base/metrics/histogram_macros.h
+++ b/base/metrics/histogram_macros.h
@@ -111,45 +111,57 @@
 
 //------------------------------------------------------------------------------
 // Provide easy general purpose histogram in a macro, just like stats counters.
-// The first four macros use 50 buckets.
+// Most of these macros use 50 buckets, but check the definition for details.
+//
+// All of these macros must be called with |constant_name| as a compile-time

[Ilya Sherman, 2016/02/24 06:47:02]

s/compile-time/runtime

[Matt Giuca, 2016/04/08 01:23:16]

Done.

+// constant. Otherwise, the above optimization will DCHECK, and failing that,
+// will result in data being written to the wrong histogram.
 
-#define LOCAL_HISTOGRAM_TIMES(name, sample) LOCAL_HISTOGRAM_CUSTOM_TIMES( \
-    name, sample, base::TimeDelta::FromMilliseconds(1), \
-    base::TimeDelta::FromSeconds(10), 50)
+#define LOCAL_HISTOGRAM_TIMES(constant_name, sample)                 \

[Ilya Sherman, 2016/02/24 06:47:02]

I'm not sure that constant_name is clearer here -- I don't think I'd really
understand what it means, without the context of knowing how these macros work. 
IMO it would be better to just annotate each of the macros with a comment that
documents that the |name| must be a runtime constant.

[Matt Giuca, 2016/02/24 07:22:14]

True. I debated whether to send this at all because I'm not sure I did it
justice. OK, I'll rename it back (which means undoing all the clang-formatting
too... not worth doing it without any other changes), and write some more docs
instead.

[Matt Giuca, 2016/04/08 01:23:16]

Done.

+  LOCAL_HISTOGRAM_CUSTOM_TIMES(constant_name, sample,                \
+                               base::TimeDelta::FromMilliseconds(1), \
+                               base::TimeDelta::FromSeconds(10), 50)
 
 // For folks that need real specific times, use this to select a precise range
 // of times you want plotted, and the number of buckets you want used.
-#define LOCAL_HISTOGRAM_CUSTOM_TIMES(name, sample, min, max, bucket_count) \
-    STATIC_HISTOGRAM_POINTER_BLOCK(name, AddTime(sample), \
-        base::Histogram::FactoryTimeGet(name, min, max, bucket_count, \
-                                        base::HistogramBase::kNoFlags))
+#define LOCAL_HISTOGRAM_CUSTOM_TIMES(constant_name, sample, min, max,        \
+                                     bucket_count)                           \
+  STATIC_HISTOGRAM_POINTER_BLOCK(                                            \
+      constant_name, AddTime(sample),                                        \
+      base::Histogram::FactoryTimeGet(constant_name, min, max, bucket_count, \
+                                      base::HistogramBase::kNoFlags))
 
-#define LOCAL_HISTOGRAM_COUNTS(name, sample) LOCAL_HISTOGRAM_CUSTOM_COUNTS( \
-    name, sample, 1, 1000000, 50)
+#define LOCAL_HISTOGRAM_COUNTS(constant_name, sample) \
+  LOCAL_HISTOGRAM_CUSTOM_COUNTS(constant_name, sample, 1, 1000000, 50)
 
-#define LOCAL_HISTOGRAM_COUNTS_100(name, sample) \
-    LOCAL_HISTOGRAM_CUSTOM_COUNTS(name, sample, 1, 100, 50)
+#define LOCAL_HISTOGRAM_COUNTS_100(constant_name, sample) \
+  LOCAL_HISTOGRAM_CUSTOM_COUNTS(constant_name, sample, 1, 100, 50)
 
-#define LOCAL_HISTOGRAM_COUNTS_10000(name, sample) \
-    LOCAL_HISTOGRAM_CUSTOM_COUNTS(name, sample, 1, 10000, 50)
+#define LOCAL_HISTOGRAM_COUNTS_10000(constant_name, sample) \
+  LOCAL_HISTOGRAM_CUSTOM_COUNTS(constant_name, sample, 1, 10000, 50)
 
-#define LOCAL_HISTOGRAM_CUSTOM_COUNTS(name, sample, min, max, bucket_count) \
-    STATIC_HISTOGRAM_POINTER_BLOCK(name, Add(sample), \
-        base::Histogram::FactoryGet(name, min, max, bucket_count, \
-                                    base::HistogramBase::kNoFlags))
+#define LOCAL_HISTOGRAM_CUSTOM_COUNTS(constant_name, sample, min, max,   \
+                                      bucket_count)                      \
+  STATIC_HISTOGRAM_POINTER_BLOCK(                                        \
+      constant_name, Add(sample),                                        \
+      base::Histogram::FactoryGet(constant_name, min, max, bucket_count, \
+                                  base::HistogramBase::kNoFlags))
 
 // This is a helper macro used by other macros and shouldn't be used directly.
-#define HISTOGRAM_ENUMERATION_WITH_FLAG(name, sample, boundary, flag) \
-    STATIC_HISTOGRAM_POINTER_BLOCK(name, Add(sample), \
-        base::LinearHistogram::FactoryGet(name, 1, boundary, boundary + 1, \
-            flag))
+#define HISTOGRAM_ENUMERATION_WITH_FLAG(constant_name, sample, boundary, flag) \
+  STATIC_HISTOGRAM_POINTER_BLOCK(                                              \
+      constant_name, Add(sample),                                              \
+      base::LinearHistogram::FactoryGet(constant_name, 1, boundary,            \
+                                        boundary + 1, flag))
 
-#define LOCAL_HISTOGRAM_PERCENTAGE(name, under_one_hundred) \
-    LOCAL_HISTOGRAM_ENUMERATION(name, under_one_hundred, 101)
+#define LOCAL_HISTOGRAM_PERCENTAGE(constant_name, under_one_hundred) \
+  LOCAL_HISTOGRAM_ENUMERATION(constant_name, under_one_hundred, 101)
 
-#define LOCAL_HISTOGRAM_BOOLEAN(name, sample) \
-    STATIC_HISTOGRAM_POINTER_BLOCK(name, AddBoolean(sample), \
-        base::BooleanHistogram::FactoryGet(name, base::Histogram::kNoFlags))
+#define LOCAL_HISTOGRAM_BOOLEAN(constant_name, sample)  \
+  STATIC_HISTOGRAM_POINTER_BLOCK(                       \
+      constant_name, AddBoolean(sample),                \
+      base::BooleanHistogram::FactoryGet(constant_name, \
+                                         base::Histogram::kNoFlags))
 
 // Support histograming of an enumerated value.  The samples should always be
 // strictly less than |boundary_value| -- this prevents you from running into
@@ -157,23 +169,27 @@
 // also that, despite explicitly setting the minimum bucket value to |1| below,
 // it is fine for enumerated histograms to be 0-indexed -- this is because
 // enumerated histograms should never have underflow.
-#define LOCAL_HISTOGRAM_ENUMERATION(name, sample, boundary_value) \
-    STATIC_HISTOGRAM_POINTER_BLOCK(name, Add(sample), \
-        base::LinearHistogram::FactoryGet(name, 1, boundary_value, \
-            boundary_value + 1, base::HistogramBase::kNoFlags))
+#define LOCAL_HISTOGRAM_ENUMERATION(constant_name, sample, boundary_value) \
+  STATIC_HISTOGRAM_POINTER_BLOCK(                                          \
+      constant_name, Add(sample),                                          \
+      base::LinearHistogram::FactoryGet(constant_name, 1, boundary_value,  \
+                                        boundary_value + 1,                \
+                                        base::HistogramBase::kNoFlags))
 
 // Support histograming of an enumerated value. Samples should be one of the
 // std::vector<int> list provided via |custom_ranges|. See comments above
 // CustomRanges::FactoryGet about the requirement of |custom_ranges|.
 // You can use the helper function CustomHistogram::ArrayToCustomRanges to
 // transform a C-style array of valid sample values to a std::vector<int>.
-#define LOCAL_HISTOGRAM_CUSTOM_ENUMERATION(name, sample, custom_ranges) \
-    STATIC_HISTOGRAM_POINTER_BLOCK(name, Add(sample), \
-        base::CustomHistogram::FactoryGet(name, custom_ranges, \
-                                          base::HistogramBase::kNoFlags))
+#define LOCAL_HISTOGRAM_CUSTOM_ENUMERATION(constant_name, sample,     \
+                                           custom_ranges)             \
+  STATIC_HISTOGRAM_POINTER_BLOCK(                                     \
+      constant_name, Add(sample),                                     \
+      base::CustomHistogram::FactoryGet(constant_name, custom_ranges, \
+                                        base::HistogramBase::kNoFlags))
 
-#define LOCAL_HISTOGRAM_MEMORY_KB(name, sample) LOCAL_HISTOGRAM_CUSTOM_COUNTS( \
-    name, sample, 1000, 500000, 50)
+#define LOCAL_HISTOGRAM_MEMORY_KB(constant_name, sample) \
+  LOCAL_HISTOGRAM_CUSTOM_COUNTS(constant_name, sample, 1000, 500000, 50)
 
 //------------------------------------------------------------------------------
 // The following macros provide typical usage scenarios for callers that wish
@@ -181,108 +197,124 @@
 // Not all systems support such UMA, but if they do, the following macros
 // should work with the service.
 
-#define UMA_HISTOGRAM_TIMES(name, sample) UMA_HISTOGRAM_CUSTOM_TIMES( \
-    name, sample, base::TimeDelta::FromMilliseconds(1), \
-    base::TimeDelta::FromSeconds(10), 50)
+#define UMA_HISTOGRAM_TIMES(constant_name, sample)                 \
+  UMA_HISTOGRAM_CUSTOM_TIMES(constant_name, sample,                \
+                             base::TimeDelta::FromMilliseconds(1), \
+                             base::TimeDelta::FromSeconds(10), 50)
 
-#define UMA_HISTOGRAM_MEDIUM_TIMES(name, sample) UMA_HISTOGRAM_CUSTOM_TIMES( \
-    name, sample, base::TimeDelta::FromMilliseconds(10), \
-    base::TimeDelta::FromMinutes(3), 50)
+#define UMA_HISTOGRAM_MEDIUM_TIMES(constant_name, sample)           \
+  UMA_HISTOGRAM_CUSTOM_TIMES(constant_name, sample,                 \
+                             base::TimeDelta::FromMilliseconds(10), \
+                             base::TimeDelta::FromMinutes(3), 50)
 
 // Use this macro when times can routinely be much longer than 10 seconds.
-#define UMA_HISTOGRAM_LONG_TIMES(name, sample) UMA_HISTOGRAM_CUSTOM_TIMES( \
-    name, sample, base::TimeDelta::FromMilliseconds(1), \
-    base::TimeDelta::FromHours(1), 50)
+#define UMA_HISTOGRAM_LONG_TIMES(constant_name, sample)            \
+  UMA_HISTOGRAM_CUSTOM_TIMES(constant_name, sample,                \
+                             base::TimeDelta::FromMilliseconds(1), \
+                             base::TimeDelta::FromHours(1), 50)
 
 // Use this macro when times can routinely be much longer than 10 seconds and
 // you want 100 buckets.
-#define UMA_HISTOGRAM_LONG_TIMES_100(name, sample) UMA_HISTOGRAM_CUSTOM_TIMES( \
-    name, sample, base::TimeDelta::FromMilliseconds(1), \
-    base::TimeDelta::FromHours(1), 100)
+#define UMA_HISTOGRAM_LONG_TIMES_100(constant_name, sample)        \
+  UMA_HISTOGRAM_CUSTOM_TIMES(constant_name, sample,                \
+                             base::TimeDelta::FromMilliseconds(1), \
+                             base::TimeDelta::FromHours(1), 100)
 
-#define UMA_HISTOGRAM_CUSTOM_TIMES(name, sample, min, max, bucket_count) \
-    STATIC_HISTOGRAM_POINTER_BLOCK(name, AddTime(sample), \
-        base::Histogram::FactoryTimeGet(name, min, max, bucket_count, \
-            base::HistogramBase::kUmaTargetedHistogramFlag))
+#define UMA_HISTOGRAM_CUSTOM_TIMES(constant_name, sample, min, max, \
+                                   bucket_count)                    \
+  STATIC_HISTOGRAM_POINTER_BLOCK(                                   \
+      constant_name, AddTime(sample),                               \
+      base::Histogram::FactoryTimeGet(                              \
+          constant_name, min, max, bucket_count,                    \
+          base::HistogramBase::kUmaTargetedHistogramFlag))
 
-#define UMA_HISTOGRAM_COUNTS(name, sample) UMA_HISTOGRAM_CUSTOM_COUNTS( \
-    name, sample, 1, 1000000, 50)
+#define UMA_HISTOGRAM_COUNTS(constant_name, sample) \
+  UMA_HISTOGRAM_CUSTOM_COUNTS(constant_name, sample, 1, 1000000, 50)
 
-#define UMA_HISTOGRAM_COUNTS_100(name, sample) UMA_HISTOGRAM_CUSTOM_COUNTS( \
-    name, sample, 1, 100, 50)
+#define UMA_HISTOGRAM_COUNTS_100(constant_name, sample) \
+  UMA_HISTOGRAM_CUSTOM_COUNTS(constant_name, sample, 1, 100, 50)
 
-#define UMA_HISTOGRAM_COUNTS_1000(name, sample) UMA_HISTOGRAM_CUSTOM_COUNTS( \
-    name, sample, 1, 1000, 50)
+#define UMA_HISTOGRAM_COUNTS_1000(constant_name, sample) \
+  UMA_HISTOGRAM_CUSTOM_COUNTS(constant_name, sample, 1, 1000, 50)
 
-#define UMA_HISTOGRAM_COUNTS_10000(name, sample) UMA_HISTOGRAM_CUSTOM_COUNTS( \
-    name, sample, 1, 10000, 50)
+#define UMA_HISTOGRAM_COUNTS_10000(constant_name, sample) \
+  UMA_HISTOGRAM_CUSTOM_COUNTS(constant_name, sample, 1, 10000, 50)
 
-#define UMA_HISTOGRAM_CUSTOM_COUNTS(name, sample, min, max, bucket_count) \
-    STATIC_HISTOGRAM_POINTER_BLOCK(name, Add(sample), \
-        base::Histogram::FactoryGet(name, min, max, bucket_count, \
-            base::HistogramBase::kUmaTargetedHistogramFlag))
+#define UMA_HISTOGRAM_CUSTOM_COUNTS(constant_name, sample, min, max, \
+                                    bucket_count)                    \
+  STATIC_HISTOGRAM_POINTER_BLOCK(                                    \
+      constant_name, Add(sample),                                    \
+      base::Histogram::FactoryGet(                                   \
+          constant_name, min, max, bucket_count,                     \
+          base::HistogramBase::kUmaTargetedHistogramFlag))
 
-#define UMA_HISTOGRAM_MEMORY_KB(name, sample) UMA_HISTOGRAM_CUSTOM_COUNTS( \
-    name, sample, 1000, 500000, 50)
+#define UMA_HISTOGRAM_MEMORY_KB(constant_name, sample) \
+  UMA_HISTOGRAM_CUSTOM_COUNTS(constant_name, sample, 1000, 500000, 50)
 
-#define UMA_HISTOGRAM_MEMORY_MB(name, sample) UMA_HISTOGRAM_CUSTOM_COUNTS( \
-    name, sample, 1, 1000, 50)
+#define UMA_HISTOGRAM_MEMORY_MB(constant_name, sample) \
+  UMA_HISTOGRAM_CUSTOM_COUNTS(constant_name, sample, 1, 1000, 50)
 
-#define UMA_HISTOGRAM_MEMORY_LARGE_MB(name, sample) \
-    UMA_HISTOGRAM_CUSTOM_COUNTS(name, sample, 1, 64000, 100)
+#define UMA_HISTOGRAM_MEMORY_LARGE_MB(constant_name, sample) \
+  UMA_HISTOGRAM_CUSTOM_COUNTS(constant_name, sample, 1, 64000, 100)
 
-#define UMA_HISTOGRAM_PERCENTAGE(name, under_one_hundred) \
-    UMA_HISTOGRAM_ENUMERATION(name, under_one_hundred, 101)
+#define UMA_HISTOGRAM_PERCENTAGE(constant_name, under_one_hundred) \
+  UMA_HISTOGRAM_ENUMERATION(constant_name, under_one_hundred, 101)
 
-#define UMA_HISTOGRAM_BOOLEAN(name, sample) \
-    STATIC_HISTOGRAM_POINTER_BLOCK(name, AddBoolean(sample), \
-        base::BooleanHistogram::FactoryGet(name, \
-            base::HistogramBase::kUmaTargetedHistogramFlag))
+#define UMA_HISTOGRAM_BOOLEAN(constant_name, sample) \
+  STATIC_HISTOGRAM_POINTER_BLOCK(                    \
+      constant_name, AddBoolean(sample),             \
+      base::BooleanHistogram::FactoryGet(            \
+          constant_name, base::HistogramBase::kUmaTargetedHistogramFlag))
 
 // The samples should always be strictly less than |boundary_value|.  For more
 // details, see the comment for the |LOCAL_HISTOGRAM_ENUMERATION| macro, above.
-#define UMA_HISTOGRAM_ENUMERATION(name, sample, boundary_value) \
-    HISTOGRAM_ENUMERATION_WITH_FLAG(name, sample, boundary_value, \
-        base::HistogramBase::kUmaTargetedHistogramFlag)
+#define UMA_HISTOGRAM_ENUMERATION(constant_name, sample, boundary_value) \
+  HISTOGRAM_ENUMERATION_WITH_FLAG(                                       \
+      constant_name, sample, boundary_value,                             \
+      base::HistogramBase::kUmaTargetedHistogramFlag)
 
 // Similar to UMA_HISTOGRAM_ENUMERATION, but used for recording stability
 // histograms.  Use this if recording a histogram that should be part of the
 // initial stability log.
-#define UMA_STABILITY_HISTOGRAM_ENUMERATION(name, sample, boundary_value) \
-    HISTOGRAM_ENUMERATION_WITH_FLAG(name, sample, boundary_value, \
-        base::HistogramBase::kUmaStabilityHistogramFlag)
-
-#define UMA_HISTOGRAM_CUSTOM_ENUMERATION(name, sample, custom_ranges) \
-    STATIC_HISTOGRAM_POINTER_BLOCK(name, Add(sample), \
-        base::CustomHistogram::FactoryGet(name, custom_ranges, \
-            base::HistogramBase::kUmaTargetedHistogramFlag))
+#define UMA_STABILITY_HISTOGRAM_ENUMERATION(constant_name, sample, \
+                                            boundary_value)        \
+  HISTOGRAM_ENUMERATION_WITH_FLAG(                                 \
+      constant_name, sample, boundary_value,                       \
+      base::HistogramBase::kUmaStabilityHistogramFlag)
+
+#define UMA_HISTOGRAM_CUSTOM_ENUMERATION(constant_name, sample, custom_ranges) \
+  STATIC_HISTOGRAM_POINTER_BLOCK(                                              \
+      constant_name, Add(sample),                                              \
+      base::CustomHistogram::FactoryGet(                                       \
+          constant_name, custom_ranges,                                        \
+          base::HistogramBase::kUmaTargetedHistogramFlag))
 
 // Scoped class which logs its time on this earth as a UMA statistic. This is
 // recommended for when you want a histogram which measures the time it takes
 // for a method to execute. This measures up to 10 seconds.
-#define SCOPED_UMA_HISTOGRAM_TIMER(name) \
-  SCOPED_UMA_HISTOGRAM_TIMER_EXPANDER(name, false, __COUNTER__)
+#define SCOPED_UMA_HISTOGRAM_TIMER(constant_name) \
+  SCOPED_UMA_HISTOGRAM_TIMER_EXPANDER(constant_name, false, __COUNTER__)
 
 // Similar scoped histogram timer, but this uses UMA_HISTOGRAM_LONG_TIMES_100,
 // which measures up to an hour, and uses 100 buckets. This is more expensive
 // to store, so only use if this often takes >10 seconds.
-#define SCOPED_UMA_HISTOGRAM_LONG_TIMER(name) \
-  SCOPED_UMA_HISTOGRAM_TIMER_EXPANDER(name, true, __COUNTER__)
+#define SCOPED_UMA_HISTOGRAM_LONG_TIMER(constant_name) \
+  SCOPED_UMA_HISTOGRAM_TIMER_EXPANDER(constant_name, true, __COUNTER__)
 
 // This nested macro is necessary to expand __COUNTER__ to an actual value.
-#define SCOPED_UMA_HISTOGRAM_TIMER_EXPANDER(name, is_long, key) \
-  SCOPED_UMA_HISTOGRAM_TIMER_UNIQUE(name, is_long, key)
+#define SCOPED_UMA_HISTOGRAM_TIMER_EXPANDER(constant_name, is_long, key) \
+  SCOPED_UMA_HISTOGRAM_TIMER_UNIQUE(constant_name, is_long, key)
 
-#define SCOPED_UMA_HISTOGRAM_TIMER_UNIQUE(name, is_long, key) \
+#define SCOPED_UMA_HISTOGRAM_TIMER_UNIQUE(constant_name, is_long, key) \
   class ScopedHistogramTimer##key { \
    public: \
     ScopedHistogramTimer##key() : constructed_(base::TimeTicks::Now()) {} \
     ~ScopedHistogramTimer##key() { \
       base::TimeDelta elapsed = base::TimeTicks::Now() - constructed_; \
       if (is_long) { \
-        UMA_HISTOGRAM_LONG_TIMES_100(name, elapsed); \
+        UMA_HISTOGRAM_LONG_TIMES_100(constant_name, elapsed); \
       } else { \
-        UMA_HISTOGRAM_TIMES(name, elapsed); \
+        UMA_HISTOGRAM_TIMES(constant_name, elapsed); \
       } \
     } \
    private: \