Bug 707945 - Remove the keywords trigger since it's a useless perf hog.
authorMarco Bonardo <mbonardo@mozilla.com>
Wed, 07 Dec 2011 21:56:23 +0100
changeset 82204 a3d66acea00353ae7a9bf88bb4315382aa8d56ab
parent 82203 a0daa1a39a20f92ccecb4c0f92dcc302070b7f48
child 82205 1b51ee2ee38818c0f1eaaea44d0c7acd2fac33ff
push id21587
push userbmo@edmorley.co.uk
push dateThu, 08 Dec 2011 15:13:43 +0000
treeherdermozilla-central@98db2311a44c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs707945
milestone11.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 707945 - Remove the keywords trigger since it's a useless perf hog. r=dietrich
toolkit/components/places/Database.cpp
toolkit/components/places/Database.h
toolkit/components/places/nsNavBookmarks.cpp
toolkit/components/places/nsPlacesTriggers.h
toolkit/components/places/tests/head_common.js
--- a/toolkit/components/places/Database.cpp
+++ b/toolkit/components/places/Database.cpp
@@ -644,16 +644,21 @@ Database::InitSchema(bool* aDatabaseMigr
         NS_ENSURE_SUCCESS(rv, rv);
       }
 
       if (currentSchemaVersion < 14) {
         rv = MigrateV14Up();
         NS_ENSURE_SUCCESS(rv, rv);
       }
 
+      if (currentSchemaVersion < 15) {
+        rv = MigrateV15Up();
+        NS_ENSURE_SUCCESS(rv, rv);
+      }
+
       // Firefox 11 uses schema version 14.
 
       // Schema Upgrades must add migration code here.
     }
   }
   else {
     // This is a new database, so we have to create all the tables and indices.
 
@@ -703,18 +708,16 @@ Database::InitSchema(bool* aDatabaseMigr
 
     // moz_bookmarks_roots.
     rv = mMainConn->ExecuteSimpleSQL(CREATE_MOZ_BOOKMARKS_ROOTS);
     NS_ENSURE_SUCCESS(rv, rv);
 
     // moz_keywords.
     rv = mMainConn->ExecuteSimpleSQL(CREATE_MOZ_KEYWORDS);
     NS_ENSURE_SUCCESS(rv, rv);
-    rv = mMainConn->ExecuteSimpleSQL(CREATE_KEYWORD_VALIDITY_TRIGGER);
-    NS_ENSURE_SUCCESS(rv, rv);
 
     // moz_favicons.
     rv = mMainConn->ExecuteSimpleSQL(CREATE_MOZ_FAVICONS);
     NS_ENSURE_SUCCESS(rv, rv);
 
     // moz_anno_attributes.
     rv = mMainConn->ExecuteSimpleSQL(CREATE_MOZ_ANNO_ATTRIBUTES);
     NS_ENSURE_SUCCESS(rv, rv);
@@ -1117,20 +1120,16 @@ Database::MigrateV7Up()
         "WHERE id IN ("
           "SELECT k.id "
           "FROM moz_keywords k "
           "LEFT OUTER JOIN moz_bookmarks b "
           "ON b.keyword_id = k.id "
           "WHERE b.id IS NULL"
         ")"));
     NS_ENSURE_SUCCESS(rv, rv);
-
-    // Now we create our trigger
-    rv = mMainConn->ExecuteSimpleSQL(CREATE_KEYWORD_VALIDITY_TRIGGER);
-    NS_ENSURE_SUCCESS(rv, rv);
   }
 
   // Add the moz_inputhistory table, if missing.
   bool tableExists = false;
   rv = mMainConn->TableExists(NS_LITERAL_CSTRING("moz_inputhistory"),
                               &tableExists);
   NS_ENSURE_SUCCESS(rv, rv);
   if (!tableExists) {
@@ -1357,16 +1356,42 @@ Database::MigrateV14Up()
 
     // And now we can make the column unique.
     rv = mMainConn->ExecuteSimpleSQL(CREATE_IDX_MOZ_FAVICONS_GUID);
     NS_ENSURE_SUCCESS(rv, rv);
   }
   return NS_OK;
 }
 
