[UMA] Remove redundant bucket_count variable from base::Histogram.

base/metrics/histogram.cc

 // Copyright (c) 2012 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.
 
 // Histogram is an object that aggregates statistics, and can summarize them in
 // various forms, including ASCII graphical, HTML, and numerically (as a
 // vector of numbers corresponding to each of the aggregating buckets).
 // See header file for details and examples.
 
 #include "base/metrics/histogram.h"
@@ skipping 79 matching lines @@
                                      size_t bucket_count,
                                      int32 flags) {
   bool valid_arguments =
       InspectConstructionArguments(name, &minimum, &maximum, &bucket_count);
   DCHECK(valid_arguments);
 
   HistogramBase* histogram = StatisticsRecorder::FindHistogram(name);
   if (!histogram) {
     // To avoid racy destruction at shutdown, the following will be leaked.
     BucketRanges* ranges = new BucketRanges(bucket_count + 1);
-    InitializeBucketRanges(minimum, maximum, bucket_count, ranges);
+    InitializeBucketRanges(minimum, maximum, ranges);
     const BucketRanges* registered_ranges =
         StatisticsRecorder::RegisterOrDeleteDuplicateRanges(ranges);
 
     Histogram* tentative_histogram =
-        new Histogram(name, minimum, maximum, bucket_count, registered_ranges);
+        new Histogram(name, minimum, maximum, registered_ranges);
 
     tentative_histogram->SetFlags(flags);
     histogram =
         StatisticsRecorder::RegisterOrDeleteDuplicate(tentative_histogram);
   }
 
   DCHECK_EQ(HISTOGRAM, histogram->GetHistogramType());
   CHECK(histogram->HasConstructionArguments(minimum, maximum, bucket_count));
   return histogram;
 }
@@ skipping 20 matching lines @@
 // consecutive buckets that is sooo small, that the integer bounds are the same
 // (effectively making one bucket get no values).  We need to avoid:
 //   ranges(i) == ranges(i + 1)
 // To avoid that, we just do a fine-grained bucket width as far as we need to
 // until we get a ratio that moves us along at least 2 units at a time.  From
 // that bucket onward we do use the exponential growth of buckets.
 //
 // static
 void Histogram::InitializeBucketRanges(Sample minimum,
                                        Sample maximum,
-                                       size_t bucket_count,
                                        BucketRanges* ranges) {
-  DCHECK_EQ(ranges->size(), bucket_count + 1);
   double log_max = log(static_cast<double>(maximum));
   double log_ratio;
   double log_next;
   size_t bucket_index = 1;
   Sample current = minimum;
   ranges->set_range(bucket_index, current);
+  size_t bucket_count = ranges->bucket_count();
   while (bucket_count > ++bucket_index) {
     double log_current;
     log_current = log(static_cast<double>(current));
     // Calculate the count'th root of the range.
     log_ratio = (log_max - log_current) / (bucket_count - bucket_index);
     // See where the next bucket would start.
     log_next = log_current + log_ratio;
     Sample next;
     next = static_cast<int>(floor(exp(log_next) + 0.5));
     if (next > current)
       current = next;
     else
       ++current;  // Just do a narrow bucket, and keep trying.
     ranges->set_range(bucket_index, current);
   }
-  ranges->set_range(ranges->size() - 1, HistogramBase::kSampleType_MAX);
+  ranges->set_range(ranges->bucket_count(), HistogramBase::kSampleType_MAX);
   ranges->ResetChecksum();
 }
 
 // static
 const int Histogram::kCommonRaceBasedCountMismatch = 5;
 
 int Histogram::FindCorruption(const HistogramSamples& samples) const {
   int inconsistencies = NO_INCONSISTENCIES;
   Sample previous_range = -1;  // Bottom range is always 0.
   for (size_t index = 0; index < bucket_count(); ++index) {
@@ skipping 23 matching lines @@
     }
   }
   return inconsistencies;
 }
 
 Sample Histogram::ranges(size_t i) const {
   return bucket_ranges_->range(i);
 }
 
 size_t Histogram::bucket_count() const {
-  return bucket_count_;
+  return bucket_ranges_->bucket_count();
 }
 
 // static
 bool Histogram::InspectConstructionArguments(const string& name,
                                              Sample* minimum,
                                              Sample* maximum,
                                              size_t* bucket_count) {
   // Defensive code for backward compatibility.
   if (*minimum < 1) {
     DVLOG(1) << "Histogram: " << name << " has bad minimum: " << *minimum;
@@ skipping 19 matching lines @@
 }
 
 HistogramType Histogram::GetHistogramType() const {
   return HISTOGRAM;
 }
 
 bool Histogram::HasConstructionArguments(Sample minimum,
                                          Sample maximum,
                                          size_t bucket_count) const {
   return ((minimum == declared_min_) && (maximum == declared_max_) &&
-          (bucket_count == bucket_count_));
+          (bucket_count == this->bucket_count()));

[jar (doing other things), 2013/06/22 01:23:15]

nit: This is made me re-read a few times.... perhaps better would be to rename
the argument |new_bucket_count| and then you don't need the "this->" to
disambiguate.

WDYT?

[Ilya Sherman, 2013/06/27 04:00:11]

Done.

 }
 
 void Histogram::Add(int value) {
   DCHECK_EQ(0, ranges(0));
-  DCHECK_EQ(kSampleType_MAX, ranges(bucket_count_));
+  DCHECK_EQ(kSampleType_MAX, ranges(bucket_count()));
 
   if (value > kSampleType_MAX - 1)
     value = kSampleType_MAX - 1;
   if (value < 0)
     value = 0;
   samples_->Accumulate(value, 1);
 }
 
 scoped_ptr<HistogramSamples> Histogram::SnapshotSamples() const {
   return SnapshotSampleVector().PassAs<HistogramSamples>();
@@ skipping 25 matching lines @@
       pickle->WriteInt(flags()) &&
       pickle->WriteInt(declared_min()) &&
       pickle->WriteInt(declared_max()) &&
       pickle->WriteUInt64(bucket_count()) &&
       pickle->WriteUInt32(bucket_ranges()->checksum());
 }
 
 Histogram::Histogram(const string& name,
                      Sample minimum,
                      Sample maximum,
-                     size_t bucket_count,
                      const BucketRanges* ranges)
   : HistogramBase(name),
     bucket_ranges_(ranges),
     declared_min_(minimum),
