Cleanup histogram classes mixing SetFlags into FactoryGet arguments

Generic cleanup of histogram class, renaming *FactoryGet to FactoryGet,
along with reformatting.

The macros were cleaned up to use common sub-macros rather than
repeating code as much.

Removed ThreadSafeHistogram (and associated ASSET_HISTOGRAM macros)
since this class was not getting used.

I introduced UMA_HISTOGRAM_ENUMERATION to support the common use
of LinearHistograms to count various enumerated values.

I added a Flags argument to all the FactoryGet routines to help avoid 
needing to call SetFlags each time a new sample is Add()ed.  This also
simplifies the code.

This will all help prepare for a "don't histogram at all" macro setting
so that I can test the impact of the histogram macro calls on
performance (since there are now so many active histograms).

BUG=31206
r=raman.tenneti
Review URL: http://codereview.chromium.org/515033

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@35295 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 50 insertions, 136 deletions

diff --git a/base/histogram_unittest.cc b/base/histogram_unittest.cc
index 4d5de51..f702a2e 100644
--- a/base/histogram_unittest.cc
+++ b/base/histogram_unittest.cc
@@ -19,17 +19,15 @@ class HistogramTest : public testing::Test {
 // Check for basic syntax and use.
 TEST(HistogramTest, StartupShutdownTest) {
   // Try basic construction
-  scoped_refptr<Histogram> histogram =
-    Histogram::HistogramFactoryGet("TestHistogram", 1, 1000, 10);
-  scoped_refptr<Histogram> histogram1 =
-      Histogram::HistogramFactoryGet("Test1Histogram", 1, 1000, 10);
-
-  scoped_refptr<Histogram> linear_histogram =
-      LinearHistogram::LinearHistogramFactoryGet("TestLinearHistogram", 1, 1000,
-          10);
-  scoped_refptr<Histogram> linear_histogram1 =
-      LinearHistogram::LinearHistogramFactoryGet("Test1LinearHistogram", 1,
-          1000, 10);
+  scoped_refptr<Histogram> histogram = Histogram::FactoryGet(
+      "TestHistogram", 1, 1000, 10, Histogram::kNoFlags);
+  scoped_refptr<Histogram> histogram1 = Histogram::FactoryGet(
+      "Test1Histogram", 1, 1000, 10, Histogram::kNoFlags);
+
+  scoped_refptr<Histogram> linear_histogram = LinearHistogram::FactoryGet(
+      "TestLinearHistogram", 1, 1000, 10, Histogram::kNoFlags);
+  scoped_refptr<Histogram> linear_histogram1 = LinearHistogram::FactoryGet(
+      "Test1LinearHistogram", 1, 1000, 10, Histogram::kNoFlags);
 
   // Use standard macros (but with fixed samples)
   HISTOGRAM_TIMES("Test2Histogram", TimeDelta::FromDays(1));
@@ -38,7 +36,7 @@ TEST(HistogramTest, StartupShutdownTest) {
   DHISTOGRAM_TIMES("Test4Histogram", TimeDelta::FromDays(1));
   DHISTOGRAM_COUNTS("Test5Histogram", 30);
 
-  ASSET_HISTOGRAM_COUNTS("Test6Histogram", 129);
+  HISTOGRAM_ENUMERATION("Test6Histogram", 129, 130);
 
   // Try to construct samples.
   Histogram::SampleSet sample1;
@@ -63,23 +61,21 @@ TEST(HistogramTest, RecordedStartupTest) {
   EXPECT_EQ(0U, histograms.size());
 
   // Try basic construction
-  scoped_refptr<Histogram> histogram =
-      Histogram::HistogramFactoryGet("TestHistogram", 1, 1000, 10);
+  scoped_refptr<Histogram> histogram = Histogram::FactoryGet(
+      "TestHistogram", 1, 1000, 10, Histogram::kNoFlags);
   histograms.clear();
   StatisticsRecorder::GetHistograms(&histograms);  // Load up lists
   EXPECT_EQ(1U, histograms.size());
-  scoped_refptr<Histogram> histogram1 =
-      Histogram::HistogramFactoryGet("Test1Histogram", 1, 1000, 10);
+  scoped_refptr<Histogram> histogram1 = Histogram::FactoryGet(
+      "Test1Histogram", 1, 1000, 10, Histogram::kNoFlags);
   histograms.clear();
   StatisticsRecorder::GetHistograms(&histograms);  // Load up lists
   EXPECT_EQ(2U, histograms.size());
 
-  scoped_refptr<Histogram> linear_histogram =
-      LinearHistogram::LinearHistogramFactoryGet(
-          "TestLinearHistogram", 1, 1000, 10);
-  scoped_refptr<Histogram> linear_histogram1 =
-      LinearHistogram::LinearHistogramFactoryGet(
-          "Test1LinearHistogram", 1, 1000, 10);
+  scoped_refptr<Histogram> linear_histogram = LinearHistogram::FactoryGet(
+      "TestLinearHistogram", 1, 1000, 10, Histogram::kNoFlags);
+  scoped_refptr<Histogram> linear_histogram1 = LinearHistogram::FactoryGet(
+      "Test1LinearHistogram", 1, 1000, 10, Histogram::kNoFlags);
   histograms.clear();
   StatisticsRecorder::GetHistograms(&histograms);  // Load up lists
   EXPECT_EQ(4U, histograms.size());
@@ -91,7 +87,7 @@ TEST(HistogramTest, RecordedStartupTest) {
   StatisticsRecorder::GetHistograms(&histograms);  // Load up lists
   EXPECT_EQ(6U, histograms.size());
 
-  ASSET_HISTOGRAM_COUNTS("TestAssetHistogram", 1000);
+  HISTOGRAM_ENUMERATION("TestEnumerationHistogram", 20, 200);
   histograms.clear();
   StatisticsRecorder::GetHistograms(&histograms);  // Load up lists
   EXPECT_EQ(7U, histograms.size());
@@ -114,8 +110,8 @@ TEST(HistogramTest, RangeTest) {
   recorder.GetHistograms(&histograms);
   EXPECT_EQ(0U, histograms.size());
 
-  scoped_refptr<Histogram> histogram = Histogram::HistogramFactoryGet(
-      "Histogram", 1, 64, 8);  // As mentioned in header file.
+  scoped_refptr<Histogram> histogram = Histogram::FactoryGet(
+      "Histogram", 1, 64, 8, Histogram::kNoFlags);  // As per header file.
   // Check that we got a nice exponential when there was enough rooom.
   EXPECT_EQ(0, histogram->ranges(0));
   int power_of_2 = 1;
@@ -125,51 +121,48 @@ TEST(HistogramTest, RangeTest) {
   }
   EXPECT_EQ(INT_MAX, histogram->ranges(8));
 
-  scoped_refptr<Histogram> short_histogram =
-      Histogram::HistogramFactoryGet("Histogram Shortened", 1, 7, 8);
+  scoped_refptr<Histogram> short_histogram = Histogram::FactoryGet(
+      "Histogram Shortened", 1, 7, 8, Histogram::kNoFlags);
   // Check that when the number of buckets is short, we get a linear histogram
   // for lack of space to do otherwise.
   for (int i = 0; i < 8; i++)
     EXPECT_EQ(i, short_histogram->ranges(i));
   EXPECT_EQ(INT_MAX, short_histogram->ranges(8));
 
-  scoped_refptr<Histogram> linear_histogram =
-      LinearHistogram::LinearHistogramFactoryGet("Linear", 1, 7, 8);
+  scoped_refptr<Histogram> linear_histogram = LinearHistogram::FactoryGet(
+      "Linear", 1, 7, 8, Histogram::kNoFlags);
   // We also get a nice linear set of bucket ranges when we ask for it
   for (int i = 0; i < 8; i++)
     EXPECT_EQ(i, linear_histogram->ranges(i));
   EXPECT_EQ(INT_MAX, linear_histogram->ranges(8));
 
-  scoped_refptr<Histogram> linear_broad_histogram =
-      LinearHistogram::LinearHistogramFactoryGet(
-          "Linear widened", 2, 14, 8);
+  scoped_refptr<Histogram> linear_broad_histogram = LinearHistogram::FactoryGet(
+      "Linear widened", 2, 14, 8, Histogram::kNoFlags);
   // ...but when the list has more space, then the ranges naturally spread out.
   for (int i = 0; i < 8; i++)
     EXPECT_EQ(2 * i, linear_broad_histogram->ranges(i));
   EXPECT_EQ(INT_MAX, linear_broad_histogram->ranges(8));
 
-  scoped_refptr<Histogram> threadsafe_histogram =
-      ThreadSafeHistogram::ThreadSafeHistogramFactoryGet("ThreadSafe", 1, 32,
-          15);
+  scoped_refptr<Histogram> transitioning_histogram =
+      Histogram::FactoryGet("LinearAndExponential", 1, 32, 15,
+                                      Histogram::kNoFlags);
   // When space is a little tight, we transition from linear to exponential.
-  // This is what happens in both the basic histogram, and the threadsafe
-  // variant (which is derived).
-  EXPECT_EQ(0, threadsafe_histogram->ranges(0));
-  EXPECT_EQ(1, threadsafe_histogram->ranges(1));
-  EXPECT_EQ(2, threadsafe_histogram->ranges(2));
-  EXPECT_EQ(3, threadsafe_histogram->ranges(3));
-  EXPECT_EQ(4, threadsafe_histogram->ranges(4));
-  EXPECT_EQ(5, threadsafe_histogram->ranges(5));
-  EXPECT_EQ(6, threadsafe_histogram->ranges(6));
-  EXPECT_EQ(7, threadsafe_histogram->ranges(7));
-  EXPECT_EQ(9, threadsafe_histogram->ranges(8));
-  EXPECT_EQ(11, threadsafe_histogram->ranges(9));
-  EXPECT_EQ(14, threadsafe_histogram->ranges(10));
-  EXPECT_EQ(17, threadsafe_histogram->ranges(11));
-  EXPECT_EQ(21, threadsafe_histogram->ranges(12));
-  EXPECT_EQ(26, threadsafe_histogram->ranges(13));
-  EXPECT_EQ(32, threadsafe_histogram->ranges(14));
-  EXPECT_EQ(INT_MAX, threadsafe_histogram->ranges(15));
+  EXPECT_EQ(0, transitioning_histogram->ranges(0));
+  EXPECT_EQ(1, transitioning_histogram->ranges(1));
+  EXPECT_EQ(2, transitioning_histogram->ranges(2));
+  EXPECT_EQ(3, transitioning_histogram->ranges(3));
+  EXPECT_EQ(4, transitioning_histogram->ranges(4));
+  EXPECT_EQ(5, transitioning_histogram->ranges(5));
+  EXPECT_EQ(6, transitioning_histogram->ranges(6));
+  EXPECT_EQ(7, transitioning_histogram->ranges(7));
+  EXPECT_EQ(9, transitioning_histogram->ranges(8));
+  EXPECT_EQ(11, transitioning_histogram->ranges(9));
+  EXPECT_EQ(14, transitioning_histogram->ranges(10));
+  EXPECT_EQ(17, transitioning_histogram->ranges(11));
+  EXPECT_EQ(21, transitioning_histogram->ranges(12));
+  EXPECT_EQ(26, transitioning_histogram->ranges(13));
+  EXPECT_EQ(32, transitioning_histogram->ranges(14));
+  EXPECT_EQ(INT_MAX, transitioning_histogram->ranges(15));
 
   recorder.GetHistograms(&histograms);
   EXPECT_EQ(5U, histograms.size());
@@ -178,8 +171,8 @@ TEST(HistogramTest, RangeTest) {
 // Make sure histogram handles out-of-bounds data gracefully.
 TEST(HistogramTest, BoundsTest) {
   const size_t kBucketCount = 50;
-  scoped_refptr<Histogram> histogram = Histogram::HistogramFactoryGet("Bounded",
-     10, 100, kBucketCount);
+  scoped_refptr<Histogram> histogram = Histogram::FactoryGet(
+      "Bounded", 10, 100, kBucketCount, Histogram::kNoFlags);
 
   // Put two samples "out of bounds" above and below.
   histogram->Add(5);
@@ -201,8 +194,8 @@ TEST(HistogramTest, BoundsTest) {
 
 // Check to be sure samples land as expected is "correct" buckets.
 TEST(HistogramTest, BucketPlacementTest) {
-  scoped_refptr<Histogram> histogram = Histogram::HistogramFactoryGet(
-      "Histogram", 1, 64, 8);  // As mentioned in header file.
+  scoped_refptr<Histogram> histogram = Histogram::FactoryGet(
+      "Histogram", 1, 64, 8, Histogram::kNoFlags);  // As per header file.
 
   // Check that we got a nice exponential since there was enough rooom.
   EXPECT_EQ(0, histogram->ranges(0));
@@ -231,84 +224,5 @@ TEST(HistogramTest, BucketPlacementTest) {
     EXPECT_EQ(i + 1, sample.counts(i));
 }
 
-static const char kAssetTestHistogramName[] = "AssetCountTest";
-static const char kAssetTestDebugHistogramName[] = "DAssetCountTest";
-void AssetCountFunction(int sample) {
-  ASSET_HISTOGRAM_COUNTS(kAssetTestHistogramName, sample);
-  DASSET_HISTOGRAM_COUNTS(kAssetTestDebugHistogramName, sample);
-}
-// Check that asset can be added and removed from buckets.
-TEST(HistogramTest, AssetCountTest) {
-  // Start up a recorder system to identify all histograms.
-  StatisticsRecorder recorder;
-
-  // Call through the macro to instantiate the static variables.
-  AssetCountFunction(100);  // Put a sample in the bucket for 100.
-
-  // Find the histogram.
-  StatisticsRecorder::Histograms histogram_list;
-  StatisticsRecorder::GetHistograms(&histogram_list);
-  ASSERT_NE(0U, histogram_list.size());
-  const Histogram* our_histogram = NULL;
-  const Histogram* our_debug_histogram = NULL;
-  for (StatisticsRecorder::Histograms::iterator it = histogram_list.begin();
-       it != histogram_list.end();
-       ++it) {
-    if (!(*it)->histogram_name().compare(kAssetTestHistogramName))
-      our_histogram = *it;
-    else if (!(*it)->histogram_name().compare(kAssetTestDebugHistogramName)) {
-      our_debug_histogram = *it;
-    }
-  }
-  ASSERT_TRUE(our_histogram);
-#ifndef NDEBUG
-  EXPECT_TRUE(our_debug_histogram);
-#else
-  EXPECT_FALSE(our_debug_histogram);
-#endif
-  // Verify it has a 1 in exactly one bucket (where we put the sample).
-  Histogram::SampleSet sample;
-  our_histogram->SnapshotSample(&sample);
-  int match_count = 0;
-  for (size_t i = 0; i < our_histogram->bucket_count(); ++i) {
-    if (sample.counts(i) > 0) {
-      EXPECT_LT(++match_count, 2) << "extra count in bucket " << i;
-    }
-  }
-  EXPECT_EQ(1, match_count);
-
-  // Remove our sample.
-  AssetCountFunction(-100);  // Remove a sample from the bucket for 100.
-  our_histogram->SnapshotSample(&sample);  // Extract data set.
-
-  // Verify that the bucket is now empty, as are all the other buckets.
-  for (size_t i = 0; i < our_histogram->bucket_count(); ++i) {
-    EXPECT_EQ(0, sample.counts(i)) << "extra count in bucket " << i;
-  }
-
-  if (!our_debug_histogram)
-    return;  // This is a production build.
-
-  // Repeat test with debug histogram.  Note that insertion and deletion above
-  // should have cancelled each other out.
-  AssetCountFunction(100);  // Add a sample into the bucket for 100.
-  our_debug_histogram->SnapshotSample(&sample);
-  match_count = 0;
-  for (size_t i = 0; i < our_debug_histogram->bucket_count(); ++i) {
-    if (sample.counts(i) > 0) {
-      EXPECT_LT(++match_count, 2) << "extra count in bucket " << i;
-    }
-  }
-  EXPECT_EQ(1, match_count);
-
-  // Remove our sample.
-  AssetCountFunction(-100);  // Remove a sample from the bucket for 100.
-  our_debug_histogram->SnapshotSample(&sample);  // Extract data set.
-
-  // Verify that the bucket is now empty, as are all the other buckets.
-  for (size_t i = 0; i < our_debug_histogram->bucket_count(); ++i) {
-    EXPECT_EQ(0, sample.counts(i)) << "extra count in bucket " << i;
-  }
-}
 
 }  // namespace