perf: make network quiet threshold configurable per pass (#2220)

* perf: make network quiet threshold configurable per pass

* lower it more

* split into 3 thresholds

* add comments

lighthouse-core/config/default.js

@@ -4,7 +4,9 @@ module.exports = {
   "passes": [{
     "passName": "defaultPass",
     "recordTrace": true,
-    "pauseBeforeTraceEndMs": 5000,
+    "pauseAfterLoadMs": 5000,
+    "networkQuietThresholdMs": 5000,
+    "pauseAfterNetworkQuietMs": 2500,
     "useThrottling": true,
     "gatherers": [
       "url",
@@ -29,6 +31,8 @@ module.exports = {
   {
     "passName": "offlinePass",
     "useThrottling": false,
+    // Just wait for onload
+    "networkQuietThresholdMs": 0,
     "gatherers": [
       "service-worker",
       "offline",
@@ -38,6 +42,8 @@ module.exports = {
   {
     "passName": "redirectPass",
     "useThrottling": false,
+    // Just wait for onload
+    "networkQuietThresholdMs": 0,
     // Speed up the redirect pass by blocking stylesheets, fonts, and images
     "blockedUrlPatterns": ["*.css", "*.jpg", "*.jpeg", "*.png", "*.gif", "*.svg", "*.ttf", "*.woff", "*.woff2"],
     "gatherers": [

lighthouse-core/gather/driver.js

@@ -25,13 +25,18 @@ const URL = require('../lib/url-shim');
 const log = require('../lib/log.js');
 const DevtoolsLog = require('./devtools-log');
 
-const PAUSE_AFTER_LOAD = 5000;
+// Controls how long to wait after onLoad before continuing
+const DEFAULT_PAUSE_AFTER_LOAD = 0;
+// Controls how long to wait between network requests before determining the network is quiet
+const DEFAULT_NETWORK_QUIET_THRESHOLD = 5000;
+// Controls how long to wait after network quiet before continuing
+const DEFAULT_PAUSE_AFTER_NETWORK_QUIET = 0;
 
 const _uniq = arr => Array.from(new Set(arr));
 
 class Driver {
   static get MAX_WAIT_FOR_FULLY_LOADED() {
-    return 25 * 1000;
+    return 30 * 1000;
   }
 
   /**
@@ -378,42 +383,46 @@ class Driver {
 
   /**
    * Returns a promise that resolves when the network has been idle for
-   * `pauseAfterLoadMs` ms and a method to cancel internal network listeners and
+   * `networkQuietThresholdMs` ms and a method to cancel internal network listeners and
    * timeout.
-   * @param {string} pauseAfterLoadMs
+   * @param {number} networkQuietThresholdMs
+   * @param {number} pauseAfterNetworkQuietMs
    * @return {{promise: !Promise, cancel: function()}}
    * @private
    */
-  _waitForNetworkIdle(pauseAfterLoadMs) {
+  _waitForNetworkIdle(networkQuietThresholdMs, pauseAfterNetworkQuietMs) {
     let idleTimeout;
     let cancel;
 
     const promise = new Promise((resolve, reject) => {
       const onIdle = () => {
         // eslint-disable-next-line no-use-before-define
-        this._networkStatusMonitor.once('networkbusy', onBusy);
+        this._networkStatusMonitor.once('network-2-busy', onBusy);
         idleTimeout = setTimeout(_ => {
           cancel();
           resolve();
-        }, pauseAfterLoadMs);
+        }, networkQuietThresholdMs);
       };
 
       const onBusy = () => {
-        this._networkStatusMonitor.once('networkidle', onIdle);
+        this._networkStatusMonitor.once('network-2-idle', onIdle);
         clearTimeout(idleTimeout);
       };
 
       cancel = () => {
         clearTimeout(idleTimeout);
-        this._networkStatusMonitor.removeListener('networkbusy', onBusy);
-        this._networkStatusMonitor.removeListener('networkidle', onIdle);
+        this._networkStatusMonitor.removeListener('network-2-busy', onBusy);
+        this._networkStatusMonitor.removeListener('network-2-idle', onIdle);
       };
 
-      if (this._networkStatusMonitor.isIdle()) {
+      if (this._networkStatusMonitor.is2Idle()) {
         onIdle();
       } else {
         onBusy();
       }
+    }).then(() => {
+      // Once idle has been determined wait another pauseAfterLoadMs
+      return new Promise(resolve => setTimeout(resolve, pauseAfterNetworkQuietMs));
     });
 
     return {
@@ -452,29 +461,34 @@ class Driver {
 
   /**
    * Returns a promise that resolves when:
-   * - it's been pauseAfterLoadMs milliseconds after both onload and the network
+   * - it's been networkQuietThresholdMs milliseconds after both onload and the network
    * has gone idle, or
    * - maxWaitForLoadedMs milliseconds have passed.
    * See https://github.com/GoogleChrome/lighthouse/issues/627 for more.
    * @param {number} pauseAfterLoadMs
+   * @param {number} networkQuietThresholdMs
+   * @param {number} pauseAfterNetworkQuietMs
    * @param {number} maxWaitForLoadedMs
    * @return {!Promise}
    * @private
    */
-  _waitForFullyLoaded(pauseAfterLoadMs, maxWaitForLoadedMs) {
+  _waitForFullyLoaded(pauseAfterLoadMs, networkQuietThresholdMs, pauseAfterNetworkQuietMs,
+      maxWaitForLoadedMs) {
     let maxTimeoutHandle;
 
     // Listener for onload. Resolves pauseAfterLoadMs ms after load.
     const waitForLoadEvent = this._waitForLoadEvent(pauseAfterLoadMs);
-    // Network listener. Resolves when the network has been idle for pauseAfterLoadMs.
-    const waitForNetworkIdle = this._waitForNetworkIdle(pauseAfterLoadMs);
+    // Network listener. Resolves pauseAfterNetworkQuietMs after when the network has been idle for
+    // networkQuietThresholdMs.
+    const waitForNetworkIdle = this._waitForNetworkIdle(networkQuietThresholdMs,
+        pauseAfterNetworkQuietMs);
 
     // Wait for both load promises. Resolves on cleanup function the clears load
     // timeout timer.
     const loadPromise = Promise.all([
       waitForLoadEvent.promise,
       waitForNetworkIdle.promise
-    ]).then(_ => {
+    ]).then(() => {
       return function() {
         log.verbose('Driver', 'loadEventFired and network considered idle');
         clearTimeout(maxTimeoutHandle);
@@ -554,15 +568,25 @@ class Driver {
   gotoURL(url, options = {}) {
     const waitForLoad = options.waitForLoad || false;
     const disableJS = options.disableJavaScript || false;
-    const pauseAfterLoadMs = (options.flags && options.flags.pauseAfterLoad) || PAUSE_AFTER_LOAD;
-    const maxWaitMs = (options.flags && options.flags.maxWaitForLoad) ||
-        Driver.MAX_WAIT_FOR_FULLY_LOADED;
+
+    let pauseAfterLoadMs = options.config && options.config.pauseAfterLoadMs;
+    let networkQuietThresholdMs = options.config && options.config.networkQuietThresholdMs;
+    let pauseAfterNetworkQuietMs = options.config && options.config.pauseAfterNetworkQuietMs;
+    let maxWaitMs = options.flags && options.flags.maxWaitForLoad;
+
+    /* eslint-disable max-len */
+    if (typeof pauseAfterLoadMs !== 'number') pauseAfterLoadMs = DEFAULT_PAUSE_AFTER_LOAD;
+    if (typeof networkQuietThresholdMs !== 'number') networkQuietThresholdMs = DEFAULT_NETWORK_QUIET_THRESHOLD;
+    if (typeof pauseAfterNetworkQuietMs !== 'number') pauseAfterNetworkQuietMs = DEFAULT_PAUSE_AFTER_NETWORK_QUIET;
+    if (typeof maxWaitMs !== 'number') maxWaitMs = Driver.MAX_WAIT_FOR_FULLY_LOADED;
+    /* eslint-enable max-len */
 
     return this._beginNetworkStatusMonitoring(url)
       .then(_ => this.sendCommand('Page.enable'))
       .then(_ => this.sendCommand('Emulation.setScriptExecutionDisabled', {value: disableJS}))
       .then(_ => this.sendCommand('Page.navigate', {url}))
-      .then(_ => waitForLoad && this._waitForFullyLoaded(pauseAfterLoadMs, maxWaitMs))
+      .then(_ => waitForLoad && this._waitForFullyLoaded(pauseAfterLoadMs,
+          networkQuietThresholdMs, pauseAfterNetworkQuietMs, maxWaitMs))
       .then(_ => this._endNetworkStatusMonitoring());
   }
 
@@ -689,20 +713,16 @@ class Driver {
       .then(_ => this.sendCommand('Tracing.start', tracingOpts));
   }
 
-  /**
-   * @param {number=} pauseBeforeTraceEndMs Wait this many milliseconds before ending the trace
-   */
-  endTrace(pauseBeforeTraceEndMs = 0) {
+  endTrace() {
     return new Promise((resolve, reject) => {
       // When the tracing has ended this will fire with a stream handle.
       this.once('Tracing.tracingComplete', streamHandle => {
         this._readTraceFromStream(streamHandle)
             .then(traceContents => resolve(traceContents), reject);
       });
 
-      // Issue the command to stop tracing after an optional delay.
-      // Audits like TTI may require slightly longer trace to find a minimum window size.
-      setTimeout(() => this.sendCommand('Tracing.end').catch(reject), pauseBeforeTraceEndMs);
+      // Issue the command to stop tracing.
+      return this.sendCommand('Tracing.end').catch(reject);
     });
   }
 

lighthouse-core/gather/gather-runner.js

@@ -92,6 +92,7 @@ class GatherRunner {
       waitForLoad: true,
       disableJavaScript: !!options.disableJavaScript,
       flags: options.flags,
+      config: options.config,
     }).then(finalUrl => {
       options.url = finalUrl;
     });
@@ -248,7 +249,7 @@ class GatherRunner {
     if (config.recordTrace) {
       pass = pass.then(_ => {
         log.log('status', 'Retrieving trace');
-        return driver.endTrace(config.pauseBeforeTraceEndMs);
+        return driver.endTrace();
       }).then(traceContents => {
         // Before Chrome 54.0.2816 (codereview.chromium.org/2161583004),
         // traceContents was an array of trace events; after, traceContents is

lighthouse-core/lib/network-recorder.js

@@ -52,6 +52,10 @@ class NetworkRecorder extends EventEmitter {
     return this.activeRequestCount() === 0;
   }
 
+  is2Idle() {
+    return this.activeRequestCount() <= 2;
+  }
+
   /**
    * Listener for the NetworkManager's RequestStarted event, which includes both
    * web socket and normal request creation.
@@ -70,6 +74,11 @@ class NetworkRecorder extends EventEmitter {
     if (activeCount === 1) {
       this.emit('networkbusy');
     }
+
+    // If exactly three requests in progress, we've transitioned from 2-idle to 2-busy.
+    if (activeCount === 3) {
+      this.emit('network-2-busy');
+    }
   }
 
   /**
@@ -92,6 +101,10 @@ class NetworkRecorder extends EventEmitter {
     if (this.isIdle()) {
       this.emit('networkidle');
     }
+
+    if (this.is2Idle()) {
+      this.emit('network-2-idle');
+    }
   }
 
   // There are a few differences between the debugging protocol naming and

lighthouse-core/test/gather/driver-test.js

@@ -176,8 +176,8 @@ describe('Browser Driver', () => {
 
     const loadOptions = {
       waitForLoad: true,
-      flags: {
-        pauseAfterLoad: 1
+      config: {
+        networkQuietThresholdMs: 1
       }
     };