Bug 984875 - Improve performance of compose window autocomplete of recipients. r=standard8
authoraceman <acelists@atlas.sk>
Mon, 26 May 2014 11:46:00 -0400
changeset 16262 57ec47cea66e855d72a6cca695b92af2f9ee2829
parent 16261 78e61031f8d42729233b2313941fe6e397bc1d2f
child 16263 40fe87433749fc6e3a44278b0d81ca2c83ceaaea
push id10162
push userryanvm@gmail.com
push dateTue, 27 May 2014 17:45:25 +0000
treeherdercomm-central@40fe87433749 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersstandard8
bugs984875
Bug 984875 - Improve performance of compose window autocomplete of recipients. r=standard8
mailnews/addrbook/src/nsAbAutoCompleteSearch.js
--- a/mailnews/addrbook/src/nsAbAutoCompleteSearch.js
+++ b/mailnews/addrbook/src/nsAbAutoCompleteSearch.js
@@ -7,18 +7,19 @@ Components.utils.import("resource:///mod
 Components.utils.import("resource://gre/modules/XPCOMUtils.jsm");
 
 const ACR = Components.interfaces.nsIAutoCompleteResult;
 const nsIAbAutoCompleteResult = Components.interfaces.nsIAbAutoCompleteResult;
 
 function nsAbAutoCompleteResult(aSearchString) {
   // Can't create this in the prototype as we'd get the same array for
   // all instances
-  this._searchResults = new Array();
+  this._searchResults = []; // final results
   this.searchString = aSearchString;
+  this._collectedValues = new Map();  // temporary unsorted results
 }
 
 nsAbAutoCompleteResult.prototype = {
   _searchResults: null,
 
   // nsIAutoCompleteResult
 
   searchString: null,
@@ -170,17 +171,16 @@ nsAbAutoCompleteSearch.prototype = {
    *
    * @param aCard        The card to check.
    * @param aEmailToUse  The email address to check against.
    * @param aSearchWords Array of words in the multi word search string.
    * @return             True if the card matches the search parameters, false
    *                     otherwise.
    */
   _checkEntry: function _checkEntry(aCard, aEmailToUse, aSearchWords) {
-    var i;
     // Joining values of many fields in a single string so that a single
     // search query can be fired on all of them at once. Separating them
     // using spaces so that field1=> "abc" and field2=> "def" on joining
     // shouldn't return true on search for "bcd".
     let cumulativeFieldText = aCard.displayName + " " +
                               aCard.firstName + " " +
                               aCard.lastName + " " +
                               aEmailToUse + " " +
@@ -202,35 +202,33 @@ nsAbAutoCompleteSearch.prototype = {
    * @param directory       The directory that the card is in.
    * @param card            The card that could be a duplicate.
    * @param emailAddress    The emailAddress (name/address combination) to check
    *                        for duplicates against.
    * @param currentResults  The current results list.
    */
   _checkDuplicate: function _checkDuplicate(directory, card, emailAddress,
                                             currentResults) {
-    var lcEmailAddress = emailAddress.toLocaleLowerCase();
+    let lcEmailAddress = emailAddress.toLocaleLowerCase();
+    let existingResult = currentResults._collectedValues.get(lcEmailAddress);
+    let popIndex = this._getPopularityIndex(directory, card);
 
-    var popIndex = this._getPopularityIndex(directory, card);
-    for (var i = 0; i < currentResults._searchResults.length; ++i) {
-      if (currentResults._searchResults[i].value.toLocaleLowerCase() ==
-          lcEmailAddress)
-      {
-        // It's a duplicate, is the new one is more popular?
-        if (popIndex > currentResults._searchResults[i].popularity) {
-          // Yes it is, so delete this element, return false and allow
-          // _addToResult to sort the new element into the correct place.
-          currentResults._searchResults.splice(i, 1);
-          return false;
-        }
-        // Not more popular, but still a duplicate. Return true and _addToResult
-        // will just forget about it.
-        return true;
+    if (existingResult) {
+      // It's a duplicate, is the new one more popular?
+      if (popIndex > existingResult.popularity) {
+        // Yes it is, so delete this element, return false and allow
+        // _addToResult to sort the new element into the correct place.
+        currentResults._collectedValues.delete(lcEmailAddress);
+        return false;
       }
+      // Not more popular, but still a duplicate. Return true and _addToResult
+      // will just forget about it.
+      return true;
     }
+
     return false;
   },
 
   /**
    * Adds a card to the results list if it isn't a duplicate. The function will
    * order the results by popularity.
    *
    * @param commentColumn  The text to be displayed in the comment column
@@ -251,40 +249,23 @@ nsAbAutoCompleteSearch.prototype = {
                                      card.getProperty("Notes", "") || card.displayName :
                                      emailToUse).toString();
 
     // If it is a duplicate, then just return and don't add it. The
     // _checkDuplicate function deals with it all for us.
     if (this._checkDuplicate(directory, card, emailAddress, result))
       return;
 
-    // Find out where to insert the card.
-    var insertPosition = 0;
-    var cardPopularityIndex = this._getPopularityIndex(directory, card);
-
-    while (insertPosition < result._searchResults.length &&
-           cardPopularityIndex <
-           result._searchResults[insertPosition].popularity)
-      ++insertPosition;
-
-    // Next sort on full address
-    while (insertPosition < result._searchResults.length &&
-           cardPopularityIndex ==
-           result._searchResults[insertPosition].popularity &&
-           ((card == result._searchResults[insertPosition].card &&
-             !isPrimaryEmail) ||
-            emailAddress > result._searchResults[insertPosition].value))
-      ++insertPosition;
-
-    result._searchResults.splice(insertPosition, 0, {
+    result._collectedValues.set(emailAddress.toLocaleLowerCase(), {
       value: emailAddress,
       comment: commentColumn,
       card: card,
+      isPrimaryEmail: isPrimaryEmail,
       emailToUse: emailToUse,
-      popularity: cardPopularityIndex
+      popularity: this._getPopularityIndex(directory, card)
     });
   },
 
   // nsIAutoCompleteSearch
 
   /**
    * Starts a search based on the given parameters.
    *
@@ -325,17 +306,17 @@ nsAbAutoCompleteSearch.prototype = {
       this._commentColumn = Services.prefs.getIntPref("mail.autoComplete.commentColumn");
     } catch(e) { }
 
     if (aPreviousResult instanceof nsIAbAutoCompleteResult &&
         aSearchString.startsWith(aPreviousResult.searchString) &&
         aPreviousResult.searchResult == ACR.RESULT_SUCCESS) {
       // We have successful previous matches, therefore iterate through the
       // list and reduce as appropriate
-      for (var i = 0; i < aPreviousResult.matchCount; ++i) {
+      for (let i = 0; i < aPreviousResult.matchCount; ++i) {
         if (this._checkEntry(aPreviousResult.getCardAt(i),
                              aPreviousResult.getEmailToUse(i), searchWords))
           // If it matches, just add it straight onto the array, these will
           // already be in order because the previous search returned them
           // in the correct order.
           result._searchResults.push({
             value: aPreviousResult.getValueAt(i),
             comment: aPreviousResult.getCommentAt(i),
@@ -358,29 +339,40 @@ nsAbAutoCompleteSearch.prototype = {
       let modelQuery = "(or(DisplayName,c,@V)(FirstName,c,@V)(LastName,c,@V)" +
                        "(NickName,c,@V)(PrimaryEmail,c,@V)(SecondEmail,c,@V)" +
                        "(and(IsMailList,=,TRUE)(Notes,c,@V)))";
       for (let searchWord of searchWords) {
         searchQuery += modelQuery.replace(/@V/g, encodeABTermValue(searchWord));
       }
       searchQuery = "?(and" + searchQuery + ")";
       // Now do the searching
-      var allABs = this._abManager.directories;
+      let allABs = this._abManager.directories;
 
       // We're not going to bother searching sub-directories, currently the
       // architecture forces all cards that are in mailing lists to be in ABs as
       // well, therefore by searching sub-directories (aka mailing lists) we're
       // just going to find duplicates.
       while (allABs.hasMoreElements()) {
-        var dir = allABs.getNext();
+        let dir = allABs.getNext();
         if (dir instanceof Components.interfaces.nsIAbDirectory &&
             dir.useForAutocomplete(params.idKey)) {
           this._searchCards(searchQuery, dir, result);
         }
       }
+
+      result._searchResults = [...result._collectedValues.values()];
+      // Order by descending popularity, then primary email before secondary
+      // for the same card, then for differing cards sort by email.
+      function order_by_popularity_and_email(a, b) {
+        return (b.popularity - a.popularity) ||
+               ((a.card == b.card && a.isPrimaryEmail) ? -1 : 0) ||
+               ((a.value < b.value) ? -1 : (a.value == b.value) ? 0 : 1);
+        // TODO: this should actually use a.value.localeCompare(b.value) .
+      }
+      result._searchResults.sort(order_by_popularity_and_email);
     }
 
     if (result.matchCount) {
       result.searchResult = ACR.RESULT_SUCCESS;
       result.defaultIndex = 0;
     }
 
     aListener.onSearchResult(this, result);