bug 1366294 - Part 7 - Use keyed histogram snaphots to build payloads. r?gfritzsche draft
authorChris H-C <chutten@mozilla.com>
Fri, 23 Jun 2017 16:38:21 -0400
changeset 610615 c0c623dd482a2c6432038774f5daf24a80465b67
parent 610614 b7e74b6ae8997dfc1664580a6a538b482173e7e2
child 610616 4ff421d296f23db6363b8ba97a067755d2d3c071
push id68956
push userbmo:chutten@mozilla.com
push dateTue, 18 Jul 2017 15:29:20 +0000
reviewersgfritzsche
bugs1366294
milestone56.0a1
bug 1366294 - Part 7 - Use keyed histogram snaphots to build payloads. r?gfritzsche TelemetrySession's getKeyedHistograms asks for each keyed histogram individually. This is inefficient and doesn't work well with the storage refactor. So, plumb through a subsession keyed histogram snapshot API and convert TelemetrySession over to using it. MozReview-Commit-ID: Af9dTqw99UA
toolkit/components/telemetry/Telemetry.cpp
toolkit/components/telemetry/Telemetry.h
toolkit/components/telemetry/TelemetryHistogram.cpp
toolkit/components/telemetry/TelemetryHistogram.h
toolkit/components/telemetry/TelemetrySession.jsm
toolkit/components/telemetry/nsITelemetry.idl
--- a/toolkit/components/telemetry/Telemetry.cpp
+++ b/toolkit/components/telemetry/Telemetry.cpp
@@ -853,17 +853,30 @@ TelemetryImpl::SnapshotSubsessionHistogr
 #else
   return NS_OK;
 #endif
 }
 
 NS_IMETHODIMP
 TelemetryImpl::GetKeyedHistogramSnapshots(JSContext *cx, JS::MutableHandle<JS::Value> ret)
 {
-  return TelemetryHistogram::GetKeyedHistogramSnapshots(cx, ret);
+  return TelemetryHistogram::GetKeyedHistogramSnapshots(cx, ret, false, false);
+}
+
+NS_IMETHODIMP
+TelemetryImpl::SnapshotSubsessionKeyedHistograms(bool clearSubsession,
+                                                 JSContext *cx,
+                                                 JS::MutableHandle<JS::Value> ret)
+{
+#if !defined(MOZ_WIDGET_ANDROID)
+  return TelemetryHistogram::GetKeyedHistogramSnapshots(cx, ret, true,
+                                                        clearSubsession);
+#else
+  return NS_OK;
+#endif
 }
 
 bool
 TelemetryImpl::GetSQLStats(JSContext *cx, JS::MutableHandle<JS::Value> ret, bool includePrivateSql)
 {
   JS::Rooted<JSObject*> root_obj(cx, JS_NewPlainObject(cx));
   if (!root_obj)
     return false;
--- a/toolkit/components/telemetry/Telemetry.h
+++ b/toolkit/components/telemetry/Telemetry.h
@@ -138,17 +138,17 @@ void AccumulateCategorical(HistogramID i
  *
  * @param id - histogram id
  * @param start - start time
  * @param end - end time
  */
 void AccumulateTimeDelta(HistogramID id, TimeStamp start, TimeStamp end = TimeStamp::Now());
 
 /**
- * Enable/disable recording for this histogram at runtime.
+ * Enable/disable recording for this histogram in this process at runtime.
  * Recording is enabled by default, unless listed at kRecordingInitiallyDisabledIDs[].
  * id must be a valid telemetry enum, otherwise an assertion is triggered.
  *
  * @param id - histogram id
  * @param enabled - whether or not to enable recording from now on.
  */
 void SetHistogramRecordingEnabled(HistogramID id, bool enabled);
 
--- a/toolkit/components/telemetry/TelemetryHistogram.cpp
+++ b/toolkit/components/telemetry/TelemetryHistogram.cpp
@@ -2011,56 +2011,77 @@ TelemetryHistogram::RegisteredKeyedHisto
 {
   StaticMutexAutoLock locker(gTelemetryHistogramMutex);
   return internal_GetRegisteredHistogramIds(true,
                                             aDataset, aCount, aHistograms);
 }
 
 nsresult
 TelemetryHistogram::GetKeyedHistogramSnapshots(JSContext *cx,
-                                               JS::MutableHandle<JS::Value> ret)
+                                               JS::MutableHandle<JS::Value> ret,
+                                               bool subsession,
+                                               bool clearSubsession)
 {
   // Runs without protection from |gTelemetryHistogramMutex|
   JS::Rooted<JSObject*> obj(cx, JS_NewPlainObject(cx));
   if (!obj) {
     return NS_ERROR_FAILURE;
   }
-
-  // for (auto iter = gKeyedHistograms.Iter(); !iter.Done(); iter.Next()) {
-  for (uint32_t id = 0; id < HistogramCount; ++id) {
-    if (!gHistogramInfos[id].keyed) {
-      continue;
-    }
-
-    // TODO: This won't get us data from different processes.
-    // We'll have to refactor this function to return {"parent": {...}, "content": {...}}.
+  ret.setObject(*obj);
 
-    // We don't want to trigger instantiation of empty keyed histograms.
-    KeyedHistogram* keyed = internal_GetKeyedHistogramById(HistogramID(id), ProcessID::Parent,
-                                                           /* instantiate = */ false);
-    if (!keyed) {
-      continue;
+  // 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();
+  }
+
+  for (uint32_t process = 0; process < static_cast<uint32_t>(ProcessID::Count); ++process) {
+    JS::Rooted<JSObject*> processObject(cx, JS_NewPlainObject(cx));
+    if (!processObject) {
+      return NS_ERROR_FAILURE;
     }
-
-    JS::RootedObject snapshot(cx, JS_NewPlainObject(cx));
-    if (!snapshot) {
+    if (!JS_DefineProperty(cx, obj, GetNameForProcessID(ProcessID(process)),
+                           processObject, JSPROP_ENUMERATE)) {
       return NS_ERROR_FAILURE;
     }
+    for (size_t id = 0; id < HistogramCount; ++id) {
+      const HistogramInfo& info = gHistogramInfos[id];
+      if (!info.keyed) {
+        continue;
+      }
 
-    if (!NS_SUCCEEDED(keyed->GetJSSnapshot(cx, snapshot, false, false))) {
-      return NS_ERROR_FAILURE;
-    }
+      if (!CanRecordInProcess(info.record_in_processes, ProcessID(process)) ||
+        ((ProcessID(process) == ProcessID::Gpu) && !includeGPUProcess)) {
+        continue;
+      }
+
+      KeyedHistogram* keyed = internal_GetKeyedHistogramById(HistogramID(id),
+                                                             ProcessID(process),
+                                                             /* instantiate = */ false);
+      if (!keyed) {
+        continue;
+      }
 
-    if (!JS_DefineProperty(cx, obj, gHistogramInfos[id].name(),
-                           snapshot, JSPROP_ENUMERATE)) {
-      return NS_ERROR_FAILURE;
+      JS::RootedObject snapshot(cx, JS_NewPlainObject(cx));
+      if (!snapshot) {
+        return NS_ERROR_FAILURE;
+      }
+
+      if (!NS_SUCCEEDED(keyed->GetJSSnapshot(cx, snapshot, subsession,
+                                             clearSubsession))) {
+        return NS_ERROR_FAILURE;
+      }
+
+      if (!JS_DefineProperty(cx, processObject, gHistogramInfos[id].name(),
+                             snapshot, JSPROP_ENUMERATE)) {
+        return NS_ERROR_FAILURE;
+      }
     }
   }
-
-  ret.setObject(*obj);
   return NS_OK;
 }
 
 size_t
 TelemetryHistogram::GetMapShallowSizesOfExcludingThis(mozilla::MallocSizeOf
                                                       aMallocSizeOf)
 {
   StaticMutexAutoLock locker(gTelemetryHistogramMutex);
--- a/toolkit/components/telemetry/TelemetryHistogram.h
+++ b/toolkit/components/telemetry/TelemetryHistogram.h
@@ -67,17 +67,18 @@ nsresult
 RegisteredHistograms(uint32_t aDataset, uint32_t *aCount,
                      char*** aHistograms);
 
 nsresult
 RegisteredKeyedHistograms(uint32_t aDataset, uint32_t *aCount,
                           char*** aHistograms);
 
 nsresult
-GetKeyedHistogramSnapshots(JSContext *cx, JS::MutableHandle<JS::Value> ret);
+GetKeyedHistogramSnapshots(JSContext *cx, JS::MutableHandle<JS::Value> ret,
+                           bool subsession, bool clearSubsession);
 
 size_t
 GetMapShallowSizesOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf);
 
 size_t
 GetHistogramSizesofIncludingThis(mozilla::MallocSizeOf aMallocSizeOf);
 
 } // namespace TelemetryHistogram
--- a/toolkit/components/telemetry/TelemetrySession.jsm
+++ b/toolkit/components/telemetry/TelemetrySession.jsm
@@ -35,23 +35,16 @@ const REASON_ABORTED_SESSION = "aborted-
 const REASON_DAILY = "daily";
 const REASON_SAVED_SESSION = "saved-session";
 const REASON_GATHER_PAYLOAD = "gather-payload";
 const REASON_GATHER_SUBSESSION_PAYLOAD = "gather-subsession-payload";
 const REASON_TEST_PING = "test-ping";
 const REASON_ENVIRONMENT_CHANGE = "environment-change";
 const REASON_SHUTDOWN = "shutdown";
 
-const HISTOGRAM_SUFFIXES = {
-  PARENT: "",
-  CONTENT: "#content",
-  GPU: "#gpu",
-  EXTENSION: "#extension",
-};
-
 const ENVIRONMENT_CHANGE_LISTENER = "TelemetrySession::onEnvironmentChange";
 
 const MS_IN_ONE_HOUR  = 60 * 60 * 1000;
 const MIN_SUBSESSION_LENGTH_MS = Preferences.get("toolkit.telemetry.minSubsessionLength", 5 * 60) * 1000;
 
 const LOGGER_NAME = "Toolkit.Telemetry";
 const LOGGER_PREFIX = "TelemetrySession" + (Utils.isContentProcess ? "#content::" : "::");
 
@@ -950,41 +943,34 @@ var Impl = {
 
   getKeyedHistograms(subsession, clearSubsession) {
     let registered =
       Telemetry.registeredKeyedHistograms(this.getDatasetType(), []);
     if (this._testing == false) {
       // Omit telemetry test histograms outside of tests.
       registered = registered.filter(id => !id.startsWith("TELEMETRY_TEST_"));
     }
+
+    let khs = subsession ? Telemetry.snapshotSubsessionKeyedHistograms(clearSubsession)
+                         : Telemetry.keyedHistogramSnapshots;
     let ret = {};
 
-    for (let id of registered) {
-      for (let suffix of Object.values(HISTOGRAM_SUFFIXES)) {
-        let keyed = Telemetry.getKeyedHistogramById(id + suffix);
-        let snapshot = null;
-        if (subsession) {
-          snapshot = clearSubsession ? keyed.snapshotSubsessionAndClear()
-                                     : keyed.subsessionSnapshot();
-        } else {
-          snapshot = keyed.snapshot();
-        }
-
-        let keys = Object.keys(snapshot);
-        if (keys.length == 0) {
-          // Skip empty keyed histogram.
-          continue;
-        }
-
-        if (!(suffix in ret)) {
-          ret[suffix] = {};
-        }
-        ret[suffix][id] = {};
-        for (let key of keys) {
-          ret[suffix][id][key] = this.packHistogram(snapshot[key]);
+    for (let [process, histograms] of Object.entries(khs)) {
+      ret[process] = {};
+      for (let [name, value] of Object.entries(histograms)) {
+        if (registered.includes(name)) {
+          let keys = Object.keys(value);
+          if (keys.length == 0) {
+            // Skip empty keyed histogram
+            continue;
+          }
+          ret[process][name] = {};
+          for (let [key, hgram] of Object.entries(value)) {
+            ret[process][name][key] = this.packHistogram(hgram);
+          }
         }
       }
     }
 
     return ret;
   },
 
   /**
--- a/toolkit/components/telemetry/nsITelemetry.idl
+++ b/toolkit/components/telemetry/nsITelemetry.idl
@@ -252,23 +252,31 @@ interface nsITelemetry : nsISupports
    *   dataset() - identifies what dataset this is in: DATASET_RELEASE_CHANNEL_OPTOUT or ...OPTIN.
    *               This is intended to be only used in tests.
    */
   [implicit_jscontext]
   jsval getHistogramById(in ACString id);
 
   /*
    * An object containing a snapshot from all of the currently registered keyed histograms.
-   * { name1: {histogramData1}, name2:{histogramData2}...}
+   * { process1: {name1: {histogramData1}, name2:{histogramData2}...}}
    * where the histogramData is as described in histogramSnapshots.
    */
   [implicit_jscontext]
   readonly attribute jsval keyedHistogramSnapshots;
 
   /**
+   * Get a snapshot of the internally duplicated subsession keyed histograms.
+   * @param clear Whether to clear out the subsession histograms after snapshotting.
+   * @return An object as histogramSnapshots, except this contains the internally duplicated keyed histograms for subsession telemetry.
+   */
+  [implicit_jscontext]
+  jsval snapshotSubsessionKeyedHistograms([optional] in boolean clear);
+
+  /**
    * Returns an array whose values are the names of histograms defined
    * in Histograms.json.
    *
    * @param dataset - DATASET_RELEASE_CHANNEL_OPTOUT or ...OPTIN
    */
   void registeredKeyedHistograms(in uint32_t dataset, out uint32_t count,
                                  [retval, array, size_is(count)] out string histograms);