-    declared_max_(maximum),
-    bucket_count_(bucket_count) {
+    declared_max_(maximum) {
   if (ranges)
     samples_.reset(new SampleVector(ranges));
 }
 
 Histogram::~Histogram() {
 }
 
 bool Histogram::PrintEmptyBucket(size_t index) const {
   return true;
 }
@@ skipping 219 matching lines @@
       int32 flags,
       const DescriptionPair descriptions[]) {
   bool valid_arguments = Histogram::InspectConstructionArguments(
       name, &minimum, &maximum, &bucket_count);
   DCHECK(valid_arguments);
 
   HistogramBase* histogram = StatisticsRecorder::FindHistogram(name);
   if (!histogram) {
     // To avoid racy destruction at shutdown, the following will be leaked.
     BucketRanges* ranges = new BucketRanges(bucket_count + 1);
-    InitializeBucketRanges(minimum, maximum, bucket_count, ranges);
+    InitializeBucketRanges(minimum, maximum, ranges);
     const BucketRanges* registered_ranges =
         StatisticsRecorder::RegisterOrDeleteDuplicateRanges(ranges);
 
     LinearHistogram* tentative_histogram =
-        new LinearHistogram(name, minimum, maximum, bucket_count,
-                            registered_ranges);
+        new LinearHistogram(name, minimum, maximum, registered_ranges);
 
     // Set range descriptions.
     if (descriptions) {
       for (int i = 0; descriptions[i].description; ++i) {
         tentative_histogram->bucket_description_[descriptions[i].sample] =
             descriptions[i].description;
       }
     }
 
     tentative_histogram->SetFlags(flags);
     histogram =
         StatisticsRecorder::RegisterOrDeleteDuplicate(tentative_histogram);
   }
 
   DCHECK_EQ(LINEAR_HISTOGRAM, histogram->GetHistogramType());
   CHECK(histogram->HasConstructionArguments(minimum, maximum, bucket_count));
   return histogram;
 }
 
 HistogramType LinearHistogram::GetHistogramType() const {
   return LINEAR_HISTOGRAM;
 }
 
 LinearHistogram::LinearHistogram(const string& name,
                                  Sample minimum,
                                  Sample maximum,
-                                 size_t bucket_count,
                                  const BucketRanges* ranges)
