Bug 599969 - Do not use steps for async visit adding
authorShawn Wilsher <me@shawnwilsher.com>
Mon, 08 Nov 2010 11:43:46 -0800
changeset 59322 3b477671678ac5e2869ae5d36526a9f38b0aa2ce
parent 59321 7d3efce2d46d8f8be71d6120c4fead3b6bb349e4
child 59323 47cdf340a18236cf9fd1b5083f7c66c92fbcaab9
push id1
push usershaver@mozilla.com
push dateTue, 04 Jan 2011 17:58:04 +0000
bugs599969
milestone2.0b8pre
Bug 599969 - Do not use steps for async visit adding Part 3 - Use the statement cache from storage to do less work on the background thread. r=mak
toolkit/components/places/src/History.cpp
toolkit/components/places/src/History.h
--- a/toolkit/components/places/src/History.cpp
+++ b/toolkit/components/places/src/History.cpp
@@ -340,45 +340,46 @@ private:
  * Adds a visit to the database.
  */
 class InsertVisitedURI : public nsRunnable
 {
 public:
   /**
    * Adds a visit to the database asynchronously.
    *
+   * @param aConnection
+   *        The database connection to use for these operations.
    * @param aPlace
    *        The location to record a visit.
    * @param [optional] aReferrer
    *        The page that "referred" us to aPlace.
    */
-  static nsresult Start(VisitData& aPlace,
+  static nsresult Start(mozIStorageConnection* aConnection,
+                        VisitData& aPlace,
                         nsIURI* aReferrer = nsnull)
   {
     NS_PRECONDITION(NS_IsMainThread(),
                     "This should be called on the main thread");
 
-    nsRefPtr<InsertVisitedURI> event = new InsertVisitedURI(aPlace, aReferrer);
+    nsRefPtr<InsertVisitedURI> event =
+      new InsertVisitedURI(aConnection, aPlace, aReferrer);
     NS_ENSURE_TRUE(event, NS_ERROR_OUT_OF_MEMORY);
 
-    nsNavHistory* navhistory = nsNavHistory::GetHistoryService();
-    NS_ENSURE_TRUE(navhistory, NS_ERROR_OUT_OF_MEMORY);
-    nsresult rv = navhistory->GetDBConnection(getter_AddRefs(event->mDBConn));
-    NS_ENSURE_SUCCESS(rv, rv);
-
     // Speculatively get a new session id for our visit.  While it is true that
     // we will use the session id from the referrer if the visit was "recent"
     // enough, we cannot call this method off of the main thread, so we have to
     // consume an id now.
+    nsNavHistory* navhistory = nsNavHistory::GetHistoryService();
+    NS_ENSURE_TRUE(navhistory, NS_ERROR_UNEXPECTED);
     event->mPlace.sessionId = navhistory->GetNewSessionID();
 
     // Get the target thread, and then start the work!
-    nsCOMPtr<nsIEventTarget> target = do_GetInterface(event->mDBConn);
-    NS_ENSURE_TRUE(target, NS_ERROR_OUT_OF_MEMORY);
-    rv = target->Dispatch(event, NS_DISPATCH_NORMAL);
+    nsCOMPtr<nsIEventTarget> target = do_GetInterface(aConnection);
+    NS_ENSURE_TRUE(target, NS_ERROR_UNEXPECTED);
+    nsresult rv = target->Dispatch(event, NS_DISPATCH_NORMAL);
     NS_ENSURE_SUCCESS(rv, rv);
 
     return NS_OK;
   }
 
   NS_IMETHOD Run()
   {
     NS_PRECONDITION(!NS_IsMainThread(),
@@ -387,48 +388,51 @@ public:
     mozStorageTransaction transaction(mDBConn, PR_FALSE,
                                       mozIStorageConnection::TRANSACTION_IMMEDIATE);
     bool known = FetchPageInfo(mPlace);
 
     // If the page was in moz_places, we need to update the entry.
     if (known) {
       NS_ASSERTION(mPlace.placeId > 0, "must have a valid place id!");
 
-      nsCOMPtr<mozIStorageStatement> stmt;
-      nsresult rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING(
-        "UPDATE moz_places "
-        "SET hidden = :hidden, typed = :typed "
-        "WHERE id = :page_id "
-      ), getter_AddRefs(stmt));
-      NS_ENSURE_SUCCESS(rv, rv);
+      nsCOMPtr<mozIStorageStatement> stmt =
+        mHistory->syncStatements.GetCachedStatement(
+          "UPDATE moz_places "
+          "SET hidden = :hidden, typed = :typed "
+          "WHERE id = :page_id "
+        );
+      NS_ENSURE_STATE(stmt);
+      mozStorageStatementScoper scoper(stmt);
 
-      rv = stmt->BindInt32ByName(NS_LITERAL_CSTRING("typed"), mPlace.typed);
+      nsresult rv = stmt->BindInt32ByName(NS_LITERAL_CSTRING("typed"),
+                                          mPlace.typed);
       NS_ENSURE_SUCCESS(rv, rv);
       rv = stmt->BindInt32ByName(NS_LITERAL_CSTRING("hidden"), mPlace.hidden);
       NS_ENSURE_SUCCESS(rv, rv);
       rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("page_id"), mPlace.placeId);
       NS_ENSURE_SUCCESS(rv, rv);
 
       rv = stmt->Execute();
       NS_ENSURE_SUCCESS(rv, rv);
     }
     // Otherwise, the page was not in moz_places, so now we have to add it.
     else {
       NS_ASSERTION(mPlace.placeId == 0, "should not have a valid place id!");
 
-      nsCOMPtr<mozIStorageStatement> stmt;
-      nsresult rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING(
-        "INSERT INTO moz_places "
-          "(url, rev_host, hidden, typed) "
-        "VALUES (:page_url, :rev_host, :hidden, :typed) "
-      ), getter_AddRefs(stmt));
-      NS_ENSURE_SUCCESS(rv, rv);
+      nsCOMPtr<mozIStorageStatement> stmt =
+        mHistory->syncStatements.GetCachedStatement(
+          "INSERT INTO moz_places "
+            "(url, rev_host, hidden, typed) "
+          "VALUES (:page_url, :rev_host, :hidden, :typed) "
+        );
+      NS_ENSURE_STATE(stmt);
+      mozStorageStatementScoper scoper(stmt);
 
       nsAutoString revHost;
-      rv = GetReversedHostname(mPlace.uri, revHost);
+      nsresult rv = GetReversedHostname(mPlace.uri, revHost);
       NS_ENSURE_SUCCESS(rv, rv);
 
       rv = URIBinder::Bind(stmt, NS_LITERAL_CSTRING("page_url"), mPlace.uri);
       NS_ENSURE_SUCCESS(rv, rv);
       rv = stmt->BindStringByName(NS_LITERAL_CSTRING("rev_host"), revHost);
       NS_ENSURE_SUCCESS(rv, rv);
       rv = stmt->BindInt32ByName(NS_LITERAL_CSTRING("typed"), mPlace.typed);
       NS_ENSURE_SUCCESS(rv, rv);
@@ -474,44 +478,49 @@ public:
     nsCOMPtr<nsIRunnable> event = new NotifyVisitObservers(mPlace, mReferrer);
     NS_ENSURE_TRUE(event, NS_ERROR_OUT_OF_MEMORY);
     rv = NS_DispatchToMainThread(event);
     NS_ENSURE_SUCCESS(rv, rv);
 
     return NS_OK;
   }
 private:
-  InsertVisitedURI(VisitData& aPlace,
+  InsertVisitedURI(mozIStorageConnection* aConnection,
+                   VisitData& aPlace,
                    nsIURI* aReferrer)
-  : mPlace(aPlace)
+  : mDBConn(aConnection)
+  , mPlace(aPlace)
+  , mHistory(History::GetService())
   {
     mReferrer.uri = aReferrer;
   }
 
   /**
    * Loads information about the page into _place from moz_places.
    *
    * @param _place
    *        The VisitData for the place we need to know information about.
    * @return true if the page was recorded in moz_places, false otherwise.
    */
   bool FetchPageInfo(VisitData& _place)
   {
     NS_PRECONDITION(_place.uri || _place.spec.Length(),
                     "must have a non-null uri or a non-empty spec!");
 
-    nsCOMPtr<mozIStorageStatement> stmt;
-    nsresult rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING(
-      "SELECT id, typed, hidden "
-      "FROM moz_places "
-      "WHERE url = :page_url "
-    ), getter_AddRefs(stmt));
-    NS_ENSURE_SUCCESS(rv, false);
+    nsCOMPtr<mozIStorageStatement> stmt =
+      mHistory->syncStatements.GetCachedStatement(
+        "SELECT id, typed, hidden "
+        "FROM moz_places "
+        "WHERE url = :page_url "
+      );
+    NS_ENSURE_TRUE(stmt, false);
+    mozStorageStatementScoper scoper(stmt);
 
-    rv = URIBinder::Bind(stmt, NS_LITERAL_CSTRING("page_url"), _place.uri);
+    nsresult rv = URIBinder::Bind(stmt, NS_LITERAL_CSTRING("page_url"),
+                                  _place.uri);
     NS_ENSURE_SUCCESS(rv, false);
 
     PRBool hasResult;
     rv = stmt->ExecuteStep(&hasResult);
     NS_ENSURE_SUCCESS(rv, false);
     if (!hasResult) {
       return false;
     }
@@ -552,26 +561,28 @@ private:
    *         visited, false otherwise.
    */
   bool FetchVisitInfo(VisitData& _place,
                       PRTime aThresholdStart = 0)
   {
     NS_PRECONDITION(_place.uri || _place.spec.Length(),
                     "must have a non-null uri or a non-empty spec!");
 
-    nsCOMPtr<mozIStorageStatement> stmt;
-    nsresult rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING(
-      "SELECT id, session, visit_date "
-      "FROM moz_historyvisits "
-      "WHERE place_id = (SELECT id FROM moz_places WHERE url = :page_url) "
-      "ORDER BY visit_date DESC "
-    ), getter_AddRefs(stmt));
-    NS_ENSURE_SUCCESS(rv, false);
+    nsCOMPtr<mozIStorageStatement> stmt =
+      mHistory->syncStatements.GetCachedStatement(
+        "SELECT id, session, visit_date "
+        "FROM moz_historyvisits "
+        "WHERE place_id = (SELECT id FROM moz_places WHERE url = :page_url) "
+        "ORDER BY visit_date DESC "
+      );
+    NS_ENSURE_TRUE(stmt, false);
+    mozStorageStatementScoper scoper(stmt);
 
-    rv = URIBinder::Bind(stmt, NS_LITERAL_CSTRING("page_url"), _place.uri);
+    nsresult rv = URIBinder::Bind(stmt, NS_LITERAL_CSTRING("page_url"),
+                                  _place.uri);
     NS_ENSURE_SUCCESS(rv, false);
 
     PRBool hasResult;
     rv = stmt->ExecuteStep(&hasResult);
     NS_ENSURE_SUCCESS(rv, false);
     if (!hasResult) {
       return false;
     }
@@ -599,26 +610,27 @@ private:
    * @param _place
    *        The VisitData for the place we need to know visit information about.
    * @param aReferrer
    *        A reference to the referrer's visit data.
    */
   nsresult AddVisit(VisitData& _place,
                     const VisitData& aReferrer)
   {
-    nsCOMPtr<mozIStorageStatement> stmt;
-    nsresult rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING(
-      "INSERT INTO moz_historyvisits "
-        "(from_visit, place_id, visit_date, visit_type, session) "
-      "VALUES (:from_visit, :page_id, :visit_date, :visit_type, :session) "
-    ), getter_AddRefs(stmt));
-    NS_ENSURE_SUCCESS(rv, rv);
+    nsCOMPtr<mozIStorageStatement> stmt =
+      mHistory->syncStatements.GetCachedStatement(
+        "INSERT INTO moz_historyvisits "
+          "(from_visit, place_id, visit_date, visit_type, session) "
+        "VALUES (:from_visit, :page_id, :visit_date, :visit_type, :session) "
+      );
+    NS_ENSURE_STATE(stmt);
+    mozStorageStatementScoper scoper(stmt);
 
-    rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("from_visit"),
-                               aReferrer.visitId);
+    nsresult rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("from_visit"),
+                                        aReferrer.visitId);
     NS_ENSURE_SUCCESS(rv, rv);
     rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("page_id"),
                                _place.placeId);
     NS_ENSURE_SUCCESS(rv, rv);
     rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("visit_date"),
                                _place.visitTime);
     NS_ENSURE_SUCCESS(rv, rv);
     PRInt32 transitionType = _place.transitionType;
