Bug 1346979 - optimize Bookmarks.jsm's updateFrecency, r?mak draft
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Mon, 27 Mar 2017 15:34:14 +0100
changeset 556121 2aacf3f8d9482983f4f98056ddf18cdc7c056398
parent 555486 916a4ee676a33355fc717f15ecb80815ba95051e
child 622792 a2d7ac32a0fd0c01e5b99ff6e206612c0c232b5c
push id52444
push userbmo:gijskruitbosch+bugs@gmail.com
push dateWed, 05 Apr 2017 10:59:37 +0000
reviewersmak
bugs1346979
milestone55.0a1
Bug 1346979 - optimize Bookmarks.jsm's updateFrecency, r?mak MozReview-Commit-ID: GN35Ec0Nn8f
toolkit/components/places/Bookmarks.jsm
--- a/toolkit/components/places/Bookmarks.jsm
+++ b/toolkit/components/places/Bookmarks.jsm
@@ -1296,17 +1296,17 @@ function insertBookmarkTree(items, sourc
          :title, :date_added, :last_modified, :guid,
          :syncChangeCounter, :syncStatus)`, items);
 
       yield setAncestorsLastModified(db, parent.guid, lastAddedForParent,
                                      syncChangeDelta);
     });
 
     // We don't wait for the frecency calculation.
-    updateFrecency(db, urls).catch(Cu.reportError);
+    updateFrecency(db, urls, true).catch(Cu.reportError);
 
     return items;
   }));
 }
 
 // Query implementation.
 
 function queryBookmarks(info) {
@@ -1777,34 +1777,39 @@ function validateBookmarkObject(input, b
 
 /**
  * Updates frecency for a list of URLs.
  *
  * @param db
  *        the Sqlite.jsm connection handle.
  * @param urls
  *        the array of URLs to update.
+ * @param [optional] collapseNotifications
+ *        whether we can send just one onManyFrecenciesChanged
+ *        notification instead of sending one notification for every URL.
  */
-var updateFrecency = Task.async(function* (db, urls) {
-  // TODO: this can be optimized for multiple-URL usage, see bug 1346979
+var updateFrecency = Task.async(function* (db, urls, collapseNotifications = false) {
   let urlQuery = 'hash("' + urls.map(url => url.href).join('"), hash("') + '")';
+
+  let frecencyClause = "CALCULATE_FRECENCY(id)";
+  if (!collapseNotifications) {
+    frecencyClause = "NOTIFY_FRECENCY(" + frecencyClause +
+                     ", url, guid, hidden, last_visit_date)";
+  }
   // We just use the hashes, since updating a few additional urls won't hurt.
   yield db.execute(
     `UPDATE moz_places
-     SET frecency = NOTIFY_FRECENCY(
-       CALCULATE_FRECENCY(id), url, guid, hidden, last_visit_date
-     ) WHERE url_hash IN ( ${urlQuery} )
+     SET hidden = (url_hash BETWEEN hash("place", "prefix_lo") AND hash("place", "prefix_hi")),
+         frecency = ${frecencyClause}
+     WHERE url_hash IN ( ${urlQuery} )
     `);
-
-  yield db.execute(
-    `UPDATE moz_places
-     SET hidden = 0
-     WHERE url_hash IN ( ${urlQuery} )
-       AND frecency <> 0
-    `);
+  if (collapseNotifications) {
+    let observers = PlacesUtils.history.getObservers();
+    notify(observers, "onManyFrecenciesChanged");
+  }
 });
 
 /**
  * Removes any orphan annotation entries.
  *
  * @param db
  *        the Sqlite.jsm connection handle.
  */
@@ -1940,17 +1945,17 @@ Task.async(function* (db, folderGuids, o
   yield addSyncChangesForRemovedTagFolders(db, itemsRemoved, syncChangeDelta);
 
   // Cleanup orphans.
   yield removeOrphanAnnotations(db);
 
   // TODO (Bug 1087576): this may leave orphan tags behind.
 
   let urls = itemsRemoved.filter(item => "url" in item).map(item => item.url);
-  updateFrecency(db, urls).then(null, Cu.reportError);
+  updateFrecency(db, urls, true).then(null, Cu.reportError);
 
   // Send onItemRemoved notifications to listeners.
   // TODO (Bug 1087580): for the case of eraseEverything, this should send a
   // single clear bookmarks notification rather than notifying for each
   // bookmark.
 
   // Notify listeners in reverse order to serve children before parents.
   let { source = Bookmarks.SOURCES.DEFAULT } = options;