Clean up sparse_histogram.h header

base/metrics/histogram_macros.h

 // Copyright 2014 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.
 
 #ifndef BASE_METRICS_HISTOGRAM_MACROS_H_
 #define BASE_METRICS_HISTOGRAM_MACROS_H_
 
 #include "base/metrics/histogram.h"
 #include "base/metrics/histogram_macros_internal.h"
 #include "base/metrics/histogram_macros_local.h"
 #include "base/time/time.h"
 
-// TODO(nikunjb): Move sparse macros to this file.
-// UMA_HISTOGRAM_SPARSE_SLOWLY is defined in sparse_histogram.h as it has
-// different #include dependencies.
-
 // TODO(rkaplow): Link to proper documentation on metric creation once we have
 // it in a good state.
 
 // All of these macros must be called with |name| as a runtime constant - it
 // doesn't have to literally be a constant, but it must be the same string on
 // all calls from a particular call site. If this rule is violated, it is
 // possible the data will be written to the wrong histogram.
 
 //------------------------------------------------------------------------------
 // Enumeration histograms.
@@ skipping 194 matching lines @@
                                               bucket_count)                    \
     INTERNAL_HISTOGRAM_CUSTOM_COUNTS_WITH_FLAG(                                \
         name, sample, min, max, bucket_count,                                  \
         base::HistogramBase::kUmaStabilityHistogramFlag)
 
 #define UMA_STABILITY_HISTOGRAM_ENUMERATION(name, sample, enum_max)            \
     INTERNAL_HISTOGRAM_ENUMERATION_WITH_FLAG(                                  \
         name, sample, enum_max,                                                \
         base::HistogramBase::kUmaStabilityHistogramFlag)
 
+// Sparse histograms are well suited for recording counts of exact sample values

[rkaplow, 2016/09/30 14:55:00]

Can you keep the formatting I added in the rest of the file? I.e. sections
grouped with 
//------------------------------------------------------------------------------
headers

[nikunjb1, 2016/10/03 22:25:41]

Done.

+// that are sparsely distributed over a large range.
+//
+// The implementation uses a lock and a map, whereas other histogram types use a

[rkaplow, 2016/09/30 14:55:00]

I think some of this technical detail could be reduced - this should just
explain when it should be used (sparse data) and how it is used

[nikunjb1, 2016/10/03 22:25:41]

Done. Moved implementation comments to internals.

+// vector and no lock. It is thus more costly to add values to, and each value
+// stored has more overhead, compared to the other histogram types. However it
+// may be more efficient in memory if the total number of sample values is small
+// compared to the range of their values.
+//
+// UMA_HISTOGRAM_ENUMERATION would be better suited for a smaller range of

[rkaplow, 2016/09/30 14:55:00]

Don't need to document ENUM here I think. If we wanted to mention this - maybe
put this near the enum macro

[nikunjb1, 2016/10/03 22:25:41]

Done.

+// enumerations that are (nearly) contiguous. Also for code that is expected to
+// run often or in a tight loop.
+//
+// UMA_HISTOGRAM_SPARSE_SLOWLY is good for sparsely distributed and or
+// infrequently recorded values.
+//
+// For instance, Sqlite.Version.* are SPARSE because for any given database,
+// there's going to be exactly one version logged, meaning no gain to having a
+// pre-allocated vector of slots once the fleet gets to version 4 or 5 or 10.

[rkaplow, 2016/09/30 14:55:00]

I'm not even sure what this comment means when it says "the fleet gets to
version 4 or 5..."

[nikunjb1, 2016/10/03 22:25:42]

Done. Added clarification.

+// Likewise Sqlite.Error.* are SPARSE, because most databases generate few or no
+// errors and there are large gaps in the set of possible errors.
+#define UMA_HISTOGRAM_SPARSE_SLOWLY(name, sample)                              \
+    INTERNAL_HISTOGRAM_SPARSE_SLOWLY(name, sample)
 
 //------------------------------------------------------------------------------
 // Deprecated histogram macros. Not recommended for current use.
 
 // Legacy name for UMA_HISTOGRAM_COUNTS_1M. Suggest using explicit naming
 // and not using this macro going forward.
 #define UMA_HISTOGRAM_COUNTS(name, sample) UMA_HISTOGRAM_CUSTOM_COUNTS(        \
     name, sample, 1, 1000000, 50)
 
 // MB-granularity memory metric. This has a short max (1G).
 #define UMA_HISTOGRAM_MEMORY_MB(name, sample)                                  \
     UMA_HISTOGRAM_CUSTOM_COUNTS(name, sample, 1, 1000, 50)
 
 // For an enum with customized range. In general, sparse histograms should be
 // used instead.
 // 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 UMA_HISTOGRAM_CUSTOM_ENUMERATION(name, sample, custom_ranges)          \
     STATIC_HISTOGRAM_POINTER_BLOCK(name, Add(sample),                          \
         base::CustomHistogram::FactoryGet(name, custom_ranges,                 \
             base::HistogramBase::kUmaTargetedHistogramFlag))
 
 #endif  // BASE_METRICS_HISTOGRAM_MACROS_H_