@@ -646,53 +658,64 @@ private:
   /**
    * Updates the frecency, and possibly the hidden-ness of aPlace.
    *
    * @param aPlace
    *        The VisitData for the place we want to update.
    */
   nsresult UpdateFrecency(const VisitData& aPlace)
   {
-    // First, set our frecency to the proper value.
-    nsCOMPtr<mozIStorageStatement> stmt;
-    nsresult rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING(
-      "UPDATE moz_places "
-      "SET frecency = CALCULATE_FRECENCY(:page_id) "
-      "WHERE id = :page_id"
-    ), getter_AddRefs(stmt));
-    NS_ENSURE_SUCCESS(rv, rv);
+    { // First, set our frecency to the proper value.
+      nsCOMPtr<mozIStorageStatement> stmt =
+        mHistory->syncStatements.GetCachedStatement(
+          "UPDATE moz_places "
+          "SET frecency = CALCULATE_FRECENCY(:page_id) "
+          "WHERE id = :page_id"
+        );
+      NS_ENSURE_STATE(stmt);
+      mozStorageStatementScoper scoper(stmt);
 
-    rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("page_id"),
-                               aPlace.placeId);
-    NS_ENSURE_SUCCESS(rv, rv);
-    rv = stmt->Execute();
-    NS_ENSURE_SUCCESS(rv, rv);
+      nsresult rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("page_id"),
+                                          aPlace.placeId);
+      NS_ENSURE_SUCCESS(rv, rv);
+      rv = stmt->Execute();
+      NS_ENSURE_SUCCESS(rv, rv);
+    }
 
