Bug 833609 - Part 2: Add timer to shrink memory after idle; r=mak

toolkit/modules/Sqlite.jsm

@@ -8,7 +8,7 @@ this.EXPORTED_SYMBOLS = [
   "Sqlite",
 ];
 
-const {interfaces: Ci, utils: Cu} = Components;
+const {classes: Cc, interfaces: Ci, utils: Cu} = Components;
 
 Cu.import("resource://gre/modules/commonjs/promise/core.js");
 Cu.import("resource://gre/modules/osfile.jsm");
@@ -43,6 +43,13 @@ let connectionCounters = {};
  *       to obtain a lock, possibly making database access slower. Defaults to
  *       true.
  *
+ *   shrinkMemoryOnConnectionIdleMS -- (integer) If defined, the connection
+ *       will attempt to minimize its memory usage after this many
+ *       milliseconds of connection idle. The connection is idle when no
+ *       statements are executing. There is no default value which means no
+ *       automatic memory minimization will occur. Please note that this is
+ *       *not* a timer on the idle service and this could fire while the
+ *       application is active.
  *
  * FUTURE options to control:
  *
@@ -69,6 +76,18 @@ function openConnection(options) {
   let sharedMemoryCache = "sharedMemoryCache" in options ?
                             options.sharedMemoryCache : true;
 
+  let openedOptions = {};
+
+  if ("shrinkMemoryOnConnectionIdleMS" in options) {
+    if (!Number.isInteger(options.shrinkMemoryOnConnectionIdleMS)) {
+      throw new Error("shrinkMemoryOnConnectionIdleMS must be an integer. " +
+                      "Got: " + options.shrinkMemoryOnConnectionIdleMS);
+    }
+
+    openedOptions.shrinkMemoryOnConnectionIdleMS =
+      options.shrinkMemoryOnConnectionIdleMS;
+  }
+
   let file = FileUtils.File(path);
   let openDatabaseFn = sharedMemoryCache ?
                          Services.storage.openDatabase :
@@ -92,7 +111,8 @@ function openConnection(options) {
       return Promise.reject(new Error("Connection is not ready."));
     }
 
-    return Promise.resolve(new OpenedConnection(connection, basename, number));
+    return Promise.resolve(new OpenedConnection(connection, basename, number,
+                                                openedOptions));
   } catch (ex) {
     log.warn("Could not open database: " + CommonUtils.exceptionStr(ex));
     return Promise.reject(ex);
@@ -143,8 +163,11 @@ function openConnection(options) {
  *        (string) The basename of this database name. Used for logging.
  * @param number
  *        (Number) The connection number to this database.
+ * @param options
+ *        (object) Options to control behavior of connection. See
+ *        `openConnection`.
  */
-function OpenedConnection(connection, basename, number) {
+function OpenedConnection(connection, basename, number, options) {
   let log = Log4Moz.repository.getLogger("Sqlite.Connection." + basename);
 
   // getLogger() returns a shared object. We can't modify the functions on this
@@ -180,6 +203,14 @@ function OpenedConnection(connection, basename, number) {
   this._inProgressCounter = 0;
 
   this._inProgressTransaction = null;
+
+  this._idleShrinkMS = options.shrinkMemoryOnConnectionIdleMS;
+  if (this._idleShrinkMS) {
+    this._idleShrinkTimer = Cc["@mozilla.org/timer;1"]
+                              .createInstance(Ci.nsITimer);
+    // We wait for the first statement execute to start the timer because
+    // shrinking now would not do anything.
+  }
 }
 
 OpenedConnection.prototype = Object.freeze({
@@ -259,7 +290,7 @@ OpenedConnection.prototype = Object.freeze({
     }
 
     this._log.debug("Request to close connection.");
-
+    this._clearIdleShrinkTimer();
     let deferred = Promise.defer();
 
     // We need to take extra care with transactions during shutdown.
@@ -389,7 +420,27 @@ OpenedConnection.prototype = Object.freeze({
       this._cachedStatements.set(sql, statement);
     }
 
-    return this._executeStatement(sql, statement, params, onRow);
+    this._clearIdleShrinkTimer();
+
+    let deferred = Promise.defer();
+
+    try {
+      this._executeStatement(sql, statement, params, onRow).then(
+        function onResult(result) {
+          this._startIdleShrinkTimer();
+          deferred.resolve(result);
+        }.bind(this),
+        function onError(error) {
+          this._startIdleShrinkTimer();
+          deferred.reject(error);
+        }.bind(this)
+      );
+    } catch (ex) {
+      this._startIdleShrinkTimer();
+      throw ex;
+    }
+
+    return deferred.promise;
   },
 
   /**
@@ -418,22 +469,32 @@ OpenedConnection.prototype = Object.freeze({
     let index = this._anonymousCounter++;
 
     this._anonymousStatements.set(index, statement);
+    this._clearIdleShrinkTimer();
+
+    let onFinished = function () {
+      this._anonymousStatements.delete(index);
+      statement.finalize();
+      this._startIdleShrinkTimer();
+    }.bind(this);
 
     let deferred = Promise.defer();
 
-    this._executeStatement(sql, statement, params, onRow).then(
-      function onResult(rows) {
-        this._anonymousStatements.delete(index);
-        statement.finalize();
-        deferred.resolve(rows);
-      }.bind(this),
+    try {
+      this._executeStatement(sql, statement, params, onRow).then(
+        function onResult(rows) {
+          onFinished();
+          deferred.resolve(rows);
+        }.bind(this),
 
-      function onError(error) {
-        this._anonymousStatements.delete(index);
-        statement.finalize();
-        deferred.reject(error);
-      }.bind(this)
-    );
+        function onError(error) {
+          onFinished();
+          deferred.reject(error);
+        }.bind(this)
+      );
+    } catch (ex) {
+      onFinished();
+      throw ex;
+    }
 
     return deferred.promise;
   },
@@ -592,7 +653,11 @@ OpenedConnection.prototype = Object.freeze({
    * @return Promise<>
    */
   shrinkMemory: function () {
-    return this.execute("PRAGMA shrink_memory");
+    this._log.info("Shrinking memory usage.");
+
+    let onShrunk = this._clearIdleShrinkTimer.bind(this);
+
+    return this.execute("PRAGMA shrink_memory").then(onShrunk, onShrunk);
   },
 
   _executeStatement: function (sql, statement, params, onRow) {
@@ -714,6 +779,24 @@ OpenedConnection.prototype = Object.freeze({
       throw new Error("Connection is not open.");
     }
   },
+
+  _clearIdleShrinkTimer: function () {
+    if (!this._idleShrinkTimer) {
+      return;
+    }
+
+    this._idleShrinkTimer.cancel();
+  },
+
+  _startIdleShrinkTimer: function () {
+    if (!this._idleShrinkTimer) {
+      return;
+    }
+
+    this._idleShrinkTimer.initWithCallback(this.shrinkMemory.bind(this),
+                                           this._idleShrinkMS,
+                                           this._idleShrinkTimer.TYPE_ONE_SHOT);
+  },
 });
 
 this.Sqlite = {

toolkit/modules/tests/xpcshell/test_sqlite.js

@@ -3,7 +3,7 @@
 
 "use strict";
 
-const {utils: Cu} = Components;
+const {classes: Cc, interfaces: Ci, utils: Cu} = Components;
 
 do_get_profile();
 
@@ -13,19 +13,38 @@ Cu.import("resource://gre/modules/Sqlite.jsm");
 Cu.import("resource://gre/modules/Task.jsm");
 
 
-function getConnection(dbName) {
-  let path = dbName + ".sqlite";
+function sleep(ms) {
+  let deferred = Promise.defer();
 
-  return Sqlite.openConnection({path: path});
+  let timer = Cc["@mozilla.org/timer;1"]
+                .createInstance(Ci.nsITimer);
+
+  timer.initWithCallback({
+    notify: function () {
+      deferred.resolve();
+    },
+  }, ms, timer.TYPE_ONE_SHOT);
+
+  return deferred.promise;
 }
 
-function getDummyDatabase(name) {
+function getConnection(dbName, extraOptions={}) {
+  let path = dbName + ".sqlite";
+  let options = {path: path};
+  for (let [k, v] in Iterator(extraOptions)) {
+    options[k] = v;
+  }
+
+  return Sqlite.openConnection(options);
+}
+
+function getDummyDatabase(name, extraOptions={}) {
   const TABLES = {
     dirs: "id INTEGER PRIMARY KEY AUTOINCREMENT, path TEXT",
     files: "id INTEGER PRIMARY KEY AUTOINCREMENT, dir_id INTEGER, path TEXT",
   };
 
-  let c = yield getConnection(name);
+  let c = yield getConnection(name, extraOptions);
 
   for (let [k, v] in Iterator(TABLES)) {
     yield c.execute("CREATE TABLE " + k + "(" + v + ")");
@@ -161,11 +180,17 @@ add_task(function test_execute_invalid_statement() {
 
   let deferred = Promise.defer();
 
+  do_check_eq(c._anonymousStatements.size, 0);
+
   c.execute("SELECT invalid FROM unknown").then(do_throw, function onError(error) {
     deferred.resolve();
   });
 
   yield deferred.promise;
+
+  // Ensure we don't leak the statement instance.
+  do_check_eq(c._anonymousStatements.size, 0);
+
   yield c.close();
 });
 
@@ -332,3 +357,117 @@ add_task(function test_shrink_memory() {
   yield c.shrinkMemory();
   yield c.close();
 });
+
+add_task(function test_no_shrink_on_init() {
+  let c = yield getConnection("no_shrink_on_init",
+                              {shrinkMemoryOnConnectionIdleMS: 200});
+
+  let oldShrink = c.shrinkMemory;
+  let count = 0;
+  Object.defineProperty(c, "shrinkMemory", {
+    value: function () {
+      count++;
+    },
+  });
+
+  // We should not shrink until a statement has been executed.
+  yield sleep(220);
+  do_check_eq(count, 0);
+
+  yield c.execute("SELECT 1");
+  yield sleep(220);
+  do_check_eq(count, 1);
+
+  yield c.close();
+});
+
+add_task(function test_idle_shrink_fires() {
+  let c = yield getDummyDatabase("idle_shrink_fires",
+                                 {shrinkMemoryOnConnectionIdleMS: 200});
+  c._clearIdleShrinkTimer();
+
+  let oldShrink = c.shrinkMemory;
+  let shrinkPromises = [];
+
+  let count = 0;
+  Object.defineProperty(c, "shrinkMemory", {
+    value: function () {
+      count++;
+      let promise = oldShrink.call(c);
+      shrinkPromises.push(promise);
+      return promise;
+    },
+  });
+
+  // We reset the idle shrink timer after monkeypatching because otherwise the
+  // installed timer callback will reference the non-monkeypatched function.
+  c._startIdleShrinkTimer();
+
+  yield sleep(220);
+  do_check_eq(count, 1);
+  do_check_eq(shrinkPromises.length, 1);
+  yield shrinkPromises[0];
+  shrinkPromises.shift();
+
+  // We shouldn't shrink again unless a statement was executed.
+  yield sleep(300);
+  do_check_eq(count, 1);
+
+  yield c.execute("SELECT 1");
+  yield sleep(300);
+
+  do_check_eq(count, 2);
+  do_check_eq(shrinkPromises.length, 1);
+  yield shrinkPromises[0];
+
+  yield c.close();
+});
+
+add_task(function test_idle_shrink_reset_on_operation() {
+  const INTERVAL = 500;
+  let c = yield getDummyDatabase("idle_shrink_reset_on_operation",
+                                 {shrinkMemoryOnConnectionIdleMS: INTERVAL});
+
+  c._clearIdleShrinkTimer();
+
+  let oldShrink = c.shrinkMemory;
+  let shrinkPromises = [];
+  let count = 0;
+
+  Object.defineProperty(c, "shrinkMemory", {
+    value: function () {
+      count++;
+      let promise = oldShrink.call(c);
+      shrinkPromises.push(promise);
+      return promise;
+    },
+  });
+
+  let now = new Date();
+  c._startIdleShrinkTimer();
+
+  let initialIdle = new Date(now.getTime() + INTERVAL);
+
+  // Perform database operations until initial scheduled time has been passed.
+  let i = 0;
+  while (new Date() < initialIdle) {
+    yield c.execute("INSERT INTO dirs (path) VALUES (?)", ["" + i]);
+    i++;
+  }
+
+  do_check_true(i > 0);
+
+  // We should not have performed an idle while doing operations.
+  do_check_eq(count, 0);
+
+  // Wait for idle timer.
+  yield sleep(INTERVAL);
+
+  // Ensure we fired.
+  do_check_eq(count, 1);
+  do_check_eq(shrinkPromises.length, 1);
+  yield shrinkPromises[0];
+
+  yield c.close();
+});
+