bug 1366294 - Part 7 - Use keyed histogram snaphots to build payloads. r=gfritzsche
☠☠ backed out by 1f566354cea1 ☠ ☠
authorChris H-C <chutten@mozilla.com>
Fri, 23 Jun 2017 16:38:21 -0400
changeset 418916 ccf043ec6160e90aeded92607d64451445c701fa
parent 418915 fb911884876f97c01889b68c14e6be40af0f6939
child 418917 43fb00c0f96d9df2cac763744eed061298763a57
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgfritzsche
bugs1366294
milestone56.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 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);