-    // Finally, we need to mark the page as not hidden if the frecency is now
-    // nonzero.
-    rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING(
-      "UPDATE moz_places "
-      "SET hidden = 0 "
-      "WHERE id = :page_id AND frecency <> 0"
-    ), getter_AddRefs(stmt));
-    NS_ENSURE_SUCCESS(rv, rv);
+    { // Now, we need to mark the page as not hidden if the frecency is now
+      // nonzero.
+      nsCOMPtr<mozIStorageStatement> stmt =
+        mHistory->syncStatements.GetCachedStatement(
+          "UPDATE moz_places "
+          "SET hidden = 0 "
+          "WHERE id = :page_id AND frecency <> 0"
+        );
+      NS_ENSURE_STATE(stmt);
+      mozStorageStatementScoper scoper(stmt);
 
-    rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("page_id"),
-                               aPlace.placeId);
-    NS_ENSURE_SUCCESS(rv, rv);
-    rv = stmt->Execute();
-    NS_ENSURE_SUCCESS(rv, rv);
+      nsresult rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("page_id"),
+                                          aPlace.placeId);
+      NS_ENSURE_SUCCESS(rv, rv);
+      rv = stmt->Execute();
+      NS_ENSURE_SUCCESS(rv, rv);
+    }
 
     return NS_OK;
   }
 
