From d4274613f09241d5d398ad229b2d1c5ac31bdccf Mon Sep 17 00:00:00 2001 From: Marco Bonardo Date: Tue, 16 Nov 2010 13:44:05 +0100 Subject: [PATCH] Bug 612281 - Remove some unnecessary reads from visits and icons addition. r=sdwilsh a=blocking --- .../places/src/AsyncFaviconHelpers.cpp | 27 ++- toolkit/components/places/src/History.cpp | 186 ++++++++++-------- toolkit/components/places/src/nsNavHistory.h | 2 + 3 files changed, 121 insertions(+), 94 deletions(-) diff --git a/toolkit/components/places/src/AsyncFaviconHelpers.cpp b/toolkit/components/places/src/AsyncFaviconHelpers.cpp index 44b00ee14e0d..2d4cbb5ab4f4 100644 --- a/toolkit/components/places/src/AsyncFaviconHelpers.cpp +++ b/toolkit/components/places/src/AsyncFaviconHelpers.cpp @@ -774,24 +774,31 @@ AsyncAssociateIconToPage::Run() rv = stmt->Execute(); NS_ENSURE_SUCCESS(rv, rv); - // Get the new page id. - rv = FetchPageInfo(mFaviconSvc->mSyncStatements, mPage); - NS_ENSURE_SUCCESS(rv, rv); - mIcon.status |= ICON_STATUS_ASSOCIATED; } // Otherwise just associate the icon to the page, if needed. else if (mPage.iconId != mIcon.id) { - nsCOMPtr stmt = - mFaviconSvc->mSyncStatements.GetCachedStatement(NS_LITERAL_CSTRING( + nsCOMPtr stmt; + if (mPage.id) { + stmt = mFaviconSvc->mSyncStatements.GetCachedStatement(NS_LITERAL_CSTRING( "UPDATE moz_places SET favicon_id = :icon_id WHERE id = :page_id" )); - NS_ENSURE_STATE(stmt); - mozStorageStatementScoper scoper(stmt); + NS_ENSURE_STATE(stmt); + rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("page_id"), mPage.id); + NS_ENSURE_SUCCESS(rv, rv); + } + else { + stmt = mFaviconSvc->mSyncStatements.GetCachedStatement(NS_LITERAL_CSTRING( + "UPDATE moz_places SET favicon_id = :icon_id WHERE url = :page_url" + )); + NS_ENSURE_STATE(stmt); + rv = URIBinder::Bind(stmt, NS_LITERAL_CSTRING("page_url"), mPage.spec); + NS_ENSURE_SUCCESS(rv, rv); + } rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("icon_id"), mIcon.id); NS_ENSURE_SUCCESS(rv, rv); - rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("page_id"), mPage.id); - NS_ENSURE_SUCCESS(rv, rv); + + mozStorageStatementScoper scoper(stmt); rv = stmt->Execute(); NS_ENSURE_SUCCESS(rv, rv); diff --git a/toolkit/components/places/src/History.cpp b/toolkit/components/places/src/History.cpp index 7bc003d41e38..6a42e6bbc1c1 100644 --- a/toolkit/components/places/src/History.cpp +++ b/toolkit/components/places/src/History.cpp @@ -331,68 +331,8 @@ public: NS_PRECONDITION(!NS_IsMainThread(), "This should not be called on the main thread"); - 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 stmt = - mHistory->syncStatements.GetCachedStatement( - "UPDATE moz_places " - "SET hidden = :hidden, typed = :typed " - "WHERE id = :page_id " - ); - NS_ENSURE_STATE(stmt); - mozStorageStatementScoper scoper(stmt); - - 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 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; - 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); - rv = stmt->BindInt32ByName(NS_LITERAL_CSTRING("hidden"), mPlace.hidden); - NS_ENSURE_SUCCESS(rv, rv); - - rv = stmt->Execute(); - NS_ENSURE_SUCCESS(rv, rv); - - // Now, we need to get the id of what we just added. - bool added = FetchPageInfo(mPlace); - NS_ASSERTION(added, "not known after adding the place!"); - } - // If we had a referrer, we want to know about its last visit to put this // new visit into the same session. if (mReferrer.uri) { @@ -411,7 +351,51 @@ public: } } - nsresult rv = AddVisit(mPlace, mReferrer); + mozStorageTransaction transaction(mDBConn, PR_FALSE, + mozIStorageConnection::TRANSACTION_IMMEDIATE); + nsresult rv; + nsCOMPtr stmt; + // 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!"); + + stmt = mHistory->syncStatements.GetCachedStatement( + "UPDATE moz_places " + "SET hidden = :hidden, typed = :typed " + "WHERE id = :page_id " + ); + NS_ENSURE_STATE(stmt); + rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("page_id"), mPlace.placeId); + 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!"); + + stmt = mHistory->syncStatements.GetCachedStatement( + "INSERT INTO moz_places " + "(url, rev_host, hidden, typed) " + "VALUES (:page_url, :rev_host, :hidden, :typed) " + ); + NS_ENSURE_STATE(stmt); + nsAutoString revHost; + nsresult rv = GetReversedHostname(mPlace.uri, revHost); + NS_ENSURE_SUCCESS(rv, rv); + rv = stmt->BindStringByName(NS_LITERAL_CSTRING("rev_host"), revHost); + NS_ENSURE_SUCCESS(rv, rv); + rv = URIBinder::Bind(stmt, NS_LITERAL_CSTRING("page_url"), mPlace.uri); + NS_ENSURE_SUCCESS(rv, rv); + } + 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); + + mozStorageStatementScoper scoper(stmt); + rv = stmt->Execute(); + NS_ENSURE_SUCCESS(rv, rv); + + rv = AddVisit(mPlace, mReferrer); NS_ENSURE_SUCCESS(rv, rv); rv = UpdateFrecency(mPlace); @@ -560,20 +544,30 @@ private: nsresult AddVisit(VisitData& _place, const VisitData& aReferrer) { - nsCOMPtr stmt = - mHistory->syncStatements.GetCachedStatement( + nsresult rv; + nsCOMPtr stmt; + if (_place.placeId) { + 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); - - 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_STATE(stmt); + rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("page_id"), mPlace.placeId); + NS_ENSURE_SUCCESS(rv, rv); + } + else { + stmt = mHistory->syncStatements.GetCachedStatement( + "INSERT INTO moz_historyvisits " + "(from_visit, place_id, visit_date, visit_type, session) " + "VALUES (:from_visit, (SELECT id FROM moz_places WHERE url = :page_url), :visit_date, :visit_type, :session) " + ); + NS_ENSURE_STATE(stmt); + rv = URIBinder::Bind(stmt, NS_LITERAL_CSTRING("page_url"), _place.uri); + NS_ENSURE_SUCCESS(rv, rv); + } + rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("from_visit"), + aReferrer.visitId); NS_ENSURE_SUCCESS(rv, rv); rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("visit_date"), _place.visitTime); @@ -589,6 +583,7 @@ private: _place.sessionId); NS_ENSURE_SUCCESS(rv, rv); + mozStorageStatementScoper scoper(stmt); rv = stmt->Execute(); NS_ENSURE_SUCCESS(rv, rv); @@ -608,37 +603,60 @@ private: */ nsresult UpdateFrecency(const VisitData& aPlace) { + nsresult rv; { // First, set our frecency to the proper value. - nsCOMPtr stmt = - mHistory->syncStatements.GetCachedStatement( + nsCOMPtr stmt; + if (aPlace.placeId) { + stmt = mHistory->syncStatements.GetCachedStatement( "UPDATE moz_places " "SET frecency = CALCULATE_FRECENCY(:page_id) " "WHERE id = :page_id" ); - NS_ENSURE_STATE(stmt); + NS_ENSURE_STATE(stmt); + rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("page_id"), mPlace.placeId); + NS_ENSURE_SUCCESS(rv, rv); + } + else { + stmt = mHistory->syncStatements.GetCachedStatement( + "UPDATE moz_places " + "SET frecency = CALCULATE_FRECENCY(id) " + "WHERE url = :page_url" + ); + NS_ENSURE_STATE(stmt); + rv = URIBinder::Bind(stmt, NS_LITERAL_CSTRING("page_url"), aPlace.uri); + NS_ENSURE_SUCCESS(rv, rv); + } mozStorageStatementScoper scoper(stmt); - nsresult rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("page_id"), - aPlace.placeId); - NS_ENSURE_SUCCESS(rv, rv); rv = stmt->Execute(); NS_ENSURE_SUCCESS(rv, rv); } { // Now, we need to mark the page as not hidden if the frecency is now // nonzero. - nsCOMPtr stmt = - mHistory->syncStatements.GetCachedStatement( + nsCOMPtr stmt; + if (aPlace.placeId) { + stmt = mHistory->syncStatements.GetCachedStatement( "UPDATE moz_places " "SET hidden = 0 " "WHERE id = :page_id AND frecency <> 0" ); - NS_ENSURE_STATE(stmt); - mozStorageStatementScoper scoper(stmt); + NS_ENSURE_STATE(stmt); + rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("page_id"), mPlace.placeId); + NS_ENSURE_SUCCESS(rv, rv); + } + else { + stmt = mHistory->syncStatements.GetCachedStatement( + "UPDATE moz_places " + "SET hidden = 0 " + "WHERE url = :page_url AND frecency <> 0" + ); + NS_ENSURE_STATE(stmt); + rv = URIBinder::Bind(stmt, NS_LITERAL_CSTRING("page_url"), aPlace.uri); + NS_ENSURE_SUCCESS(rv, rv); + } - nsresult rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("page_id"), - aPlace.placeId); - NS_ENSURE_SUCCESS(rv, rv); + mozStorageStatementScoper scoper(stmt); rv = stmt->Execute(); NS_ENSURE_SUCCESS(rv, rv); } diff --git a/toolkit/components/places/src/nsNavHistory.h b/toolkit/components/places/src/nsNavHistory.h index 66f29635e1e2..441e4c839150 100644 --- a/toolkit/components/places/src/nsNavHistory.h +++ b/toolkit/components/places/src/nsNavHistory.h @@ -519,6 +519,8 @@ public: case DB_VISITS_FOR_FRECENCY: return NS_IsMainThread() ? mDBVisitsForFrecency : mDBAsyncThreadVisitsForFrecency; + default: + NS_NOTREACHED("Trying to handle an unknown statement"); } return nsnull; }