Bug 1383210 - Use precomputed histogram buckets r=gfritzsche

The log and exp calls in base::Histogram::InitializeBucketRange()
were showing up in profiles. This patch uses the precomputed
buckets for exponential histograms instead of computing them at
runtime. Though linear histograms do show up in the profile that
prompted this change, they contribute much less, and due to the
trivial nature of generating these, it's unlikely that a static
cache would provide much if any speedup.

MozReview-Commit-ID: IavFwoWjFhk

--HG--
extra : rebase_source : ad7d641ab2982f5cf8d202c7c382bfc26daa4bd5

ipc/chromium/src/base/histogram.cc

@@ -84,7 +84,9 @@ const size_t Histogram::kBucketCount_MAX = 16384u;
 Histogram* Histogram::FactoryGet(Sample minimum,
                                  Sample maximum,
                                  size_t bucket_count,
-                                 Flags flags) {
+                                 Flags flags,
+                                 const int* buckets) {
+  DCHECK(buckets);
   Histogram* histogram(NULL);
 
   // Defensive code.
@@ -94,7 +96,7 @@ Histogram* Histogram::FactoryGet(Sample minimum,
     maximum = kSampleType_MAX - 1;
 
   histogram = new Histogram(minimum, maximum, bucket_count);
-  histogram->InitializeBucketRange();
+  histogram->InitializeBucketRangeFromData(buckets);
   histogram->SetFlags(flags);
 
   DCHECK_EQ(HISTOGRAM, histogram->histogram_type());
@@ -102,14 +104,6 @@ Histogram* Histogram::FactoryGet(Sample minimum,
   return histogram;
 }
 
-Histogram* Histogram::FactoryTimeGet(TimeDelta minimum,
-                                     TimeDelta maximum,
-                                     size_t bucket_count,
-                                     Flags flags) {
-  return FactoryGet(minimum.InMilliseconds(), maximum.InMilliseconds(),
-                    bucket_count, flags);
-}
-
 void Histogram::Add(int value) {
   if (value > kSampleType_MAX - 1)
     value = kSampleType_MAX - 1;
@@ -278,39 +272,10 @@ Histogram::~Histogram() {
   DCHECK(ValidateBucketRanges());
 }
 
-// Calculate what range of values are held in each bucket.
-// We have to be careful that we don't pick a ratio between starting points in
-// 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.
-void Histogram::InitializeBucketRange() {
-  double log_max = log(static_cast<double>(declared_max()));
-  double log_ratio;
-  double log_next;
-  size_t bucket_index = 1;
-  Sample current = declared_min();
-  SetBucketRange(bucket_index, current);
-  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;
-    int 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.
-    SetBucketRange(bucket_index, current);
-  }
+void Histogram::InitializeBucketRangeFromData(const int* buckets) {
+  ranges_.assign(buckets, buckets + bucket_count());
   ResetRangeChecksum();
-
-  DCHECK_EQ(bucket_count(), bucket_index);
+  DCHECK(ValidateBucketRanges());
 }
 
 bool Histogram::PrintEmptyBucket(size_t index) const {

ipc/chromium/src/base/histogram.h

@@ -180,11 +180,8 @@ class Histogram {
   static Histogram* FactoryGet(Sample minimum,
                                Sample maximum,
                                size_t bucket_count,
-                               Flags flags);
-  static Histogram* FactoryTimeGet(base::TimeDelta minimum,
-                                   base::TimeDelta maximum,
-                                   size_t bucket_count,
-                                   Flags flags);
+                               Flags flags,
+                               const int* buckets);
 
   virtual ~Histogram();
 
@@ -248,8 +245,8 @@ class Histogram {
   Histogram(Sample minimum, Sample maximum, size_t bucket_count);
   Histogram(TimeDelta minimum, TimeDelta maximum, size_t bucket_count);
 
-  // Initialize ranges_ mapping.
-  void InitializeBucketRange();
+  // Initialize ranges_ mapping from raw data.
+  void InitializeBucketRangeFromData(const int* buckets);
 
   // Method to override to skip the display of the i'th bucket if it's empty.
   virtual bool PrintEmptyBucket(size_t index) const;

toolkit/components/telemetry/TelemetryHistogram.cpp

@@ -240,7 +240,7 @@ namespace {
 
 // Factory function for histogram instances.
 Histogram*
-internal_CreateHistogramInstance(const HistogramInfo& info);
+internal_CreateHistogramInstance(const HistogramInfo& info, int bucketsOffset);
 
 bool
 internal_IsHistogramEnumId(HistogramID aID)
@@ -265,7 +265,8 @@ internal_GetHistogramById(HistogramID histogramId, ProcessID processId, SessionT
   }
 
   const HistogramInfo& info = gHistogramInfos[histogramId];
-  h = internal_CreateHistogramInstance(info);
+  const int bucketsOffset = gExponentialBucketLowerBoundIndex[histogramId];
+  h = internal_CreateHistogramInstance(info, bucketsOffset);
   MOZ_ASSERT(h);
   gHistogramStorage[histogramId][uint32_t(processId)][uint32_t(sessionType)] = h;
   return h;
@@ -456,7 +457,7 @@ internal_CheckHistogramArguments(const HistogramInfo& info)
 }
 
 Histogram*
-internal_CreateHistogramInstance(const HistogramInfo& passedInfo)
+internal_CreateHistogramInstance(const HistogramInfo& passedInfo, int bucketsOffset)
 {
   if (NS_FAILED(internal_CheckHistogramArguments(passedInfo))) {
     MOZ_ASSERT(false, "Failed histogram argument checks.");
@@ -483,7 +484,8 @@ internal_CreateHistogramInstance(const HistogramInfo& passedInfo)
   Histogram* h = nullptr;
   switch (info.histogramType) {
   case nsITelemetry::HISTOGRAM_EXPONENTIAL:
-    h = Histogram::FactoryGet(info.min, info.max, info.bucketCount, flags);
+    h = Histogram::FactoryGet(info.min, info.max, info.bucketCount, flags,
+                              &gExponentialBucketLowerBounds[bucketsOffset]);
     break;
   case nsITelemetry::HISTOGRAM_LINEAR:
   case nsITelemetry::HISTOGRAM_CATEGORICAL:
@@ -679,7 +681,8 @@ KeyedHistogram::GetHistogram(const nsCString& key, Histogram** histogram,
     return NS_OK;
   }
 
-  Histogram* h = internal_CreateHistogramInstance(mHistogramInfo);
+  int bucketsOffset = gExponentialBucketLowerBoundIndex[mId];
+  Histogram* h = internal_CreateHistogramInstance(mHistogramInfo, bucketsOffset);
   if (!h) {
     return NS_ERROR_FAILURE;
   }

toolkit/components/telemetry/gen-histogram-data.py

@@ -136,6 +136,37 @@ def write_histogram_static_asserts(output, histograms):
         fn(output, histogram)
 
 
+def write_exponential_histogram_ranges(output, histograms):
+    # For now we use this as a special cache only for exponential histograms,
+    # which require exp and log calls that show up in profiles. Initialization
+    # of other histograms also shows up in profiles, but it's unlikely that we
+    # would see much speedup since calculating their buckets is fairly trivial,
+    # and grabbing them from static data would likely incur a CPU cache miss.
+    print("const int gExponentialBucketLowerBounds[] = {", file=output)
+    for histogram in histograms:
+        if histogram.kind() == 'exponential':
+            ranges = histogram.ranges()
+            print(','.join(map(str, ranges)), ',', file=output)
+    print("};", file=output)
+
+    print("const int gExponentialBucketLowerBoundIndex[] = {", file=output)
+    offset = 0
+    for histogram in histograms:
+        cpp_guard = histogram.cpp_guard()
+        if cpp_guard:
+            print("#if defined(%s)" % cpp_guard, file=output)
+
+        if histogram.kind() == 'exponential':
+            print("%d," % offset, file=output)
+            offset += histogram.n_buckets()
+        else:
+            print("-1,", file=output)
+
+        if cpp_guard:
+            print("#endif", file=output)
+    print("};", file=output)
+
+
 def write_debug_histogram_ranges(output, histograms):
     ranges_lengths = []
 
@@ -190,6 +221,7 @@ def main(output, *filenames):
 
     print(banner, file=output)
     write_histogram_table(output, histograms)
+    write_exponential_histogram_ranges(output, histograms)
     write_histogram_static_asserts(output, histograms)
     write_debug_histogram_ranges(output, histograms)