-  nsCOMPtr<mozIStorageConnection> mDBConn;
+  mozIStorageConnection* mDBConn;
 
   VisitData mPlace;
   VisitData mReferrer;
+
+  /**
+   * Strong reference to the History object because we do not want it to
+   * disappear out from under us.
+   */
+  nsRefPtr<History> mHistory;
 };
 
 /**
  * Fail-safe mechanism for ensuring that your task completes, no matter what.
  * Pass this around as an nsAutoPtr in your steps to guarantee that when all
  * your steps are finished, your task is finished.
  *
  * Be sure to use AppendTask to add your first step to the queue.
@@ -863,16 +886,17 @@ protected:
 
 ////////////////////////////////////////////////////////////////////////////////
 //// History
 
 History* History::gService = NULL;
 
 History::History()
 : mShuttingDown(false)
+, syncStatements(mDBConn)
 {
   NS_ASSERTION(!gService, "Ruh-roh!  This service has already been created!");
   gService = this;
 
   nsCOMPtr<nsIObserverService> os = mozilla::services::GetObserverService();
   NS_WARN_IF_FALSE(os, "Observer service was not found!");
   if (os) {
     (void)os->AddObserver(this, TOPIC_PLACES_SHUTDOWN, PR_FALSE);
@@ -972,21 +996,17 @@ mozIStorageAsyncStatement*
 History::GetIsVisitedStatement()
 {
   if (mIsVisitedStatement) {
     return mIsVisitedStatement;
   }
 
   // If we don't yet have a database connection, go ahead and clone it now.
   if (!mReadOnlyDBConn) {
-    nsNavHistory* history = nsNavHistory::GetHistoryService();
-    NS_ENSURE_TRUE(history, nsnull);
-
-    nsCOMPtr<mozIStorageConnection> dbConn;
-    (void)history->GetDBConnection(getter_AddRefs(dbConn));
+    mozIStorageConnection* dbConn = GetDBConn();
     NS_ENSURE_TRUE(dbConn, nsnull);
 
     (void)dbConn->Clone(PR_TRUE, getter_AddRefs(mReadOnlyDBConn));
     NS_ENSURE_TRUE(mReadOnlyDBConn, nsnull);
   }
 
   // Now we can create our cached statement.
   nsresult rv = mReadOnlyDBConn->CreateAsyncStatement(NS_LITERAL_CSTRING(
@@ -1022,16 +1042,32 @@ History::GetSingleton()
     gService = new History();
     NS_ENSURE_TRUE(gService, nsnull);
   }
 
   NS_ADDREF(gService);
   return gService;
 }
 
+mozIStorageConnection*
+History::GetDBConn()
+{
+  if (mDBConn) {
+    return mDBConn;
+  }
+
+  nsNavHistory* history = nsNavHistory::GetHistoryService();
+  NS_ENSURE_TRUE(history, nsnull);
+
+  nsresult rv = history->GetDBConnection(getter_AddRefs(mDBConn));
+  NS_ENSURE_SUCCESS(rv, nsnull);
+
+  return mDBConn;
+}
+
 void
 History::StartNextTask()
 {
   if (mShuttingDown) {
     return;
   }
 
   nsCOMPtr<Step> nextTask =
@@ -1050,16 +1086,18 @@ History::Shutdown()
   mShuttingDown = true;
 
   while (mPendingVisits.PeekFront()) {
     nsCOMPtr<Step> deadTaskWalking =
       dont_AddRef(static_cast<Step*>(mPendingVisits.PopFront()));
   }
 
   // Clean up our statements and connection.
+  syncStatements.FinalizeStatements();
+
   if (mReadOnlyDBConn) {
     if (mIsVisitedStatement) {
       (void)mIsVisitedStatement->Finalize();
     }
     (void)mReadOnlyDBConn->AsyncClose(nsnull);
   }
 }
 
@@ -1145,17 +1183,20 @@ History::VisitURI(nsIURI* aURI,
   place.typed = place.transitionType == nsINavHistoryService::TRANSITION_TYPED;
   place.hidden =
     place.transitionType == nsINavHistoryService::TRANSITION_FRAMED_LINK ||
     place.transitionType == nsINavHistoryService::TRANSITION_EMBED ||
     redirected;
   place.visitTime = PR_Now();
   place.uri = aURI;
 
-  rv = InsertVisitedURI::Start(place, aLastVisitedURI);
+  mozIStorageConnection* dbConn = GetDBConn();
+  NS_ENSURE_STATE(dbConn);
+
+  rv = InsertVisitedURI::Start(dbConn, place, aLastVisitedURI);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Finally, notify that we've been visited.
   nsCOMPtr<nsIObserverService> obsService =
     mozilla::services::GetObserverService();
   if (obsService) {
     obsService->NotifyObservers(aURI, NS_LINK_VISITED_EVENT_TOPIC, nsnull);
   }
@@ -1333,16 +1374,16 @@ History::Observe(nsISupports* aSubject, 
   }
 
   return NS_OK;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 //// nsISupports
 
-NS_IMPL_ISUPPORTS2(
+NS_IMPL_THREADSAFE_ISUPPORTS2(
   History
 , IHistory
 , nsIObserver
 )
 
 } // namespace places
 } // namespace mozilla
--- a/toolkit/components/places/src/History.h
+++ b/toolkit/components/places/src/History.h
@@ -44,16 +44,17 @@
 #include "mozilla/dom/Link.h"
 #include "nsTHashtable.h"
 #include "nsString.h"
 #include "nsURIHashKey.h"
 #include "nsTArray.h"
 #include "nsDeque.h"
 #include "nsIObserver.h"
 #include "mozIStorageConnection.h"
+#include "mozilla/storage/StatementCache.h"
 
 namespace mozilla {
 namespace places {
 
 #define NS_HISTORYSERVICE_CID \
   {0x0937a705, 0x91a6, 0x417a, {0x82, 0x92, 0xb2, 0x2e, 0xb1, 0x0d, 0xa8, 0x6c}}
 
 class History : public IHistory
@@ -105,20 +106,39 @@ public:
   static History* GetService();
 
   /**
    * Obtains a pointer that has had AddRef called on it.  Used by the service
    * manager only.
    */
   static History* GetSingleton();
 
+  /**
+   * Statement cache that is used for background thread statements only.
+   */
+  storage::StatementCache<mozIStorageStatement> syncStatements;
+
 private:
   virtual ~History();
 
   /**
+   * Obtains a read-write database connection.
+   */
+  mozIStorageConnection* GetDBConn();
+
+  /**
+   * A read-write database connection used for adding history visits and setting
+   * a page's title.
+   *
+   * @note this should only be accessed by GetDBConn.
+   * @note this is the same connection as the one found on nsNavHistory.
+   */
+  nsCOMPtr<mozIStorageConnection> mDBConn;
+
+  /**
    * A read-only database connection used for checking if a URI is visited.
    *
    * @note this should only be accessed by GetIsVisistedStatement and Shutdown.
    */
   nsCOMPtr<mozIStorageConnection> mReadOnlyDBConn;
 
   /**
    * An asynchronous statement to query if a URI is visited or not.