Bug 1200169 - Making the slow add-on watcher more tolerant;r=Felipe
☠☠ backed out by dd6d447fc6e0 ☠ ☠
authorDavid Rajchenbach-Teller <dteller@mozilla.com>
Tue, 15 Dec 2015 11:21:37 +0100
changeset 318967 b94d7e7648e1d7406809b968eb1310951d461205
parent 318966 9115adb58910737dcbdca45607b2205aefde2056
child 318968 3fbbaedcf49587e25e5e4a3c19fbb3093abb9bf3
push id8951
push userbenj@benj.me
push dateTue, 05 Jan 2016 13:08:54 +0000
reviewersFelipe
bugs1200169
milestone46.0a1
Bug 1200169 - Making the slow add-on watcher more tolerant;r=Felipe Showing alerts more than once is annoying for the user and basically useless. We therefore change a bit our strategy: - if an add-on has behaved correctly for the last 5 minutes, reset our counter of offences; - don't display alerts for an add-on more than once per 6 hours. The only exception is if the add-on freezes the browser (i.e. causes it to stop for more than 5 seconds at a time), in which case we display the alert regardless of past offences, up to once per 10 minutes.
toolkit/components/perfmonitoring/AddonWatcher.jsm
--- a/toolkit/components/perfmonitoring/AddonWatcher.jsm
+++ b/toolkit/components/perfmonitoring/AddonWatcher.jsm
@@ -120,34 +120,45 @@ this.AddonWatcher = {
       }
 
       let now = Cu.now();
       if (now - this._initializedTimeStamp < this._warmupPeriod) {
         // Heuristic: do not report slowdowns during or just after startup.
         return;
       }
 
-      let jankThreshold = Preferences.get("browser.addon-watch.jank-threshold-micros", /* 128 ms == 7 frames*/ 128000);
-      let occurrencesBetweenAlerts = Preferences.get("browser.addon-watch.occurrences-between-alerts", 3);
-      let delayBetweenAlerts = Preferences.get("browser.addon-watch.delay-between-alerts-ms", .5 * 3600 * 1000 /* 1/2h */);
-      let prescriptionDelay = Preferences.get("browser.addon-watch.prescription-delay", 2 * 3600 * 1000 /* 2 hours */);
-      let highestNumberOfAddonsToReport = Preferences.get("browser.addon-watch.max-simultaneous-reports", 1);
-
-      addons = addons.filter(x => x.details.highestJank >= jankThreshold).
-        sort((a, b) => a.details.highestJank - b.details.highestJank);
-
       // Report immediately to Telemetry, regardless of whether we report to
       // the user.
       for (let {source: {addonId}, details} of addons) {
         Telemetry.getKeyedHistogramById("PERF_MONITORING_SLOW_ADDON_JANK_US").
           add(addonId, details.highestJank);
         Telemetry.getKeyedHistogramById("PERF_MONITORING_SLOW_ADDON_CPOW_US").
           add(addonId, details.highestCPOW);
       }
 
+      // We expect that users don't care about real-time alerts unless their
+      // browser is going very, very slowly. Therefore, we use the following
+      // heuristic:
+      // - if jank is above freezeThreshold (e.g. 5 seconds), report immediately; otherwise
+      // - if jank is below jankThreshold (e.g. 128ms), disregard; otherwise
+      // - if the latest jank was more than prescriptionDelay (e.g. 5 minutes) ago, reset number of occurrences;
+      // - if we have had fewer than occurrencesBetweenAlerts janks (e.g. 3) since last alert, disregard; otherwise
+      // - if we have displayed an alert for this add-on less than delayBetweenAlerts ago (e.g. 6h), disregard; otherwise
+      // - also, don't report more than highestNumberOfAddonsToReport (e.g. 1) at once.
+      let freezeThreshold = Preferences.get("browser.addon-watch.freeze-threshold-micros", /* 5 seconds */ 5000000);
+      let jankThreshold = Preferences.get("browser.addon-watch.jank-threshold-micros", /* 256 ms == 8 frames*/ 256000);
+      let occurrencesBetweenAlerts = Preferences.get("browser.addon-watch.occurrences-between-alerts", 3);
+      let delayBetweenAlerts = Preferences.get("browser.addon-watch.delay-between-alerts-ms", 6 * 3600 * 1000 /* 6h */);
+      let delayBetweenFreeeAlerts = Preferences.get("browser.addon-watch.delay-between-freeze-alerts-ms", 2 * 60 * 1000 /* 2 min */);
+      let prescriptionDelay = Preferences.get("browser.addon-watch.prescription-delay", 5 * 60 * 1000 /* 5 minutes */);
+      let highestNumberOfAddonsToReport = Preferences.get("browser.addon-watch.max-simultaneous-reports", 1);
+
+      addons = addons.filter(x => x.details.highestJank >= jankThreshold).
+        sort((a, b) => a.details.highestJank - b.details.highestJank);
+
       for (let {source: {addonId}, details} of addons) {
         if (highestNumberOfAddonsToReport <= 0) {
           return;
         }
         if (this._ignoreList.has(addonId)) {
           // Add-on is ignored.
           continue;
         }
@@ -156,25 +167,35 @@ this.AddonWatcher = {
         if (now - alerts.latestOccurrence >= prescriptionDelay) {
           // While this add-on has already caused slownesss, this
           // was a long time ago, let's forgive.
           alerts.occurrencesSinceLastNotification = 0;
         }
 
         alerts.occurrencesSinceLastNotification++;
         alerts.occurrences++;
-        if (alerts.occurrencesSinceLastNotification <= occurrencesBetweenAlerts) {
-          // While the add-on has caused jank at least once, we are only
-          // interested in repeat offenders. Store the data for future use.
-          continue;
-        }
-        if (now - alerts.latestNotificationTimeStamp <= delayBetweenAlerts) {
-          // We have already displayed an alert for this add-on recently.
-          // Wait a little before displaying another one.
-          continue;
+
+        if (details.highestJank < freezeThreshold) {
+          if (alerts.occurrencesSinceLastNotification <= occurrencesBetweenAlerts) {
+            // While the add-on has caused jank at least once, we are only
+            // interested in repeat offenders. Store the data for future use.
+            continue;
+          }
+          if (now - alerts.latestNotificationTimeStamp <= delayBetweenAlerts) {
+            // We have already displayed an alert for this add-on recently.
+            // Wait a little before displaying another one.
+            continue;
+          }
+        } else {
+          // Even in case of freeze, we want to avoid needlessly spamming the user.
+          if (now - alerts.latestNotificationTimeStamp <= delayBetweenFreezeAlerts) {
+            // We have already displayed an alert for this add-on recently.
+            // Wait a little before displaying another one.
+            continue;
+          }
         }
 
         // Ok, time to inform the user.
         alerts.latestNotificationTimeStamp = now;
         alerts.occurrencesSinceLastNotification = 0;
         try {
           this._callback(addonId);
         } catch (ex) {