improve performance by caching sources and reusing them when getting a source from a message

modules/collection.js

@@ -241,16 +241,18 @@ this._log.info("got " + groups.length + " groups");
     this._messageIndex = {};
 
     let statement = this._generateStatement();
+    let message;
     try {
       while (statement.step()) {
-        let message = new SnowlMessage(statement.row.id,
-                                       statement.row.subject,
-                                       statement.row.author,
-                                       statement.row.link,
-                                       SnowlDateUtils.julianToJSDate(statement.row.timestamp),
-                                       (statement.row.read ? true : false),
-                                       statement.row.authorIcon,
-                                       SnowlDateUtils.julianToJSDate(statement.row.received));
+        message = new SnowlMessage({ id: statement.row.id,
+                               sourceID: statement.row.sourceID,
+                                subject: statement.row.subject,
+                                 author: statement.row.author,
+                                   link: statement.row.link,
+                              timestamp: SnowlDateUtils.julianToJSDate(statement.row.timestamp),
+                                  _read: (statement.row.read ? true : false),
+                             authorIcon: statement.row.authorIcon,
+                               received: SnowlDateUtils.julianToJSDate(statement.row.received) });
         this._messages.push(message);
         this._messageIndex[message.id] = message;
       }
@@ -301,8 +303,14 @@ this._log.info("got " + groups.length + " groups");
   },
 
   _generateStatement: function() {
-    let columns = ["messages.id", "subject", "authors.name AS author", "link",
-                   "timestamp", "read", "authors.iconURL AS authorIcon",
+    let columns = ["messages.id",
+                   "sourceID",
+                   "subject",
+                   "authors.name AS author",
+                   "link",
+                   "timestamp",
+                   "read",
+                   "authors.iconURL AS authorIcon",
                    "received"];
 
     if (this.groupIDColumn) {

modules/message.js

@@ -51,49 +51,43 @@ Cu.import("resource://snowl/modules/service.js");
 Cu.import("resource://snowl/modules/source.js");
 Cu.import("resource://snowl/modules/utils.js");
 
-function SnowlMessage(aID, aSubject, aAuthor, aLink, aTimestamp, aRead, aAuthorIcon, aReceived) {
-  this.id = aID;
-  this.subject = aSubject;
-  this.author = aAuthor;
-  this.link = aLink;
-  this.timestamp = aTimestamp;
-  this._read = aRead;
-  this.authorIcon = aAuthorIcon;
-  this.received = aReceived;
+function SnowlMessage(props) {
+  // The way this currently works requires instantiators to pass the value
+  // of the read property via its private name _read, which seems wrong.
+  // FIXME: make it so callers can pass read via its public name.
+  for (let name in props)
+    this[name] = props[name];
 }
 
-SnowlMessage.get = function(aID) {
+// FIXME: refactor this with the similar code in the SnowlCollection::messages getter.
+// FIXME: retrieve an author object instead of just specific properties of the author.
+// FIXME: retrieve all basic properties of the message in a single query.
+SnowlMessage.get = function(id) {
   let message;
 
   let statement = SnowlDatastore.createStatement(
-    "SELECT subject, authors.name AS author, link, timestamp, read, " +
+    "SELECT sourceID, subject, authors.name AS author, link, timestamp, read, " +
     "       authors.iconURL AS authorIcon, received " +
     "FROM messages LEFT JOIN people AS authors ON messages.authorID = authors.id " +
     "WHERE messages.id = :id"
   );
 
   try {
-    statement.params.id = aID;
+    statement.params.id = id;
     if (statement.step()) {
-      message = new SnowlMessage(aID,
-                                 statement.row.subject,
-                                 statement.row.author,
-                                 statement.row.link,
-                                 SnowlDateUtils.julianToJSDate(statement.row.timestamp),
-                                 (statement.row.read ? true : false),
-                                 statement.row.authorIcon,
-                                 SnowlDateUtils.julianToJSDate(statement.row.received));
+      message = new SnowlMessage({ id: id,
+                             sourceID: statement.row.sourceID,
+                              subject: statement.row.subject,
+                               author: statement.row.author,
+                                 link: statement.row.link,
+                            timestamp: SnowlDateUtils.julianToJSDate(statement.row.timestamp),
+                                _read: (statement.row.read ? true : false),
+                           authorIcon: statement.row.authorIcon,
+                             received: SnowlDateUtils.julianToJSDate(statement.row.received) });
     }
     else {
       // Message not there, create structure anyway
-      message = new SnowlMessage(null,
-                                 null,
-                                 null,
-                                 null,
-                                 null,
-                                 null,
-                                 null,
-                                 null);
+      message = new SnowlMessage({});
     }
   }
   finally {
@@ -209,29 +203,8 @@ SnowlMessage.prototype = {
     return this._notfound;
   },
 
-  // FIXME: for performance, make this a class property rather than an instance
-  // property?
-  get _getSourceIDStatement() {
-    let statement = SnowlDatastore.createStatement(
-      "SELECT sourceID FROM messages WHERE id = :id"
-    );
-    this.__defineGetter__("_getSourceIDStatement", function() { return statement });
-    return this._getSourceIDStatement;
-  },
-
   get source() {
-    if (!this._source) {
-      try {
-        this._getSourceIDStatement.params.id = this.id;
-        if (this._getSourceIDStatement.step())
-          this._source = SnowlService.getAccount(this._getSourceIDStatement.row.sourceID);
-      }
-      finally {
-        this._getSourceIDStatement.reset();
-      }
-    }
-
-    return this._source;
-  },
+    return SnowlService.sourcesByID[this.sourceID];
+  }
 
 };

modules/service.js

@@ -46,6 +46,7 @@ Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
 // modules that are generic
 Cu.import("resource://snowl/modules/log4moz.js");
+Cu.import("resource://snowl/modules/Observers.js");
 Cu.import("resource://snowl/modules/Preferences.js");
 Cu.import("resource://snowl/modules/URI.js");
 
@@ -101,6 +102,8 @@ let SnowlService = {
     this._registerFeedHandler();
     this._initTimer();
 
+    Observers.add(this, "snowl:sources:changed");
+
     // FIXME: refresh stale sources on startup in a way that doesn't hang
     // the UI thread.
     //this.refreshStaleSources();
@@ -146,6 +149,28 @@ let SnowlService = {
                                               null);
   },
 
+
+  //**************************************************************************//
+  // Event & Notification Handlers
+
+  // nsIObserver
+  observe: function(subject, topic, data) {
+    switch (topic) {
+      case "snowl:sources:changed":
+        this._onSourcesChanged();
+        break;
+    }
+  },
+
+  _onSourcesChanged: function() {
+    // Invalidate the cache of sources indexed by ID.
+    this._sourcesByID = null;
+  },
+
+
+  //**************************************************************************//
+  // Accounts, Sources, Targets
+
   _accountTypeConstructors: {},
   addAccountType: function(constructor) {
     if (constructor in this._accountTypeConstructors)
@@ -231,6 +256,17 @@ let SnowlService = {
     return this.accounts.filter(function(acct) acct.implements(SnowlSource));
   },
 
+  _sourcesByID: null,
+  get sourcesByID() {
+    if (!this._sourcesByID) {
+      this._sourcesByID = {};
+      for each (let source in this.sources)
+        this._sourcesByID[source.id] = source;
+    }
+
+    return this._sourcesByID;
+  },
+
   get targets() {
     return this.accounts.filter(function(acct) acct.implements(SnowlTarget));
   },