bug 1218576 - Aggregate child categorical histograms in the parent process. r=gfritzsche
authorChris H-C <chutten@mozilla.com>
Fri, 12 Aug 2016 14:51:48 -0400
changeset 314448 c97a5b2b086accd52fb5dd0b27eb6d5127fb6fc8
parent 314447 6e20455447661423af908193c9458895df211c7e
child 314449 5d3383a8df5f53550e54f8132fdb69b4479f721c
push id20574
push usercbook@mozilla.com
push dateTue, 20 Sep 2016 10:05:16 +0000
treeherderfx-team@14705f779a46 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgfritzsche
bugs1218576
milestone51.0a1
bug 1218576 - Aggregate child categorical histograms in the parent process. r=gfritzsche Rewrite the categorical histogram accumulation code to use the common path. This way it gets remote accumulation for cheap. MozReview-Commit-ID: 3q6gdSvBix
toolkit/components/telemetry/TelemetryHistogram.cpp
--- a/toolkit/components/telemetry/TelemetryHistogram.cpp
+++ b/toolkit/components/telemetry/TelemetryHistogram.cpp
@@ -653,33 +653,16 @@ internal_HistogramAdd(Histogram& histogr
       return NS_OK;
     }
     dataset = gHistograms[id].dataset;
   }
 
   return internal_HistogramAdd(histogram, value, dataset);
 }
 
-nsresult
-internal_HistogramAddCategorical(mozilla::Telemetry::ID id, const nsCString& label)
-{
-  uint32_t labelId = 0;
-  if (NS_FAILED(gHistograms[id].label_id(label.get(), &labelId))) {
-    return NS_ERROR_ILLEGAL_VALUE;
-  }
-
-  Histogram* h = nullptr;
-  nsresult rv = internal_GetHistogramByEnumId(id, &h);
-  if (NS_FAILED(rv)) {
-    return rv;
-  }
-
-  return internal_HistogramAdd(*h, labelId);
-}
-
 void
 internal_HistogramClear(Histogram& aHistogram, bool onlySubsession)
 {
   MOZ_ASSERT(XRE_IsParentProcess());
   if (!XRE_IsParentProcess()) {
     return;
   }
   if (!onlySubsession) {
@@ -1470,23 +1453,21 @@ internal_JSHistogram_Add(JSContext *cx, 
       gHistograms[id].histogramType == nsITelemetry::HISTOGRAM_CATEGORICAL) {
     // For categorical histograms we allow passing a string argument that specifies the label.
     nsAutoJSString label;
     if (!label.init(cx, args[0])) {
       JS_ReportError(cx, "Invalid string parameter");
       return false;
     }
 
-    nsresult rv = internal_HistogramAddCategorical(id, NS_ConvertUTF16toUTF8(label));
+    nsresult rv = gHistograms[id].label_id(NS_ConvertUTF16toUTF8(label).get(), &value);
     if (NS_FAILED(rv)) {
       JS_ReportError(cx, "Unknown label for categorical histogram");
       return false;
     }
-
-    return true;
   } else {
     // All other accumulations expect one numerical argument.
     if (!args.length()) {
       JS_ReportError(cx, "Expected one argument");
       return false;
     }
 
     if (!(args[0].isNumber() || args[0].isBoolean())) {
@@ -2145,17 +2126,21 @@ TelemetryHistogram::Accumulate(const cha
 void
 TelemetryHistogram::AccumulateCategorical(mozilla::Telemetry::ID aId,
                                           const nsCString& label)
 {
   StaticMutexAutoLock locker(gTelemetryHistogramMutex);
   if (!internal_CanRecordBase()) {
     return;
   }
-  internal_HistogramAddCategorical(aId, label);
+  uint32_t labelId = 0;
+  if (NS_FAILED(gHistograms[aId].label_id(label.get(), &labelId))) {
+    return;
+  }
+  internal_Accumulate(aId, labelId);
 }
 
 void
 TelemetryHistogram::AccumulateChild(const nsTArray<Accumulation>& aAccumulations)
 {
   MOZ_ASSERT(XRE_IsParentProcess());
   StaticMutexAutoLock locker(gTelemetryHistogramMutex);
   if (!internal_CanRecordBase()) {