Bug 1457127 - Create common helpers for taking histogram snapshots. r=chutten,janerik
authorAlessio Placitelli <alessio.placitelli@gmail.com>
Wed, 16 May 2018 13:09:33 +0200
changeset 419063 440073e5e466ea0662537569b6b1539b089b35fe
parent 419062 2af84ea1c49dbf09d9f4349ff7899f98e53eeb4e
child 419064 ae169019f9e4e1734f6ed63c0ef7430fe9a74e22
push id34025
push userapavel@mozilla.com
push dateMon, 21 May 2018 09:46:09 +0000
treeherdermozilla-central@8850728602d6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerschutten, janerik
bugs1457127
milestone62.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1457127 - Create common helpers for taking histogram snapshots. r=chutten,janerik This patch factors out the code needed to take snapshots for plain histograms. These functions will be used for generating a snapshot for the persistence logic as well. MozReview-Commit-ID: 37kx3WidlhJ
toolkit/components/telemetry/TelemetryHistogram.cpp
--- a/toolkit/components/telemetry/TelemetryHistogram.cpp
+++ b/toolkit/components/telemetry/TelemetryHistogram.cpp
@@ -147,24 +147,32 @@ struct HistogramInfo {
   bool allows_key(const nsACString& key) const;
 };
 
 enum reflectStatus {
   REFLECT_OK,
   REFLECT_FAILURE
 };
 
-// Struct used to keep information about the histograms for which a
+// Structs used to keep information about the histograms for which a
 // snapshot should be created.
 struct HistogramSnapshotData {
   nsTArray<Histogram::Sample> mBucketRanges;
   nsTArray<Histogram::Count> mBucketCounts;
   int64_t mSampleSum; // Same type as Histogram::SampleSet::sum_
 };
 
