Bug 1457127 - Create common helpers for taking histogram snapshots. r?chutten,janerik draft
authorAlessio Placitelli <alessio.placitelli@gmail.com>
Wed, 16 May 2018 13:09:33 +0200
changeset 797012 308147ead26c1969e534b6c97d63e54e3f5918fc
parent 796662 54063deb2f1caf8c4acf6461d3ba779805835c96
child 797013 ce886f36d4965c776acdd866c2c5ab5ce5fe117f
push id110396
push userbmo:alessio.placitelli@gmail.com
push dateFri, 18 May 2018 15:30:15 +0000
reviewerschutten, janerik
bugs1457127
milestone62.0a1
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,