Bug 1442353 - Reuse timeoutPromise in Sqlite.jsm r=florian
authorThom Chiovoloni <tchiovoloni@mozilla.com>
Thu, 01 Mar 2018 10:43:07 -0800
changeset 461262 a2f94b5f23a0b7b17d00886f814a3b3b287ebf20
parent 461261 18bbbdc02213d1ad5937dc06c90719cdd7de0314
child 461263 9ed7f3c8ad84c1759efef362e8862cc6259bb4fd
push id9165
push userasasaki@mozilla.com
push dateThu, 26 Apr 2018 21:04:54 +0000
treeherdermozilla-beta@064c3804de2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersflorian
bugs1442353
milestone60.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 1442353 - Reuse timeoutPromise in Sqlite.jsm r=florian MozReview-Commit-ID: 6AlvYliZcmy
toolkit/modules/Sqlite.jsm
--- a/toolkit/modules/Sqlite.jsm
+++ b/toolkit/modules/Sqlite.jsm
@@ -3,18 +3,20 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 var EXPORTED_SYMBOLS = [
   "Sqlite",
 ];
 
-// The time to wait before considering a transaction stuck and rejecting it.
-const TRANSACTIONS_QUEUE_TIMEOUT_MS = 240000; // 4 minutes
+// The maximum time to wait before considering a transaction stuck and rejecting
+// it. (Note that the minimum amount of time we wait is 20% less than this, see
+// the `_getTimeoutPromise` method on `ConnectionData` for details).
+const TRANSACTIONS_QUEUE_TIMEOUT_MS = 300000; // 5 minutes
 
 ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
 ChromeUtils.import("resource://gre/modules/Timer.jsm");
 
 XPCOMUtils.defineLazyModuleGetters(this, {
   AsyncShutdown: "resource://gre/modules/AsyncShutdown.jsm",
   Services: "resource://gre/modules/Services.jsm",
   OS: "resource://gre/modules/osfile.jsm",
@@ -260,16 +262,24 @@ function ConnectionData(connection, iden
       identifier: this._identifier,
       isCloseRequested: this._closeRequested,
       hasDbConn: !!this._dbConn,
       hasInProgressTransaction: this._hasInProgressTransaction,
       pendingStatements: this._pendingStatements.size,
       statementCounter: this._statementCounter,
     })
   );
+
+  // We avoid creating a timer every transaction that exists solely as a safety
+  // check (e.g. one that never should fire) by reusing it if it's sufficiently
+  // close to when the previous promise was created (see bug 1442353 and
+  // `_getTimeoutPromise` for more info).
+  this._timeoutPromise = null;
+  // The last timestamp when we should consider using `this._timeoutPromise`.
+  this._timeoutPromiseExpires = 0;
 }
 
 /**
  * Map of connection identifiers to ConnectionData objects
  *
  * The connection identifier is a human-readable name of the
  * database. Used by finalization witnesses to be able to close opened
  * connections on garbage collection.
@@ -637,20 +647,17 @@ ConnectionData.prototype = Object.freeze
           this._hasInProgressTransaction = false;
         }
       })();
 
       // If a transaction yields on a never resolved promise, or is mistakenly
       // nested, it could hang the transactions queue forever.  Thus we timeout
       // the execution after a meaningful amount of time, to ensure in any case
       // we'll proceed after a while.
-      let timeoutPromise = new Promise((resolve, reject) => {
-        setTimeout(() => reject(new Error("Transaction timeout, most likely caused by unresolved pending work.")),
-                   TRANSACTIONS_QUEUE_TIMEOUT_MS);
-      });
+      let timeoutPromise = this._getTimeoutPromise();
       return Promise.race([transactionPromise, timeoutPromise]);
     });
     // Atomically update the queue before anyone else has a chance to enqueue
     // further transactions.
     this._transactionQueue = promise.catch(ex => { console.error(ex); });
 
     // Make sure that we do not shutdown the connection during a transaction.
     this._barrier.client.addBlocker(`Transaction (${this._getOperationId()})`,
@@ -846,16 +853,38 @@ ConnectionData.prototype = Object.freeze
   _startIdleShrinkTimer() {
     if (!this._idleShrinkTimer) {
       return;
     }
 
     this._idleShrinkTimer.initWithCallback(this.shrinkMemory.bind(this),
                                            this._idleShrinkMS,
                                            this._idleShrinkTimer.TYPE_ONE_SHOT);
+  },
+
+  // Returns a promise that will resolve after a time comprised between 80% of
+  // `TRANSACTIONS_QUEUE_TIMEOUT_MS` and `TRANSACTIONS_QUEUE_TIMEOUT_MS`. Use
+  // this promise instead of creating several individual timers to reduce the
+  // overhead due to timers (see bug 1442353).
+  _getTimeoutPromise() {
+    if (this._timeoutPromise && Cu.now() <= this._timeoutPromiseExpires) {
+      return this._timeoutPromise;
+    }
+    let timeoutPromise = new Promise((resolve, reject) => {
+      setTimeout(() => {
+        // Clear out this._timeoutPromise if it hasn't changed since we set it.
+        if (this._timeoutPromise == timeoutPromise) {
+          this._timeoutPromise = null;
+        }
+        reject(new Error("Transaction timeout, most likely caused by unresolved pending work."));
+      }, TRANSACTIONS_QUEUE_TIMEOUT_MS);
+    });
+    this._timeoutPromise = timeoutPromise;
+    this._timeoutPromiseExpires = Cu.now() + TRANSACTIONS_QUEUE_TIMEOUT_MS * 0.2;
+    return this._timeoutPromise;
   }
 });
 
 /**
  * Opens a connection to a SQLite database.
  *
  * The following parameters can control the connection:
  *