Bug 1495181 - Chunk matches in the providers manager. r=adw
authorMarco Bonardo <mbonardo@mozilla.com>
Tue, 09 Oct 2018 16:49:04 +0000
changeset 496022 299b6e20c942209d2a1c5ffd7330ff28ad03abd4
parent 496021 190cbcbf2dd268cb48de9dd0a970edc5f36d85f4
child 496023 02e58f0b7d607d46c42b7e70876101b2b523be4c
push id9984
push userffxbld-merge
push dateMon, 15 Oct 2018 21:07:35 +0000
treeherdermozilla-beta@183d27ea8570 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersadw
bugs1495181
milestone64.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 1495181 - Chunk matches in the providers manager. r=adw Differential Revision: https://phabricator.services.mozilla.com/D7879
browser/components/urlbar/UrlbarProvidersManager.jsm
--- a/browser/components/urlbar/UrlbarProvidersManager.jsm
+++ b/browser/components/urlbar/UrlbarProvidersManager.jsm
@@ -24,16 +24,21 @@ XPCOMUtils.defineLazyGetter(this, "logge
   Log.repository.getLogger("Places.Urlbar.ProvidersManager"));
 
 // List of available local providers, each is implemented in its own jsm module
 // and will track different queries internally by queryContext.
 var localProviderModules = {
   UrlbarProviderOpenTabs: "resource:///modules/UrlbarProviderOpenTabs.jsm",
 };
 