+struct HistogramSnapshotInfo {
+  HistogramSnapshotData data;
+  HistogramID histogramID;
+};
+
+typedef mozilla::Vector<HistogramSnapshotInfo> HistogramSnapshotsArray;
+typedef mozilla::Vector<HistogramSnapshotsArray> HistogramProcessSnapshotsArray;
+
 // The following is used to handle snapshot information for keyed histograms.
 typedef nsDataHashtable<nsCStringHashKey, HistogramSnapshotData> KeyedHistogramSnapshotData;
 
 class KeyedHistogram {
 public:
   KeyedHistogram(HistogramID id, const HistogramInfo& info);
   ~KeyedHistogram();
   nsresult GetHistogram(const nsCString& name, Histogram** histogram);
@@ -744,16 +752,84 @@ internal_ShouldReflectHistogram(Histogra
   uint32_t type = gHistogramInfos[id].histogramType;
   if (internal_IsEmpty(h) && type != nsITelemetry::HISTOGRAM_FLAG) {
     return false;
   }
 
   return true;
 }
 
+/**
+ * Helper function to get a snapshot of the histograms.
+ *
+ * @param {aLock} the lock proof.
+ * @param {aDataset} the dataset for which the snapshot is being requested.
+ * @param {aClearSubsession} whether or not to clear the data after
+ *        taking the snapshot.
+ * @param {aIncludeGPU} whether or not to include data for the GPU.
+ * @param {aOutSnapshot} the container in which the snapshot data will be stored.
+ * @return {nsresult} NS_OK if the snapshot was successfully taken or
+ *         NS_ERROR_OUT_OF_MEMORY if it failed to allocate memory.
+ */
+nsresult
+internal_GetHistogramsSnapshot(const StaticMutexAutoLock& aLock,
+                               unsigned int aDataset,
+                               bool aClearSubsession,
+                               bool aIncludeGPU,
+                               HistogramProcessSnapshotsArray& aOutSnapshot)
+{
+  if (!aOutSnapshot.resize(static_cast<uint32_t>(ProcessID::Count))) {
+    return NS_ERROR_OUT_OF_MEMORY;
+  }
+
+  for (uint32_t process = 0; process < static_cast<uint32_t>(ProcessID::Count); ++process) {
+    HistogramSnapshotsArray& hArray = aOutSnapshot[process];
+
+    for (size_t i = 0; i < HistogramCount; ++i) {
+      const HistogramInfo& info = gHistogramInfos[i];
+      if (info.keyed) {
+        continue;
+      }
+
+      HistogramID id = HistogramID(i);
+
+      if (!CanRecordInProcess(info.record_in_processes, ProcessID(process)) ||
+          ((ProcessID(process) == ProcessID::Gpu) && !aIncludeGPU)) {
+        continue;
+      }
+
+      if (!IsInDataset(info.dataset, aDataset)) {
+        continue;
+      }
+
+      bool shouldInstantiate =
+        info.histogramType == nsITelemetry::HISTOGRAM_FLAG;
+      Histogram* h = internal_GetHistogramById(id, ProcessID(process),
+                                               shouldInstantiate);
+      if (!h || internal_IsExpired(h) || !internal_ShouldReflectHistogram(h, id)) {
+        continue;
+      }
+
+      HistogramSnapshotData snapshotData;
+      if (NS_FAILED(internal_GetHistogramAndSamples(aLock, h, snapshotData))) {
+        continue;
+      }
+
+      if (!hArray.emplaceBack(HistogramSnapshotInfo{snapshotData, id})) {
+        return NS_ERROR_OUT_OF_MEMORY;
+      }
+
+      if (aClearSubsession) {
+        h->Clear();
+      }
+    }
+  }
+  return NS_OK;
+}
+
 } // namespace
 
 ////////////////////////////////////////////////////////////////////////
 ////////////////////////////////////////////////////////////////////////
 //
 // PRIVATE: class KeyedHistogram and internal_ReflectKeyedHistogram
 
 namespace {
@@ -2220,89 +2296,42 @@ TelemetryHistogram::CreateHistogramSnaps
 
   // Include the GPU process in histogram snapshots only if we actually tried
   // to launch a process for it.
   bool includeGPUProcess = false;
   if (auto gpm = mozilla::gfx::GPUProcessManager::Get()) {
     includeGPUProcess = gpm->AttemptedGPUProcess();
   }
 
-  // Struct used to keep information about the histograms for which a
-  // snapshot should be created
-  struct HistogramProcessInfo {
-    HistogramSnapshotData data;
-    HistogramID histogramID;
-  };
-
-  mozilla::Vector<mozilla::Vector<HistogramProcessInfo>> processHistArray;
+  HistogramProcessSnapshotsArray processHistArray;
   {
-    if (!processHistArray.resize(static_cast<uint32_t>(ProcessID::Count))) {
-      return NS_ERROR_OUT_OF_MEMORY;
-    }
-
     StaticMutexAutoLock locker(gTelemetryHistogramMutex);
-    for (uint32_t process = 0; process < static_cast<uint32_t>(ProcessID::Count); ++process) {
-      mozilla::Vector<HistogramProcessInfo>& hArray = processHistArray[process];
-
-      for (size_t i = 0; i < HistogramCount; ++i) {
-        const HistogramInfo& info = gHistogramInfos[i];
-        if (info.keyed) {
-          continue;
-        }
-
-        HistogramID id = HistogramID(i);
-
-        if (!CanRecordInProcess(info.record_in_processes, ProcessID(process)) ||
-            ((ProcessID(process) == ProcessID::Gpu) && !includeGPUProcess)) {
-          continue;
-        }
-
-        if (!IsInDataset(info.dataset, aDataset)) {
-          continue;
-        }
-
-        bool shouldInstantiate =
-          info.histogramType == nsITelemetry::HISTOGRAM_FLAG;
-        Histogram* h = internal_GetHistogramById(id, ProcessID(process),
-                                                 shouldInstantiate);
-        if (!h || internal_IsExpired(h) || !internal_ShouldReflectHistogram(h, id)) {
-          continue;
-        }
-
-        HistogramSnapshotData snapshotData;
-        if (NS_FAILED(internal_GetHistogramAndSamples(locker, h, snapshotData))) {
-          continue;
-        }
-
-        if (!hArray.emplaceBack(HistogramProcessInfo{snapshotData, id})) {
-          return NS_ERROR_OUT_OF_MEMORY;
-        }
-
-        if (aClearSubsession) {
-          h->Clear();
-        }
-      }
+    nsresult rv = internal_GetHistogramsSnapshot(locker,
+                                                 aDataset,
+                                                 aClearSubsession,
+                                                 includeGPUProcess,
+                                                 processHistArray);
+    if (NS_FAILED(rv)) {
+      return rv;
     }
   }
 
   // Make the JS calls on the stashed histograms for every process
   for (uint32_t process = 0; process < processHistArray.length(); ++process) {
     JS::Rooted<JSObject*> processObject(aCx, JS_NewPlainObject(aCx));
     if (!processObject) {
       return NS_ERROR_FAILURE;
     }
     if (!JS_DefineProperty(aCx, root_obj,
                            GetNameForProcessID(ProcessID(process)),
                            processObject, JSPROP_ENUMERATE)) {
       return NS_ERROR_FAILURE;
     }
 
-    const mozilla::Vector<HistogramProcessInfo>& hArray = processHistArray[process];
-    for (size_t i = 0; i < hArray.length(); ++i) {
-      const HistogramProcessInfo& hData = hArray[i];
+    for (const HistogramSnapshotInfo& hData : processHistArray[process]) {
       HistogramID id = hData.histogramID;
 
       JS::Rooted<JSObject*> hobj(aCx, JS_NewPlainObject(aCx));
       if (!hobj) {
         return NS_ERROR_FAILURE;
       }
 
       if (NS_FAILED(internal_ReflectHistogramAndSamples(aCx,