+nsresult
+Database::MigrateV15Up()
+{
+  MOZ_ASSERT(NS_IsMainThread());
+
+  // Drop moz_bookmarks_beforedelete_v1_trigger, since it's more expensive than
+  // useful.
+  nsresult rv = mMainConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "DROP TRIGGER IF EXISTS moz_bookmarks_beforedelete_v1_trigger"
+  ));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  // Remove any orphan keywords.
+  rv = mMainConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "DELETE FROM moz_keywords "
+    "WHERE NOT EXISTS ( "
+      "SELECT id "
+      "FROM moz_bookmarks "
+      "WHERE keyword_id = moz_keywords.id "
+    ")"
+  ));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  return NS_OK;
+}
+
 void
 Database::Shutdown()
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(!mShuttingDown);
 
   mMainThreadStatements.FinalizeStatements();
   mMainThreadAsyncStatements.FinalizeStatements();
--- a/toolkit/components/places/Database.h
+++ b/toolkit/components/places/Database.h
@@ -42,17 +42,17 @@
 #include "nsWeakReference.h"
 #include "nsIInterfaceRequestorUtils.h"
 #include "nsIObserver.h"
 #include "mozilla/storage.h"
 #include "mozilla/storage/StatementCache.h"
 
 // This is the schema version. Update it at any schema change and add a
 // corresponding migrateVxx method below.
-#define DATABASE_SCHEMA_VERSION 14
+#define DATABASE_SCHEMA_VERSION 15
 
 // Fired after Places inited.
 #define TOPIC_PLACES_INIT_COMPLETE "places-init-complete"
 // Fired when initialization fails due to a locked database.
 #define TOPIC_DATABASE_LOCKED "places-database-locked"
 // This topic is received when the profile is about to be lost.  Places does
 // initial shutdown work and notifies TOPIC_PLACES_SHUTDOWN to all listeners.
 // Any shutdown work that requires the Places APIs should happen here.
@@ -285,16 +285,17 @@ protected:
    */
   nsresult MigrateV7Up();
   nsresult MigrateV8Up();
   nsresult MigrateV9Up();
   nsresult MigrateV10Up();
   nsresult MigrateV11Up();
   nsresult MigrateV13Up();
   nsresult MigrateV14Up();
