Bug 833609 - Part 2: Add timer to shrink memory after idle; r=mak
authorGregory Szorc <gps@mozilla.com>
Thu, 24 Jan 2013 13:30:20 -0800
changeset 129847 2616a8dbe520e4b91c4faa39cde06c841b314283
parent 129846 3da328ea9e2c191221bcaf19518db146599f8a14
child 129848 e49e91aefec47ec64691345f07bc098d1ae375c9
push id2323
push userbbajaj@mozilla.com
push dateMon, 01 Apr 2013 19:47:02 +0000
treeherdermozilla-beta@7712be144d91 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs833609
milestone21.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 833609 - Part 2: Add timer to shrink memory after idle; r=mak
toolkit/modules/Sqlite.jsm
toolkit/modules/tests/xpcshell/test_sqlite.js
--- a/toolkit/modules/Sqlite.jsm
+++ b/toolkit/modules/Sqlite.jsm
@@ -3,17 +3,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 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");
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://services-common/log4moz.js");
 
 XPCOMUtils.defineLazyModuleGetter(this, "CommonUtils",
@@ -38,16 +38,23 @@ let connectionCounters = {};
  *       file does not exist, a new database will be created.
  *
  *   sharedMemoryCache -- (bool) Whether multiple connections to the database
  *       share the same memory cache. Sharing the memory cache likely results
  *       in less memory utilization. However, sharing also requires connections
  *       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:
  *
  *   special named databases
  *   pragma TEMP STORE = MEMORY
  *   TRUNCATE JOURNAL
  *   SYNCHRONOUS = full
  *
@@ -64,16 +71,28 @@ function openConnection(options) {
   }
 
   // Retains absolute paths and normalizes relative as relative to profile.
   let path = OS.Path.join(OS.Constants.Path.profileDir, options.path);
 
   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 :
                          Services.storage.openUnsharedDatabase;
 
   let basename = OS.Path.basename(path);
 
   if (!connectionCounters[basename]) {
@@ -87,17 +106,18 @@ function openConnection(options) {
   try {
     let connection = openDatabaseFn(file);
 
     if (!connection.connectionReady) {
       log.warn("Connection is not ready.");
       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);
   }
 }
 
 
 /**
@@ -138,18 +158,21 @@ function openConnection(options) {
  *   Support for SAVEPOINT (named/nested transactions) might be useful.
  *
  * @param connection
  *        (mozIStorageConnection) Underlying SQLite connection.
  * @param basename
  *        (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
   // object since they would have effect on all instances and last write would
   // win. So, we create a "proxy" object with our custom functions. Everything
   // else is proxied back to the shared logger instance via prototype
   // inheritance.
   let logProxy = {__proto__: log};
@@ -175,16 +198,24 @@ function OpenedConnection(connection, ba
 
   this._cachedStatements = new Map();
   this._anonymousStatements = new Map();
   this._anonymousCounter = 0;
   this._inProgressStatements = new Map();
   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({
   TRANSACTION_DEFERRED: "DEFERRED",
   TRANSACTION_IMMEDIATE: "IMMEDIATE",
   TRANSACTION_EXCLUSIVE: "EXCLUSIVE",
 
   TRANSACTION_TYPES: ["DEFERRED", "IMMEDIATE", "EXCLUSIVE"],
@@ -254,17 +285,17 @@ OpenedConnection.prototype = Object.free
    * @return Promise<>
    */
   close: function () {
     if (!this._connection) {
       return Promise.resolve();
     }
 
     this._log.debug("Request to close connection.");
-
+    this._clearIdleShrinkTimer();
     let deferred = Promise.defer();
 
     // We need to take extra care with transactions during shutdown.
     //
     // If we don't have a transaction in progress, we can proceed with shutdown
     // immediately.
     if (!this._inProgressTransaction) {
       this._finalize(deferred);
@@ -384,17 +415,37 @@ OpenedConnection.prototype = Object.free
     }
 
     let statement = this._cachedStatements.get(sql);
     if (!statement) {
       statement = this._connection.createAsyncStatement(sql);
       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;
   },
 
   /**
    * Execute a one-shot SQL statement.
    *
    * If you find yourself feeding the same SQL string in this function, you
    * should *not* use this function and instead use `executeCached`.
    *
@@ -413,32 +464,42 @@ OpenedConnection.prototype = Object.free
     }
 
     this._ensureOpen();
 
     let statement = this._connection.createAsyncStatement(sql);
     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;
   },
 
   /**
    * Whether a transaction is currently in progress.
    */
   get transactionInProgress() {
@@ -587,17 +648,21 @@ OpenedConnection.prototype = Object.free
   },
 
   /**
    * Free up as much memory from the underlying database connection as possible.
    *
    * @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) {
     if (statement.state != statement.MOZ_STORAGE_STATEMENT_READY) {
       throw new Error("Statement is not ready for execution.");
     }
 
     if (onRow && typeof(onRow) != "function") {
@@ -709,13 +774,31 @@ OpenedConnection.prototype = Object.free
     return deferred.promise;
   },
 
   _ensureOpen: function () {
     if (!this._open) {
       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 = {
   openConnection: openConnection,
 };
--- a/toolkit/modules/tests/xpcshell/test_sqlite.js
+++ b/toolkit/modules/tests/xpcshell/test_sqlite.js
@@ -1,36 +1,55 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
 "use strict";
 
-const {utils: Cu} = Components;
+const {classes: Cc, interfaces: Ci, utils: Cu} = Components;
 
 do_get_profile();
 
 Cu.import("resource://gre/modules/commonjs/promise/core.js");
 Cu.import("resource://gre/modules/osfile.jsm");
 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();
+
+  let timer = Cc["@mozilla.org/timer;1"]
+                .createInstance(Ci.nsITimer);
 
-  return Sqlite.openConnection({path: path});
+  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 + ")");
   }
 
   throw new Task.Result(c);
 }
 
@@ -156,21 +175,27 @@ add_task(function test_close_cached() {
   yield c.close();
 });
 
 add_task(function test_execute_invalid_statement() {
   let c = yield getDummyDatabase("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();
 });
 
 add_task(function test_on_row_exception_ignored() {
   let c = yield getDummyDatabase("on_row_exception_ignored");
 
   let sql = "INSERT INTO dirs (path) VALUES (?)";
   for (let i = 0; i < 10; i++) {
@@ -327,8 +352,122 @@ add_task(function test_shrink_memory() {
   let c = yield getDummyDatabase("shrink_memory");
 
   // It's just a simple sanity test. We have no way of measuring whether this
   // actually does anything.
 
   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();
+});
+