Bug 1338522 - Add telemetry for UI responsiveness during import of profile data from another browser. r=francois,Gijs a=jcristau
authorDão Gottwald <dao@mozilla.com>
Wed, 22 Feb 2017 12:15:24 +0100
changeset 367226 c8cefb50d5a80b546777cc712d4de17ca92154f3
parent 367225 0316bb85a29c03cdc70f74598678bce3a0998c1f
child 367227 60618c6f74d22b886b776403876e7acb6e42aeab
push id6959
push userdgottwald@mozilla.com
push dateWed, 22 Feb 2017 11:17:03 +0000
treeherdermozilla-beta@c8cefb50d5a8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfrancois, Gijs, jcristau
bugs1338522
milestone52.0
Bug 1338522 - Add telemetry for UI responsiveness during import of profile data from another browser. r=francois,Gijs a=jcristau
browser/components/migration/MigrationUtils.jsm
toolkit/components/telemetry/Histograms.json
toolkit/modules/ResponsivenessMonitor.jsm
toolkit/modules/moz.build
--- a/browser/components/migration/MigrationUtils.jsm
+++ b/browser/components/migration/MigrationUtils.jsm
@@ -23,16 +23,18 @@ XPCOMUtils.defineLazyModuleGetter(this, 
 XPCOMUtils.defineLazyModuleGetter(this, "BookmarkHTMLUtils",
                                   "resource://gre/modules/BookmarkHTMLUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "LoginHelper",
                                   "resource://gre/modules/LoginHelper.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "PlacesUtils",
                                   "resource://gre/modules/PlacesUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "PromiseUtils",
                                   "resource://gre/modules/PromiseUtils.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "ResponsivenessMonitor",
+                                  "resource://gre/modules/ResponsivenessMonitor.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Sqlite",
                                   "resource://gre/modules/Sqlite.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "TelemetryStopwatch",
                                   "resource://gre/modules/TelemetryStopwatch.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "WindowsRegistry",
                                   "resource://gre/modules/WindowsRegistry.jsm");
 
 var gMigrators = null;
@@ -207,17 +209,17 @@ this.MigratorPrototype = {
     let resources = this._getMaybeCachedResources(aProfile);
     if (!resources) {
       return [];
     }
     let types = resources.map(r => r.type);
     return types.reduce((a, b) => { a |= b; return a }, 0);
   },
 
-  getKey: function MP_getKey() {
+  getBrowserKey: function MP_getBrowserKey() {
     return this.contractID.match(/\=([^\=]+)$/)[1];
   },
 
   /**
    * DO NOT OVERRIDE - After deCOMing migration, the UI will just call
    * migrate for each resource.
    *
    * @see nsIBrowserProfileMigrator
@@ -226,56 +228,80 @@ this.MigratorPrototype = {
     let resources = this._getMaybeCachedResources(aProfile);
     if (resources.length == 0)
       throw new Error("migrate called for a non-existent source");
 
     if (aItems != Ci.nsIBrowserProfileMigrator.ALL)
       resources = resources.filter(r => aItems & r.type);
 
     // Used to periodically give back control to the main-thread loop.
-    let unblockMainThread = function () {
+    let unblockMainThread = function() {
       return new Promise(resolve => {
         Services.tm.mainThread.dispatch(resolve, Ci.nsIThread.DISPATCH_NORMAL);
       });
     };
 
-    let getHistogramForResourceType = resourceType => {
+    let getHistogramIdForResourceType = (resourceType, template) => {
       if (resourceType == MigrationUtils.resourceTypes.HISTORY) {
-        return "FX_MIGRATION_HISTORY_IMPORT_MS";
+        return template.replace("*", "HISTORY");
       }
       if (resourceType == MigrationUtils.resourceTypes.BOOKMARKS) {
-        return "FX_MIGRATION_BOOKMARKS_IMPORT_MS";
+        return template.replace("*", "BOOKMARKS");
       }
       if (resourceType == MigrationUtils.resourceTypes.PASSWORDS) {
-        return "FX_MIGRATION_LOGINS_IMPORT_MS";
+        return template.replace("*", "LOGINS");
       }
       return null;
     };
-    let maybeStartTelemetryStopwatch = (resourceType) => {
-      let histogram = getHistogramForResourceType(resourceType);
-      if (histogram) {
-        TelemetryStopwatch.startKeyed(histogram, this.getKey());
+
+    let browserKey = this.getBrowserKey();
+
+    let maybeStartTelemetryStopwatch = resourceType => {
+      let histogramId = getHistogramIdForResourceType(resourceType, "FX_MIGRATION_*_IMPORT_MS");
+      if (histogramId) {
+        TelemetryStopwatch.startKeyed(histogramId, browserKey);
       }
+      return histogramId;
     };
-    let maybeStopTelemetryStopwatch = (resourceType) => {
-      let histogram = getHistogramForResourceType(resourceType);
-      if (histogram) {
-        TelemetryStopwatch.finishKeyed(histogram, this.getKey());
+
+    let maybeStartResponsivenessMonitor = resourceType => {
+      let responsivenessMonitor;
+      let responsivenessHistogramId =
+        getHistogramIdForResourceType(resourceType, "FX_MIGRATION_*_JANK_MS");
+      if (responsivenessHistogramId) {
+        responsivenessMonitor = new ResponsivenessMonitor();
+      }
+      return {responsivenessMonitor, responsivenessHistogramId};
+    };
+
+    let maybeFinishResponsivenessMonitor = (responsivenessMonitor, histogramId) => {
+      if (responsivenessMonitor) {
+        let accumulatedDelay = responsivenessMonitor.finish();
+        if (histogramId) {
+          try {
+            Services.telemetry.getKeyedHistogramById(histogramId)
+                    .add(browserKey, accumulatedDelay);
+          } catch (ex) {
+            Cu.reportError(histogramId + ": " + ex);
+          }
+        }
       }
     };
 
     let collectQuantityTelemetry = () => {
-      try {
-        for (let resourceType of Object.keys(MigrationUtils._importQuantities)) {
-          let histogramId =
-            "FX_MIGRATION_" + resourceType.toUpperCase() + "_QUANTITY";
-          let histogram = Services.telemetry.getKeyedHistogramById(histogramId);
-          histogram.add(this.getKey(), MigrationUtils._importQuantities[resourceType]);
+      for (let resourceType of Object.keys(MigrationUtils._importQuantities)) {
+        let histogramId =
+          "FX_MIGRATION_" + resourceType.toUpperCase() + "_QUANTITY";
+        try {
+          Services.telemetry.getKeyedHistogramById(histogramId)
+                  .add(browserKey, MigrationUtils._importQuantities[resourceType]);
+        } catch (ex) {
+          Cu.reportError(histogramId + ": " + ex);
         }
-      } catch (ex) { /* Telemetry is exception-happy */ }
+      }
     };
 
     // Called either directly or through the bookmarks import callback.
     let doMigrate = Task.async(function*() {
       let resourcesGroupedByItems = new Map();
       resources.forEach(function(resource) {
         if (!resourcesGroupedByItems.has(resource.type)) {
           resourcesGroupedByItems.set(resource.type, new Set());
@@ -289,54 +315,55 @@ this.MigratorPrototype = {
       let notify = function(aMsg, aItemType) {
         Services.obs.notifyObservers(null, aMsg, aItemType);
       };
 
       for (let resourceType of Object.keys(MigrationUtils._importQuantities)) {
         MigrationUtils._importQuantities[resourceType] = 0;
       }
       notify("Migration:Started");
-      for (let [key, value] of resourcesGroupedByItems) {
-        // Workaround bug 449811.
-        let migrationType = key, itemResources = value;
-
+      for (let [migrationType, itemResources] of resourcesGroupedByItems) {
         notify("Migration:ItemBeforeMigrate", migrationType);
 
-        maybeStartTelemetryStopwatch(migrationType);
+        let stopwatchHistogramId = maybeStartTelemetryStopwatch(migrationType);
+
+        let {responsivenessMonitor, responsivenessHistogramId} =
+          maybeStartResponsivenessMonitor(migrationType);
 
         let itemSuccess = false;
         for (let res of itemResources) {
-          // Workaround bug 449811.
-          let resource = res;
           let completeDeferred = PromiseUtils.defer();
           let resourceDone = function(aSuccess) {
-            itemResources.delete(resource);
+            itemResources.delete(res);
             itemSuccess |= aSuccess;
             if (itemResources.size == 0) {
               notify(itemSuccess ?
                      "Migration:ItemAfterMigrate" : "Migration:ItemError",
                      migrationType);
               resourcesGroupedByItems.delete(migrationType);
 
-              maybeStopTelemetryStopwatch(migrationType);
+              if (stopwatchHistogramId) {
+                TelemetryStopwatch.finishKeyed(stopwatchHistogramId, browserKey);
+              }
+
+              maybeFinishResponsivenessMonitor(responsivenessMonitor, responsivenessHistogramId);
 
               if (resourcesGroupedByItems.size == 0) {
                 collectQuantityTelemetry();
                 notify("Migration:Ended");
               }
             }
             completeDeferred.resolve();
           };
 
           // If migrate throws, an error occurred, and the callback
           // (itemMayBeDone) might haven't been called.
           try {
-            resource.migrate(resourceDone);
-          }
-          catch (ex) {
+            res.migrate(resourceDone);
+          } catch (ex) {
             Cu.reportError(ex);
             resourceDone(false);
           }
 
           // Certain resources must be ran sequentially or they could fail,
           // for example bookmarks and history (See bug 1272652).
           if (migrationType == MigrationUtils.resourceTypes.BOOKMARKS ||
               migrationType == MigrationUtils.resourceTypes.HISTORY) {
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -5151,16 +5151,49 @@
     "expires_in_version": "54",
     "kind": "exponential",
     "n_buckets": 70,
     "high": 100000,
     "releaseChannelCollection": "opt-out",
     "keyed": true,
     "description": "How long it took to import logins (passwords) from another browser, keyed by the name of the browser."
   },
+  "FX_MIGRATION_BOOKMARKS_JANK_MS": {
+    "bug_numbers": [1338522],
+    "alert_emails": ["dao@mozilla.com"],
+    "expires_in_version": "58",
+    "kind": "exponential",
+    "n_buckets": 20,
+    "high": 60000,
+    "releaseChannelCollection": "opt-out",
+    "keyed": true,
+    "description": "Accumulated timer delay (variance between when the timer was expected to fire and when it actually fired) in milliseconds as an indicator for decreased main-thread responsiveness while importing bookmarks from another browser, keyed by the name of the browser (see gAvailableMigratorKeys in MigrationUtils.jsm). The import is happening on a background thread and should ideally not affect the UI noticeably."
+  },
+  "FX_MIGRATION_HISTORY_JANK_MS": {
+    "bug_numbers": [1338522],
+    "alert_emails": ["dao@mozilla.com"],
+    "expires_in_version": "58",
+    "kind": "exponential",
+    "n_buckets": 20,
+    "high": 60000,
+    "releaseChannelCollection": "opt-out",
+    "keyed": true,
+    "description": "Accumulated timer delay (variance between when the timer was expected to fire and when it actually fired) in milliseconds as an indicator for decreased main-thread responsiveness while importing history from another browser, keyed by the name of the browser (see gAvailableMigratorKeys in MigrationUtils.jsm). The import is happening on a background thread and should ideally not affect the UI noticeably."
+  },
+  "FX_MIGRATION_LOGINS_JANK_MS": {
+    "bug_numbers": [1338522],
+    "alert_emails": ["dao@mozilla.com"],
+    "expires_in_version": "58",
+    "kind": "exponential",
+    "n_buckets": 20,
+    "high": 60000,
+    "releaseChannelCollection": "opt-out",
+    "keyed": true,
+    "description": "Accumulated timer delay (variance between when the timer was expected to fire and when it actually fired) in milliseconds as an indicator for decreased main-thread responsiveness while importing logins / passwords from another browser, keyed by the name of the browser (see gAvailableMigratorKeys in MigrationUtils.jsm). The import is happening on a background thread and should ideally not affect the UI noticeably."
+  },
   "FX_MIGRATION_BOOKMARKS_QUANTITY": {
     "bug_numbers": [1279501],
     "alert_emails": ["gijs@mozilla.com"],
     "expires_in_version": "56",
     "kind": "exponential",
     "n_buckets": 20,
     "high": 1000,
     "releaseChannelCollection": "opt-out",
new file mode 100644
--- /dev/null
+++ b/toolkit/modules/ResponsivenessMonitor.jsm
@@ -0,0 +1,37 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+"use strict";
+
+this.EXPORTED_SYMBOLS = ["ResponsivenessMonitor"];
+
+const { classes: Cc, interfaces: Ci } = Components;
+
+function ResponsivenessMonitor(intervalMS = 100) {
+  this._intervalMS = intervalMS;
+  this._prevTimestamp = Date.now();
+  this._accumulatedDelay = 0;
+  this._timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
+  this._timer.initWithCallback(this, this._intervalMS, Ci.nsITimer.TYPE_REPEATING_SLACK);
+}
+
+ResponsivenessMonitor.prototype = {
+  notify() {
+    let now = Date.now();
+    this._accumulatedDelay += Math.max(0, now - this._prevTimestamp - this._intervalMS);
+    this._prevTimestamp = now;
+  },
+
+  abort() {
+    if (this._timer) {
+      this._timer.cancel();
+      this._timer = null;
+    }
+  },
+
+  finish() {
+    this.abort();
+    return this._accumulatedDelay;
+  },
+};
--- a/toolkit/modules/moz.build
+++ b/toolkit/modules/moz.build
@@ -73,16 +73,17 @@ EXTRA_JS_MODULES += [
     'PromiseUtils.jsm',
     'PropertyListUtils.jsm',
     'RemoteController.jsm',
     'RemoteFinder.jsm',
     'RemotePageManager.jsm',
     'RemoteSecurityUI.jsm',
     'RemoteWebProgress.jsm',
     'ResetProfile.jsm',
+    'ResponsivenessMonitor.jsm',
     'secondscreen/PresentationApp.jsm',
     'secondscreen/RokuApp.jsm',
     'secondscreen/SimpleServiceDiscovery.jsm',
     'SelectContentHelper.jsm',
     'SelectParentHelper.jsm',
     'ServiceRequest.jsm',
     'Services.jsm',
     'SessionRecorder.jsm',