+  nsresult MigrateV15Up();
 
   nsresult CheckAndUpdateGUIDs();
 
 private:
   ~Database();
 
   /**
    * Singleton getter, invoked by class instantiation.
--- a/toolkit/components/places/nsNavBookmarks.cpp
+++ b/toolkit/components/places/nsNavBookmarks.cpp
@@ -2581,21 +2581,46 @@ nsNavBookmarks::SetItemIndex(PRInt64 aIt
 
   return NS_OK;
 }
 
 
 nsresult
 nsNavBookmarks::UpdateKeywordsHashForRemovedBookmark(PRInt64 aItemId)
 {
-  nsAutoString kw;
-  if (NS_SUCCEEDED(GetKeywordForBookmark(aItemId, kw)) && !kw.IsEmpty()) {
+  nsAutoString keyword;
+  if (NS_SUCCEEDED(GetKeywordForBookmark(aItemId, keyword)) &&
+      !keyword.IsEmpty()) {
     nsresult rv = EnsureKeywordsHash();
     NS_ENSURE_SUCCESS(rv, rv);
     mBookmarkToKeywordHash.Remove(aItemId);
+
+    // If the keyword is unused, remove it from the database.
+    keywordSearchData searchData;
+    searchData.keyword.Assign(keyword);
+    searchData.itemId = -1;
+    mBookmarkToKeywordHash.EnumerateRead(SearchBookmarkForKeyword, &searchData);
+    if (searchData.itemId == -1) {
+      nsCOMPtr<mozIStorageAsyncStatement> stmt = mDB->GetAsyncStatement(
+        "DELETE FROM moz_keywords "
+        "WHERE keyword = :keyword "
+        "AND NOT EXISTS ( "
+          "SELECT id "
+          "FROM moz_bookmarks "
+          "WHERE keyword_id = moz_keywords.id "
+        ")"
+      );
+      NS_ENSURE_STATE(stmt);
+
+      rv = stmt->BindStringByName(NS_LITERAL_CSTRING("keyword"), keyword);
+      NS_ENSURE_SUCCESS(rv, rv);
+      nsCOMPtr<mozIStoragePendingStatement> pendingStmt;
+      rv = stmt->ExecuteAsync(nsnull, getter_AddRefs(pendingStmt));
+      NS_ENSURE_SUCCESS(rv, rv);
+    }
   }
   return NS_OK;
 }
 
 
 NS_IMETHODIMP
 nsNavBookmarks::SetKeywordForBookmark(PRInt64 aBookmarkId,
                                       const nsAString& aUserCasedKeyword)
--- a/toolkit/components/places/nsPlacesTriggers.h
+++ b/toolkit/components/places/nsPlacesTriggers.h
@@ -45,37 +45,16 @@
 /**
  * Exclude these visit types:
  *  0 - invalid
  *  4 - EMBED
  *  7 - DOWNLOAD
  *  7 - FRAMED_LINK
  **/
 #define EXCLUDED_VISIT_TYPES "0, 4, 7, 8"
-/**
- * Trigger checks to ensure that at least one bookmark is still using a keyword
- * when any bookmark is deleted.  If there are no more bookmarks using it, the
- * keyword is deleted.
- */
-#define CREATE_KEYWORD_VALIDITY_TRIGGER NS_LITERAL_CSTRING( \
-  "CREATE TRIGGER moz_bookmarks_beforedelete_v1_trigger " \
-  "BEFORE DELETE ON moz_bookmarks FOR EACH ROW " \
-  "WHEN OLD.keyword_id NOT NULL " \
-  "BEGIN " \
-    "DELETE FROM moz_keywords " \
-    "WHERE id = OLD.keyword_id " \
-    "AND NOT EXISTS ( " \
-      "SELECT id " \
-      "FROM moz_bookmarks " \
-      "WHERE keyword_id = OLD.keyword_id " \
-      "AND id <> OLD.id " \
-      "LIMIT 1 " \
-    ");" \
-  "END" \
-)
 
 /**
  * This triggers update visit_count and last_visit_date based on historyvisits
  * table changes.
  */
 #define CREATE_HISTORYVISITS_AFTERINSERT_TRIGGER NS_LITERAL_CSTRING( \
   "CREATE TEMP TRIGGER moz_historyvisits_afterinsert_v2_trigger " \
   "AFTER INSERT ON moz_historyvisits FOR EACH ROW " \
--- a/toolkit/components/places/tests/head_common.js
+++ b/toolkit/components/places/tests/head_common.js
@@ -30,17 +30,17 @@
  * use your version of this file under the terms of the MPL, indicate your
  * decision by deleting the provisions above and replace them with the notice
  * and other provisions required by the GPL or the LGPL. If you do not delete
  * the provisions above, a recipient may use your version of this file under
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
 
-const CURRENT_SCHEMA_VERSION = 14;
+const CURRENT_SCHEMA_VERSION = 15;
 
 const NS_APP_USER_PROFILE_50_DIR = "ProfD";
 const NS_APP_PROFILE_DIR_STARTUP = "ProfDS";
 const NS_APP_BOOKMARKS_50_FILE = "BMarks";
 
 // Shortcuts to transitions type.
 const TRANSITION_LINK = Ci.nsINavHistoryService.TRANSITION_LINK;
 const TRANSITION_TYPED = Ci.nsINavHistoryService.TRANSITION_TYPED;