+// To improve dataflow and reduce UI work, when a match is added by a
+// non-immediate provider, we notify it to the controller after a delay, so
+// that we can chunk matches coming in that timeframe into a single call.
+const CHUNK_MATCHES_DELAY_MS = 16;
+
 /**
  * Class used to create a manager.
  * The manager is responsible to keep a list of providers, instantiate query
  * objects and pass those to the providers.
  */
 class ProvidersManager {
   constructor() {
     // Tracks the available providers.
@@ -79,17 +84,17 @@ class ProvidersManager {
 
   /**
    * Starts querying.
    * @param {object} queryContext The query context object
    * @param {object} controller a UrlbarController instance
    */
   async startQuery(queryContext, controller) {
     logger.info(`Query start ${queryContext.searchString}`);
-    let query = Object.seal(new Query(queryContext, controller, this.providers));
+    let query = new Query(queryContext, controller, this.providers);
     this.queries.set(queryContext, query);
     await query.start();
   }
 
   /**
    * Cancels a running query.
    * @param {object} queryContext
    */
@@ -143,19 +148,16 @@ class Query {
    * @param {object} providers
    *        Map of all the providers by type and name
    */
   constructor(queryContext, controller, providers) {
     this.context = queryContext;
     this.context.results = [];
     this.controller = controller;
     this.providers = providers;
-    // Track the delay timer.
-    this.sleepResolve = Promise.resolve();
-    this.sleepTimer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
     this.started = false;
     this.canceled = false;
     this.complete = false;
   }
 
   /**
    * Starts querying.
    */
@@ -169,69 +171,149 @@ class Query {
     let promises = [];
     for (let provider of this.providers.get(UrlbarUtils.PROVIDER_TYPE.IMMEDIATE).values()) {
       if (this.canceled) {
         break;
       }
       promises.push(provider.startQuery(this.context, this.add));
     }
 
-    await new Promise(resolve => {
-      let time = UrlbarPrefs.get("delay");
-      this.sleepResolve = resolve;
-      this.sleepTimer.initWithCallback(resolve, time, Ci.nsITimer.TYPE_ONE_SHOT);
-    });
+    // Tracks the delay timer. We will fire (in this specific case, cancel would
+    // do the same, since the callback is empty) the timer when the search is
+    // canceled, unblocking start().
+    this._sleepTimer = new SkippableTimer(() => {}, UrlbarPrefs.get("delay"));
+    await this._sleepTimer.promise;
 
     for (let providerType of [UrlbarUtils.PROVIDER_TYPE.NETWORK,
                               UrlbarUtils.PROVIDER_TYPE.PROFILE,
                               UrlbarUtils.PROVIDER_TYPE.EXTENSION]) {
       for (let provider of this.providers.get(providerType).values()) {
         if (this.canceled) {
           break;
         }
         promises.push(provider.startQuery(this.context, this.add.bind(this)));
       }
     }
 
     await Promise.all(promises.map(p => p.catch(Cu.reportError)));
 
+    if (this._chunkTimer) {
+      // All the providers are done returning results, so we can stop chunking.
+      await this._chunkTimer.fire();
+    }
+
     // Nothing should be failing above, since we catch all the promises, thus
     // this is not in a finally for now.
     this.complete = true;
   }
 
   /**
    * Cancels this query.
    * @note Invoking cancel multiple times is a no-op.
    */
   cancel() {
     if (this.canceled) {
       return;
     }
     this.canceled = true;
-    this.sleepTimer.cancel();
     for (let providers of this.providers.values()) {
       for (let provider of providers.values()) {
         provider.cancelQuery(this.context);
       }
     }
-    this.sleepResolve();
+    if (this._chunkTimer) {
+      this._chunkTimer.cancel().catch(Cu.reportError);
+    }
+    if (this._sleepTimer) {
+      this._sleepTimer.fire().catch(Cu.reportError);
+    }
   }
 
   /**
    * Adds a match returned from a provider to the results set.
    * @param {object} provider
    * @param {object} match
    */
   add(provider, match) {
     // Stop returning results as soon as we've been canceled.
     if (this.canceled) {
       return;
     }
-    // TODO:
-    //  * coalesce results in timed chunks: we don't want to notify every single
-    //    result as soon as it arrives, we'll rather collect results for a few
-    //    ms, then send them
-    //  * pass results to a muxer before sending them back to the controller.
     this.context.results.push(match);
-    this.controller.receiveResults(this.context);
+
+
+    let notifyResults = () => {
+      if (this._chunkTimer) {
+        this._chunkTimer.cancel().catch(Cu.reportError);
+        delete this._chunkTimer;
+      }
+      // TODO:
+      //  * pass results to a muxer before sending them back to the controller.
+      this.controller.receiveResults(this.context);
+    };
+
+    // If the provider is not of immediate type, chunk results, to improve the
+    // dataflow and reduce UI flicker.
+    if (provider.type == UrlbarUtils.PROVIDER_TYPE.IMMEDIATE) {
+      notifyResults();
+    } else if (!this._chunkTimer) {
+      this._chunkTimer = new SkippableTimer(notifyResults, CHUNK_MATCHES_DELAY_MS);
+    }
   }
 }
+
+/**
+ * Class used to create a timer that can be manually fired, to immediately
+ * invoke the callback, or canceled, as necessary.
+ * Examples:
+ *   let timer = new SkippableTimer();
+ *   // Invokes the callback immediately without waiting for the delay.
+ *   await timer.fire();
+ *   // Cancel the timer, the callback won't be invoked.
+ *   await timer.cancel();
+ *   // Wait for the timer to have elapsed.
+ *   await timer.promise;
+ */
+class SkippableTimer {
+  /**
+   * Creates a skippable timer for the given callback and time.
+   * @param {function} callback To be invoked when requested
+   * @param {number} time A delay in milliseconds to wait for
+   */
+  constructor(callback, time) {
+    let timerPromise = new Promise(resolve => {
+      this._timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
+      this._timer.initWithCallback(() => {
+        logger.debug(`Elapsed ${time}ms timer`);
+        resolve();
+      }, time, Ci.nsITimer.TYPE_ONE_SHOT);
+      logger.debug(`Started ${time}ms timer`);
+    });
+
+    let firePromise = new Promise(resolve => {
+      this.fire = () => {
+        logger.debug(`Skipped ${time}ms timer`);
+        resolve();
+        return this.promise;
+      };
+    });
+
+    this.promise = Promise.race([timerPromise, firePromise]).then(() => {
+      // If we've been canceled, don't call back.
+      if (this._timer) {
+        callback();
+      }
+    });
+  }
+
+  /**
+   * Allows to cancel the timer and the callback won't be invoked.
+   * It is not strictly necessary to await for this, the promise can just be
+   * used to ensure all the internal work is complete.
+   * @returns {promise} Resolved once all the cancelation work is complete.
+   */
+  cancel() {
+    logger.debug(`Canceling timer for ${this._timer.delay}ms`);
+    this._timer.cancel();
+    delete this._timer;
+    return this.fire();
+  }
+}