bug 1218577 - Support subsession hgrams for child aggregation r=gfritzsche
☠☠ backed out by 1530103e032c ☠ ☠
authorChris H-C <chutten@mozilla.com>
Wed, 29 Jun 2016 15:36:07 -0400
changeset 312993 a0a4829d0ca0
parent 312992 fc16cda7781b
child 312994 7fdd6b6ab594
push id20479
push userkwierso@gmail.com
push dateThu, 08 Sep 2016 01:08:46 +0000
treeherderfx-team@fb7c6b034329 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgfritzsche
bugs1218577
milestone51.0a1
bug 1218577 - Support subsession hgrams for child aggregation r=gfritzsche The original commit didn't properly support subsession histograms, so rectify that lapse by adding support for stripping out the base name of a histogram when trying to determine its id. MozReview-Commit-ID: LvUek6f5WUx
toolkit/components/telemetry/TelemetryHistogram.cpp
--- a/toolkit/components/telemetry/TelemetryHistogram.cpp
+++ b/toolkit/components/telemetry/TelemetryHistogram.cpp
@@ -331,16 +331,26 @@ HistogramInfo::label_id(const char* labe
       *labelId = i;
       return NS_OK;
     }
   }
 
   return NS_ERROR_FAILURE;
 }
 
+bool
+StringEndsWith(const std::string& name, const std::string& suffix)
+{
+  if (name.size() < suffix.size()) {
+    return false;
+  }
+
+  return name.compare(name.size() - suffix.size(), suffix.size(), suffix) == 0;
+}
+
 } // namespace
 
 
 ////////////////////////////////////////////////////////////////////////
 ////////////////////////////////////////////////////////////////////////
 //
 // PRIVATE: Histogram Get, Add, Clone, Clear functions
 
@@ -415,24 +425,36 @@ internal_HistogramGet(const char *name, 
     break;
   default:
     NS_ASSERTION(false, "Invalid histogram type");
     return NS_ERROR_INVALID_ARG;
   }
   return NS_OK;
 }
 
+CharPtrEntryType*
+internal_GetHistogramMapEntry(const char* name)
+{
+  nsDependentCString histogramName(name);
+  NS_NAMED_LITERAL_CSTRING(suffix, CHILD_HISTOGRAM_SUFFIX);
+  if (!StringEndsWith(histogramName, suffix)) {
+    return gHistogramMap.GetEntry(name);
+  }
+  auto root = Substring(histogramName, 0, histogramName.Length() - suffix.Length());
+  return gHistogramMap.GetEntry(PromiseFlatCString(root).get());
+}
+
 nsresult
 internal_GetHistogramEnumId(const char *name, mozilla::Telemetry::ID *id)
 {
   if (!gInitDone) {
     return NS_ERROR_FAILURE;
   }
 
-  CharPtrEntryType *entry = gHistogramMap.GetEntry(name);
+  CharPtrEntryType *entry = internal_GetHistogramMapEntry(name);
   if (!entry) {
     return NS_ERROR_INVALID_ARG;
   }
   *id = entry->mData;
   return NS_OK;
 }
 
 // O(1) histogram lookup by numeric id
@@ -494,17 +516,19 @@ internal_GetHistogramByName(const nsACSt
 {
   mozilla::Telemetry::ID id;
   nsresult rv
     = internal_GetHistogramEnumId(PromiseFlatCString(name).get(), &id);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
-  rv = internal_GetHistogramByEnumId(id, ret);
+  bool isChild = StringEndsWith(name,
+                                NS_LITERAL_CSTRING(CHILD_HISTOGRAM_SUFFIX));
+  rv = internal_GetHistogramByEnumId(id, ret, isChild);
   if (NS_FAILED(rv))
     return rv;
 
   return NS_OK;
 }
 
 /**
  * This clones a histogram |existing| with the id |existingId| to a
@@ -550,42 +574,53 @@ internal_CloneHistogram(const nsACString
   if (NS_FAILED(rv)) {
     return nullptr;
   }
 
   return internal_CloneHistogram(newName, existingId, *existing);
 }
 
 #if !defined(MOZ_WIDGET_GONK) && !defined(MOZ_WIDGET_ANDROID)
+
 Histogram*
 internal_GetSubsessionHistogram(Histogram& existing)
 {
   mozilla::Telemetry::ID id;
   nsresult rv
     = internal_GetHistogramEnumId(existing.histogram_name().c_str(), &id);
   if (NS_FAILED(rv) || gHistograms[id].keyed) {
     return nullptr;
   }
 
+  bool isChild = StringEndsWith(existing.histogram_name(),
+                                CHILD_HISTOGRAM_SUFFIX);
+
   static Histogram* subsession[mozilla::Telemetry::HistogramCount] = {};
-  if (subsession[id]) {
-    return subsession[id];
+  static Histogram* subsessionChild[mozilla::Telemetry::HistogramCount] = {};
+  Histogram* cached = isChild ? subsessionChild[id] : subsession[id];
+  if (cached) {
+    return cached;
   }
 
   NS_NAMED_LITERAL_CSTRING(prefix, SUBSESSION_HISTOGRAM_PREFIX);
   nsDependentCString existingName(gHistograms[id].id());
   if (StringBeginsWith(existingName, prefix)) {
     return nullptr;
   }
 
   nsCString subsessionName(prefix);
-  subsessionName.Append(existingName);
+  subsessionName.Append(existing.histogram_name().c_str());
 
-  subsession[id] = internal_CloneHistogram(subsessionName, id, existing);
-  return subsession[id];
+  Histogram* clone = internal_CloneHistogram(subsessionName, id, existing);
+  if (isChild) {
+    subsessionChild[id] = clone;
+  } else {
+    subsession[id] = clone;
+  }
+  return clone;
 }
 #endif
 
 nsresult
 internal_HistogramAdd(Histogram& histogram, int32_t value, uint32_t dataset)
 {
   // Check if we are allowed to record the data.
   bool canRecordDataset = CanRecordDataset(dataset,