Clean up sparse_histogram.h header

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 9cb21ec563a2ed54e45cdd885b4d2501a84ad985..7da7e73bda984b2ac36cf120cb42cef497bc8318 100644
--- a/base/metrics/histogram_macros.h
+++ b/base/metrics/histogram_macros.h
@@ -10,10 +10,6 @@
 #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.
 
@@ -228,6 +224,29 @@
         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.