Bug 1060891 - Reduce stats service memory usage and avoid jank on /list. r=florian

--HG--
extra : rebase_source : 4f7fbafff166474e5e8a00ed03fbc2d7237530a4

chat/components/public/imIAccount.idl

@@ -44,9 +44,8 @@ interface prplIChatRoomField: nsISupports {
 /*
  * Information about a chat room and the fields required to join it.
  */
-[scriptable, uuid(3fc279d9-b280-4f4a-babd-b4e328e65a01)]
+[scriptable, uuid(017d5951-fdd0-4f26-b697-fcc138cd2861)]
 interface prplIRoomInfo: nsISupports {
-  readonly attribute AUTF8String accountId;
   readonly attribute AUTF8String name;
   readonly attribute AUTF8String topic;
   readonly attribute unsigned long participantCount;
@@ -56,15 +55,14 @@ interface prplIRoomInfo: nsISupports {
 /*
  * Callback passed to an account's requestRoomInfo function.
  */
-[scriptable, function, uuid(d3c13bde-b852-44a8-a0dc-c781366cf110)]
+[scriptable, function, uuid(43102a36-883a-421d-a6ac-126aafee5a28)]
 interface prplIRoomInfoCallback: nsISupports {
-  /* aRooms is an array containing a batch of prplIRoomInfo. This will be called
+  /* aRooms is an array of chatroom names. This will be called
    * multiple times as batches of chat rooms are received. The number of rooms
    * in each batch is left for the prplIAccount implementation to decide.
    * aCompleted will be true when aRooms is the last batch.
    */
-  void onRoomInfoAvailable([array, size_is(aCount)] in prplIRoomInfo aRooms,
-                           in prplIAccount aAccount,
+  void onRoomInfoAvailable([array, size_is(aCount)] in wstring aRooms,
                            in boolean aCompleted,
                            in unsigned long aCount);
 };
@@ -73,7 +71,7 @@ interface prplIRoomInfoCallback: nsISupports {
 /*
  * This interface should be implemented by the protocol plugin.
  */
-[scriptable, uuid(55331ab9-54e9-4cd3-bb19-a46da76898e5)]
+[scriptable, uuid(3ce02a3c-f38b-4a1e-9050-a19bea1cb6c1)]
 interface prplIAccount: nsISupports {
   readonly attribute imIAccount imAccount;
 
@@ -117,10 +115,12 @@ interface prplIAccount: nsISupports {
   readonly attribute boolean canJoinChat;
   nsISimpleEnumerator getChatRoomFields();
   prplIChatRoomFieldValues getChatRoomDefaultFieldValues([optional] in AUTF8String aDefaultChatName);
-  /* Request information on available chat rooms, to be returned via the
-   * callback.
+
+  /* Request information on available chat rooms, whose names are returned
+   * via the callback.
    */
   void requestRoomInfo(in prplIRoomInfoCallback aCallback);
+  prplIRoomInfo getRoomInfo(in AUTF8String aRoomName);
   readonly attribute boolean isRoomInfoStale;
 
   /*

chat/modules/jsProtoHelper.jsm

@@ -207,6 +207,7 @@ var GenericAccountPrototype = {
     return new ChatRoomFieldValues(defaultFieldValues);
   },
   requestRoomInfo: function(aCallback) { throw Cr.NS_ERROR_NOT_IMPLEMENTED; },
+  getRoomInfo: function(aName) { throw Cr.NS_ERROR_NOT_IMPLEMENTED; },
   get isRoomInfoStale() { return false; },
 
   getPref: function (aName, aType) {

chat/protocols/irc/irc.js

@@ -765,6 +765,21 @@ ircAccountBuddy.prototype = {
   }
 };
 
+function ircRoomInfo(aName, aAccount) {
+  this.name = aName;
+  this._account = aAccount;
+}
+ircRoomInfo.prototype = {
+  __proto__: ClassInfo("prplIRoomInfo", "IRC RoomInfo Object"),
+  get topic() { return this._account._channelList.get(this.name).topic; },
+  get participantCount() {
+    return this._account._channelList.get(this.name).participantCount;
+  },
+  get chatRoomFieldValues() {
+    return this._account.getChatRoomDefaultFieldValues(this.name);
+  }
+};
+
 function ircAccount(aProtocol, aImAccount) {
   this._init(aProtocol, aImAccount);
   this.buddies = new NormalizedMap(this.normalizeNick.bind(this));
@@ -941,13 +956,14 @@ ircAccount.prototype = {
     return true;
   },
 
-  // Channels are stored as prplIRoomInfo.
-  _channelList: [],
+  // Room info: maps channel names to {topic, participantCount}.
+  _channelList: new Map(),
   _roomInfoCallbacks: new Set(),
   // If true, we have sent the LIST request and are waiting for replies.
   _pendingList: false,
-  // Callbacks receive at most this many channels per call.
-  _channelsPerBatch: 25,
+  // Callbacks receive this many channels per call while results are incoming.
+  _channelsPerBatch: 50,
+  _currentBatch: [],
   _lastListTime: 0,
   get isRoomInfoStale() { return Date.now() - this._lastListTime > kListRefreshInterval; },
   // Called by consumers that want a list of available channels, which are
@@ -962,15 +978,16 @@ ircAccount.prototype = {
     // Send a LIST request if the channel list is stale and a current request
     // has not been sent.
     if (this.isRoomInfoStale && !this._pendingList) {
-      this._channelList = [];
+      this._channelList = new Map();
+      this._currentBatch = [];
       this._pendingList = true;
       this._lastListTime = Date.now();
       this.sendMessage("LIST");
     }
     // Otherwise, pass channels that have already been received to the callback.
     else {
-      aCallback.onRoomInfoAvailable(this._channelList, this, !this._pendingList,
-                                    this._channelList.length);
+      let rooms = [...this._channelList.keys()];
+      aCallback.onRoomInfoAvailable(rooms, !this._pendingList, rooms.length);
     }
 
     if (this._pendingList)
@@ -978,16 +995,18 @@ ircAccount.prototype = {
   },
   // Pass room info for any remaining channels to callbacks and clean up.
   _sendRemainingRoomInfo: function() {
-    let remainingChannelCount = this._channelList.length % this._channelsPerBatch;
-    if (remainingChannelCount) {
-      let remainingChannels = this._channelList.slice(-remainingChannelCount);
+    if (this._currentBatch.length) {
       for (let callback of this._roomInfoCallbacks) {
-        callback.onRoomInfoAvailable(remainingChannels, this, true,
-                                     remainingChannelCount);
+        callback.onRoomInfoAvailable(this._currentBatch, true,
+                                     this._currentBatch.length);
       }
     }
     this._roomInfoCallbacks.clear();
     delete this._pendingList;
+    delete this._currentBatch;
+  },
+  getRoomInfo: function(aName) {
+    return new ircRoomInfo(aName, this);
   },
 
   // The last time a buffered command was sent.

chat/protocols/irc/ircBase.jsm

@@ -27,20 +27,6 @@ Cu.import("resource:///modules/ircHandlers.jsm");
 Cu.import("resource:///modules/ircUtils.jsm");
 Cu.import("resource:///modules/jsProtoHelper.jsm");
 
-function ircRoomInfo(aName, aTopic, aParticipantCount, aAccount) {
-  this.name = aName;
-  this.topic = aTopic;
-  this.participantCount = aParticipantCount;
-  this._account = aAccount;
-}
-ircRoomInfo.prototype = {
-  __proto__: ClassInfo("prplIRoomInfo", "IRC RoomInfo Object"),
-  get accountId() { return this._account.imAccount.id; },
-  get chatRoomFieldValues() {
-    return this._account.getChatRoomDefaultFieldValues(this.name);
-  }
-}
-
 function privmsg(aAccount, aMessage, aIsNotification) {
   let params = {incoming: true};
   if (aIsNotification)
@@ -811,14 +797,16 @@ var ircBase = {
       // 1058584. This hack can be removed when bug 1058653 is fixed.
       topic = topic ? topic.normalize() : "";
 
-      this._channelList.push(new ircRoomInfo(name, topic, participantCount, this));
+      this._channelList.set(name,
+                            {topic: topic, participantCount: participantCount});
+      this._currentBatch.push(name);
       // Give callbacks a batch of channels of length _channelsPerBatch.
-      if (this._channelList.length % this._channelsPerBatch == 0) {
-        let channelBatch = this._channelList.slice(-this._channelsPerBatch);
+      if (this._currentBatch.length == this._channelsPerBatch) {
         for (let callback of this._roomInfoCallbacks) {
-          callback.onRoomInfoAvailable(channelBatch, this, false,
+          callback.onRoomInfoAvailable(this._currentBatch, false,
                                        this._channelsPerBatch);
         }
+        this._currentBatch = [];
       }
       return true;
     },

chat/protocols/irc/ircCommands.jsm

@@ -10,6 +10,8 @@ var {classes: Cc, interfaces: Ci, utils: Cu} = Components;
 
 Cu.import("resource:///modules/imXPCOMUtils.jsm");
 Cu.import("resource:///modules/ircUtils.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "Task",
+  "resource://gre/modules/Task.jsm");
 
 // Shortcut to get the JavaScript conversation object.
 function getConv(aConv) { return aConv.wrappedJSObject; };
@@ -269,14 +271,27 @@ var commands = [
       let account = getAccount(aConv);
       let serverName = account._currentServerName;
       let serverConv = account.getConversation(serverName);
+      let pendingChats = [];
       account.requestRoomInfo({onRoomInfoAvailable: function(aRooms) {
-        aRooms.forEach(function(aRoom) {
-          serverConv.writeMessage(serverName,
-                                  aRoom.name +
-                                  " (" + aRoom.participantCount + ") " +
-                                  aRoom.topic,
-                                  {incoming: true, noLog: true});
-        });
+        if (!pendingChats.length) {
+          Task.spawn(function*() {
+            // pendingChats has no rooms added yet, so ensure we wait a tick.
+            let t = 0;
+            const kMaxBlockTime = 10; // Unblock every 10ms.
+            do {
+              if (Date.now() > t) {
+                yield Promise.resolve();
+                t = Date.now() + kMaxBlockTime;
+              }
+              let name = pendingChats.pop();
+              let roomInfo = account.getRoomInfo(name);
+              serverConv.writeMessage(serverName,
+                name + " (" + roomInfo.participantCount + ") " + roomInfo.topic,
+                {incoming: true, noLog: true});
+            } while (pendingChats.length);
+          });
+        }
+        pendingChats = pendingChats.concat(aRooms);
       }}, true);
       if (aReturnedConv)
         aReturnedConv.value = serverConv;

im/components/ibConvStatsService.js

@@ -5,6 +5,7 @@
 var {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components;
 Cu.import("resource:///modules/imXPCOMUtils.jsm");
 Cu.import("resource:///modules/imServices.jsm");
+Cu.import("resource://gre/modules/Task.jsm")
 Cu.import("resource://gre/modules/osfile.jsm");
 
 var kNotificationsToObserve =
@@ -198,44 +199,62 @@ ConvStatsService.prototype = {
     this._contactsById.delete(aId);
   },
 
-  // Queue of RoomInfo to be added.
-  _pendingChats: [],
   // The last time an update notification was sent to observers.
   _lastUpdateNotification: 0,
   // Account ids from which chat room info has been requested.
   // We send an update notification if this is empty after adding chat rooms.
   _accountsRequestingRoomInfo: new Set(),
-  _addPendingChats: function() {
-    let begin = Date.now();
-    for (let time = 0; time < 15 && this._pendingChats.length;
-         time = Date.now() - begin) {
-      let chat = this._pendingChats.pop();
-      let accountId = chat.accountId;
-      let chatList = this._chatsByAccountIdAndName.get(accountId);
-      if (!chatList) {
-        chatList = new Map();
-        this._chatsByAccountIdAndName.set(accountId, chatList);
-      }
-      // If a chat is already added, we remove it and re-add to refresh.
-      else if (chatList.has(chat.name)) {
-        this._convs.splice(
-          this._convs.indexOf(chatList.get(chat.name)), 1);
-      }
-      let possibleConv = new PossibleChat(chat);
-      let pos = this._getPositionToInsert(possibleConv, this._convs);
-      this._convs.splice(pos, 0, possibleConv);
-      chatList.set(chat.name, possibleConv);
-    }
-    if (this._pendingChats.length)
-      executeSoon(() => this._addPendingChats());
-    else
-      delete this._addingPendingChats;
-    let now = Date.now();
-    if ((!this._accountsRequestingRoomInfo.size && !this._pendingChats.length) ||
-        now - this._lastUpdateNotification > 500) {
-      this._notifyObservers("updated");
-      this._lastUpdateNotification = now;
+  _addPendingChats: function(aAccountId, aRoomInfo) {
+    if (this._pendingChats) {
+      this._pendingChats.push([aAccountId, aRoomInfo]);
+      return;
     }
+
+    this._pendingChats = [[aAccountId, aRoomInfo]];
+    Task.spawn(function*() {
+      let t = Date.now();
+      let sendUpdateNotification = () => {
+        if ((!this._accountsRequestingRoomInfo.size &&
+             !this._pendingChats.length) ||
+            t - this._lastUpdateNotification > 500) {
+          this._notifyObservers("updated");
+          this._lastUpdateNotification = t;
+        }
+      };
+
+      while (this._pendingChats.length) {
+        let [accountId, rooms] = this._pendingChats.pop();
+
+        let chatList = this._chatsByAccountIdAndName.get(accountId);
+        if (!chatList) // Account no longer connected.
+          continue;
+
+        for (let name of rooms) {
+          // If a chat is already added, we remove it and re-add to refresh.
+          if (chatList.has(name))
+            this._convs.splice(this._convs.indexOf(chatList.get(name)), 1);
+          let possibleConv = new PossibleChat(accountId, name);
+          let pos = this._getPositionToInsert(possibleConv, this._convs);
+          this._convs.splice(pos, 0, possibleConv);
+          chatList.set(name, possibleConv);
+
+          // Unblock every 10ms.
+          if (Date.now() > t + 10) {
+            yield Promise.resolve();
+            t = Date.now();
+            sendUpdateNotification();
+            if (!this._chatsByAccountIdAndName.has(accountId)) {
+              // The account was disconnected in the meantime, so don't add
+              // any more chats to it.
+              break;
+            }
+          }
+        }
+      }
+      t = Date.now();
+      sendUpdateNotification();
+      delete this._pendingChats;
+    }.bind(this));
   },
 
   _removeChatsForAccount: function(aAccId) {
@@ -245,7 +264,6 @@ ConvStatsService.prototype = {
     this._convs = this._convs.filter(c =>
       c.source != "chat" || c.accountId != aAccId);
     this._chatsByAccountIdAndName.delete(aAccId);
-    this._pendingChats = this._pendingChats.filter(c => c.accountId != aAccId);
   },
 
   _getPositionToInsert: function(aPossibleConversation, aArrayToInsert) {
@@ -376,18 +394,17 @@ ConvStatsService.prototype = {
         // Discard any chat room data we already have.
         this._removeChatsForAccount(id);
         try {
-          acc.prplAccount.requestRoomInfo((aRoomInfo, aPrplAccount, aCompleted) => {
+          this._chatsByAccountIdAndName.set(id, new Map());
+          acc.prplAccount.requestRoomInfo((aRoomInfo, aCompleted) => {
             if (aCompleted)
-              this._accountsRequestingRoomInfo.delete(acc.id);
-            this._pendingChats = this._pendingChats.concat(aRoomInfo);
-            if (this._addingPendingChats)
-              return;
-            this._addingPendingChats = true;
-            executeSoon(() => this._addPendingChats());
+              this._accountsRequestingRoomInfo.delete(id);
+            this._addPendingChats(id, aRoomInfo);
           });
-          this._accountsRequestingRoomInfo.add(acc.id);
+          this._accountsRequestingRoomInfo.add(id);
         } catch(e) {
-          if (e.result != Cr.NS_ERROR_NOT_IMPLEMENTED)
+          if (e.result == Cr.NS_ERROR_NOT_IMPLEMENTED)
+            this._chatsByAccountIdAndName.delete(id);
+          else
             Cu.reportError(e);
           continue;
         }
@@ -661,34 +678,36 @@ PossibleConvFromContact.prototype = {
   createConversation: function() { return this.contact.createConversation(); }
 };
 
-function PossibleChat(aRoomInfo) {
-  this._roomInfo = aRoomInfo;
+function PossibleChat(aAccountId, aName) {
+  this.displayName = aName;
+  this.accountId = aAccountId;
   let account = this.account;
   this.id = getConversationId(account.protocol.normalizedName,
                               account.normalizedName,
-                              account.normalize(aRoomInfo.name), true);
+                              account.normalize(aName), true);
 }
 PossibleChat.prototype = {
   get isChat() { return true; },
   get statusType() { return Ci.imIStatusInfo.STATUS_AVAILABLE; },
   get buddyIconFilename() { return ""; },
-  get displayName() { return this._roomInfo.name; },
   get lowerCaseName() {
     return this._lowerCaseName || (this._lowerCaseName = this.displayName.toLowerCase());
   },
   get statusText() {
-    return "(" + this._roomInfo.participantCount + ") " + this._roomInfo.topic;
+    let roomInfo = this.account.prplAccount.getRoomInfo(this.displayName);
+    return "(" + roomInfo.participantCount + ") " + roomInfo.topic;
   },
   get infoText() { return this.account.normalizedName; },
   get source() { return "chat"; },
-  get accountId() { return this._roomInfo.accountId; },
   get account() { return Services.accounts.getAccountById(this.accountId); },
   createConversation: function() {
-    this.account.joinChat(this._roomInfo.chatRoomFieldValues);
+    let account = this.account;
+    account.joinChat(account.prplAccount.getRoomInfo(this.displayName)
+                                        .chatRoomFieldValues);
     // Work around the fact that joinChat doesn't return the conv.
     return Services.conversations
-                   .getConversationByNameAndAccount(this._roomInfo.name,
-                                                    this.account, true);
+                   .getConversationByNameAndAccount(this.displayName,
+                                                    account, true);
   },
   get computedScore() {
     let stats = gStatsByConvId[this.id];