-    : Histogram(name, minimum, maximum, bucket_count, ranges) {
+    : Histogram(name, minimum, maximum, ranges) {
 }
 
 double LinearHistogram::GetBucketSize(Count current, size_t i) const {
   DCHECK_GT(ranges(i + 1), ranges(i));
   // Adjacent buckets with different widths would have "surprisingly" many (few)
   // samples in a histogram if we didn't normalize this way.
   double denominator = ranges(i + 1) - ranges(i);
   return current/denominator;
 }
 
 const string LinearHistogram::GetAsciiBucketRange(size_t i) const {
   int range = ranges(i);
   BucketDescriptionMap::const_iterator it = bucket_description_.find(range);
   if (it == bucket_description_.end())
     return Histogram::GetAsciiBucketRange(i);
   return it->second;
 }
 
 bool LinearHistogram::PrintEmptyBucket(size_t index) const {
   return bucket_description_.find(ranges(index)) == bucket_description_.end();
 }
 
 // static
 void LinearHistogram::InitializeBucketRanges(Sample minimum,
                                              Sample maximum,
-                                             size_t bucket_count,
                                              BucketRanges* ranges) {
-  DCHECK_EQ(ranges->size(), bucket_count + 1);
   double min = minimum;
   double max = maximum;
-  size_t i;
-  for (i = 1; i < bucket_count; ++i) {
+  size_t bucket_count = ranges->bucket_count();
+  for (size_t i = 1; i < bucket_count; ++i) {
     double linear_range =
-        (min * (bucket_count -1 - i) + max * (i - 1)) / (bucket_count - 2);
+        (min * (bucket_count - 1 - i) + max * (i - 1)) / (bucket_count - 2);
     ranges->set_range(i, static_cast<Sample>(linear_range + 0.5));
   }
-  ranges->set_range(ranges->size() - 1, HistogramBase::kSampleType_MAX);
+  ranges->set_range(ranges->bucket_count(), HistogramBase::kSampleType_MAX);
   ranges->ResetChecksum();
 }
 
 // static
 HistogramBase* LinearHistogram::DeserializeInfoImpl(PickleIterator* iter) {
   string histogram_name;
   int flags;
   int declared_min;
   int declared_max;
   uint64 bucket_count;
@@ skipping 15 matching lines @@
 
 //------------------------------------------------------------------------------
 // This section provides implementation for BooleanHistogram.
 //------------------------------------------------------------------------------
 
 HistogramBase* BooleanHistogram::FactoryGet(const string& name, int32 flags) {
   HistogramBase* histogram = StatisticsRecorder::FindHistogram(name);
   if (!histogram) {
     // To avoid racy destruction at shutdown, the following will be leaked.
     BucketRanges* ranges = new BucketRanges(4);
-    LinearHistogram::InitializeBucketRanges(1, 2, 3, ranges);
+    LinearHistogram::InitializeBucketRanges(1, 2, ranges);
     const BucketRanges* registered_ranges =
         StatisticsRecorder::RegisterOrDeleteDuplicateRanges(ranges);
 
     BooleanHistogram* tentative_histogram =
         new BooleanHistogram(name, registered_ranges);
 
     tentative_histogram->SetFlags(flags);
     histogram =
         StatisticsRecorder::RegisterOrDeleteDuplicate(tentative_histogram);
   }
 
   DCHECK_EQ(BOOLEAN_HISTOGRAM, histogram->GetHistogramType());
   return histogram;
 }
 
 HistogramType BooleanHistogram::GetHistogramType() const {
   return BOOLEAN_HISTOGRAM;
 }
 
 BooleanHistogram::BooleanHistogram(const string& name,
                                    const BucketRanges* ranges)
-    : LinearHistogram(name, 1, 2, 3, ranges) {}
+    : LinearHistogram(name, 1, 2, ranges) {}
 
 HistogramBase* BooleanHistogram::DeserializeInfoImpl(PickleIterator* iter) {
   string histogram_name;
   int flags;
   int declared_min;
   int declared_max;
   uint64 bucket_count;
   uint32 range_checksum;
 
   if (!ReadHistogramArguments(iter, &histogram_name, &flags, &declared_min,
@@ skipping 55 matching lines @@
     // values, FactoryGet will take care of removing them.
     all_values.push_back(value + 1);
   }
   return all_values;
 }
 
 CustomHistogram::CustomHistogram(const string& name,
                                  const BucketRanges* ranges)
     : Histogram(name,
                 ranges->range(1),
-                ranges->range(ranges->size() - 2),
-                ranges->size() - 1,
+                ranges->range(ranges->bucket_count() - 1),
                 ranges) {}
 
 bool CustomHistogram::SerializeInfoImpl(Pickle* pickle) const {
   if (!Histogram::SerializeInfoImpl(pickle))
     return false;
 
   // Serialize ranges. First and last ranges are alwasy 0 and INT_MAX, so don't
   // write them.
-  for (size_t i = 1; i < bucket_ranges()->size() - 1; ++i) {
+  for (size_t i = 1; i < bucket_ranges()->bucket_count(); ++i) {
     if (!pickle->WriteInt(bucket_ranges()->range(i)))
       return false;
   }
   return true;
 }
 
 double CustomHistogram::GetBucketSize(Count current, size_t i) const {
   return 1;
 }
 
@@ skipping 54 matching lines @@
 
   BucketRanges* bucket_ranges = new BucketRanges(ranges.size());
   for (size_t i = 0; i < ranges.size(); i++) {
     bucket_ranges->set_range(i, ranges[i]);
   }
   bucket_ranges->ResetChecksum();
   return bucket_ranges;
 }
 
 }  // namespace base