Bug 572030 - Use a memory cache for keywords associations. r=dietrich a=sdwilsh

This commit is contained in:
Marco Bonardo 2010-08-09 17:59:43 +02:00
parent a81ecb4fab
commit 11be4ff865
3 changed files with 293 additions and 114 deletions

View File

@ -61,6 +61,8 @@
#include "mozilla/FunctionTimer.h"
#define BOOKMARKS_TO_KEYWORDS_INITIAL_CACHE_SIZE 64
const PRInt32 nsNavBookmarks::kFindBookmarksIndex_ID = 0;
const PRInt32 nsNavBookmarks::kFindBookmarksIndex_Type = 1;
const PRInt32 nsNavBookmarks::kFindBookmarksIndex_PlaceID = 2;
@ -94,6 +96,31 @@ PLACES_FACTORY_SINGLETON_IMPLEMENTATION(nsNavBookmarks, gBookmarksService)
#define GUID_ANNO NS_LITERAL_CSTRING("placesInternal/GUID")
#define READ_ONLY_ANNO NS_LITERAL_CSTRING("placesInternal/READ_ONLY")
namespace {
struct keywordSearchData
{
PRInt64 itemId;
nsString keyword;
};
PLDHashOperator
SearchBookmarkForKeyword(nsTrimInt64HashKey::KeyType aKey,
const nsString aValue,
void* aUserArg)
{
keywordSearchData* data = reinterpret_cast<keywordSearchData*>(aUserArg);
if (data->keyword.Equals(aValue)) {
data->itemId = aKey;
return PL_DHASH_STOP;
}
return PL_DHASH_NEXT;
}
} // Anonymous namespace.
nsNavBookmarks::nsNavBookmarks() : mItemCount(0)
, mRoot(0)
, mBookmarksRoot(0)
@ -336,12 +363,6 @@ nsNavBookmarks::GetStatement(const nsCOMPtr<mozIStorageStatement>& aStmt)
RETURN_IF_STMT(mDBSetItemIndex, NS_LITERAL_CSTRING(
"UPDATE moz_bookmarks SET position = :item_index WHERE id = :item_id"));
// Get keyword text for bookmark id.
RETURN_IF_STMT(mDBGetKeywordForBookmark, NS_LITERAL_CSTRING(
"SELECT k.keyword FROM moz_bookmarks b "
"JOIN moz_keywords k ON k.id = b.keyword_id "
"WHERE b.id = :item_id"));
// Get keyword text for bookmarked URI.
RETURN_IF_STMT(mDBGetKeywordForURI, NS_LITERAL_CSTRING(
"SELECT k.keyword "
@ -356,19 +377,6 @@ nsNavBookmarks::GetStatement(const nsCOMPtr<mozIStorageStatement>& aStmt)
"JOIN moz_bookmarks b ON b.fk = h.id "
"JOIN moz_keywords k ON k.id = b.keyword_id"));
// Get URI for keyword.
RETURN_IF_STMT(mDBGetURIForKeyword, NS_LITERAL_CSTRING(
"SELECT url FROM moz_keywords k "
"JOIN moz_bookmarks b ON b.keyword_id = k.id "
"JOIN moz_places_temp h ON b.fk = h.id "
"WHERE k.keyword = :keyword "
"UNION ALL "
"SELECT url FROM moz_keywords k "
"JOIN moz_bookmarks b ON b.keyword_id = k.id "
"JOIN moz_places h ON b.fk = h.id "
"WHERE k.keyword = :keyword "
"LIMIT 1"));
RETURN_IF_STMT(mDBAdjustPosition, NS_LITERAL_CSTRING(
"UPDATE moz_bookmarks SET position = position + :delta "
"WHERE parent = :parent "
@ -495,8 +503,6 @@ nsNavBookmarks::FinalizeStatements() {
mDBSetItemLastModified,
mDBSetItemIndex,
mDBGetKeywordForURI,
mDBGetKeywordForBookmark,
mDBGetURIForKeyword,
mDBAdjustPosition,
mDBRemoveItem,
mDBGetLastChildId,
@ -1120,6 +1126,9 @@ nsNavBookmarks::RemoveItem(PRInt64 aItemId)
NS_ENSURE_TRUE(history, NS_ERROR_OUT_OF_MEMORY);
rv = history->UpdateFrecency(placeId, IsRealBookmark(placeId));
NS_ENSURE_SUCCESS(rv, rv);
rv = UpdateKeywordsHashForRemovedBookmark(aItemId);
NS_ENSURE_SUCCESS(rv, rv);
}
NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
@ -1688,6 +1697,9 @@ nsNavBookmarks::RemoveFolderChildren(PRInt64 aFolderId)
NS_ENSURE_TRUE(history, NS_ERROR_OUT_OF_MEMORY);
rv = history->UpdateFrecency(placeId, IsRealBookmark(placeId));
NS_ENSURE_SUCCESS(rv, rv);
rv = UpdateKeywordsHashForRemovedBookmark(child.itemId);
NS_ENSURE_SUCCESS(rv, rv);
}
}
@ -2746,96 +2758,95 @@ nsNavBookmarks::SetItemIndex(PRInt64 aItemId, PRInt32 aNewIndex)
}
nsresult
nsNavBookmarks::UpdateKeywordsHashForRemovedBookmark(PRInt64 aItemId)
{
nsAutoString kw;
if (NS_SUCCEEDED(GetKeywordForBookmark(aItemId, kw)) && !kw.IsEmpty()) {
nsresult rv = EnsureKeywordsHash();
NS_ENSURE_SUCCESS(rv, rv);
mBookmarkToKeywordHash.Remove(aItemId);
}
return NS_OK;
}
NS_IMETHODIMP
nsNavBookmarks::SetKeywordForBookmark(PRInt64 aBookmarkId,
const nsAString& aKeyword)
{
NS_ENSURE_ARG_MIN(aBookmarkId, 1);
mozStorageTransaction transaction(mDBConn, PR_FALSE);
nsresult rv;
PRInt64 keywordId = 0;
if (!aKeyword.IsEmpty()) {
// Shortcuts are always lowercased internally.
nsAutoString kwd(aKeyword);
ToLowerCase(kwd);
// Attempt to find a pre-existing keyword record.
nsCOMPtr<mozIStorageStatement> getKeywordStmnt;
rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING(
"SELECT id from moz_keywords WHERE keyword = :keyword"),
getter_AddRefs(getKeywordStmnt));
NS_ENSURE_SUCCESS(rv, rv);
rv = getKeywordStmnt->BindStringByName(NS_LITERAL_CSTRING("keyword"), kwd);
NS_ENSURE_SUCCESS(rv, rv);
PRBool hasResult;
rv = getKeywordStmnt->ExecuteStep(&hasResult);
NS_ENSURE_SUCCESS(rv, rv);
if (hasResult) {
rv = getKeywordStmnt->GetInt64(0, &keywordId);
NS_ENSURE_SUCCESS(rv, rv);
}
else {
// Create a new keyword record.
nsCOMPtr<mozIStorageStatement> addKeywordStmnt;
rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING(
"INSERT INTO moz_keywords (keyword) VALUES (:keyword)"),
getter_AddRefs(addKeywordStmnt));
NS_ENSURE_SUCCESS(rv, rv);
rv = addKeywordStmnt->BindStringByName(NS_LITERAL_CSTRING("keyword"), kwd);
NS_ENSURE_SUCCESS(rv, rv);
rv = addKeywordStmnt->Execute();
NS_ENSURE_SUCCESS(rv, rv);
nsCOMPtr<mozIStorageStatement> idStmt;
rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING(
"SELECT id "
"FROM moz_keywords "
"ORDER BY ROWID DESC "
"LIMIT 1"),
getter_AddRefs(idStmt));
NS_ENSURE_SUCCESS(rv, rv);
rv = idStmt->ExecuteStep(&hasResult);
NS_ENSURE_SUCCESS(rv, rv);
NS_ASSERTION(hasResult, "hasResult is false but the call succeeded?");
rv = idStmt->GetInt64(0, &keywordId);
NS_ENSURE_SUCCESS(rv, rv);
}
}
// Update bookmark record w/ the keyword's id or null.
nsCOMPtr<mozIStorageStatement> updateKeywordStmnt;
rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING(
"UPDATE moz_bookmarks SET keyword_id = :keyword_id, lastModified = :date "
"WHERE id = :item_id"),
getter_AddRefs(updateKeywordStmnt));
nsresult rv = EnsureKeywordsHash();
NS_ENSURE_SUCCESS(rv, rv);
if (keywordId > 0) {
rv = updateKeywordStmnt->BindInt64ByName(NS_LITERAL_CSTRING("keyword_id"),
keywordId);
// Shortcuts are always lowercased internally.
nsAutoString keyword(aKeyword);
ToLowerCase(keyword);
// Check if bookmark was already associated to a keyword.
nsAutoString oldKeyword;
rv = GetKeywordForBookmark(aBookmarkId, oldKeyword);
NS_ENSURE_SUCCESS(rv, rv);
// Trying to set the same value or to remove a nonexistent keyword is a no-op.
if (keyword.Equals(oldKeyword) || (keyword.IsEmpty() && oldKeyword.IsEmpty()))
return NS_OK;
mozStorageTransaction transaction(mDBConn, PR_FALSE);
nsCOMPtr<mozIStorageStatement> updateBookmarkStmt;
rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING(
"UPDATE moz_bookmarks "
"SET keyword_id = (SELECT id FROM moz_keywords WHERE keyword = :keyword), "
"lastModified = :date "
"WHERE id = :item_id "
), getter_AddRefs(updateBookmarkStmt));
NS_ENSURE_SUCCESS(rv, rv);
if (keyword.IsEmpty()) {
// Remove keyword association from the hash.
mBookmarkToKeywordHash.Remove(aBookmarkId);
rv = updateBookmarkStmt->BindNullByName(NS_LITERAL_CSTRING("keyword"));
}
else {
rv = updateKeywordStmnt->BindNullByName(NS_LITERAL_CSTRING("keyword_id"));
else {
// We are associating bookmark to a new keyword. Create a new keyword
// record if needed.
nsCOMPtr<mozIStorageStatement> newKeywordStmt;
rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING(
"INSERT OR IGNORE INTO moz_keywords (keyword) VALUES (:keyword)"
), getter_AddRefs(newKeywordStmt));
NS_ENSURE_SUCCESS(rv, rv);
rv = newKeywordStmt->BindStringByName(NS_LITERAL_CSTRING("keyword"),
keyword);
NS_ENSURE_SUCCESS(rv, rv);
rv = newKeywordStmt->Execute();
NS_ENSURE_SUCCESS(rv, rv);
// Add new keyword association to the hash, removing the old one if needed.
if (!oldKeyword.IsEmpty())
mBookmarkToKeywordHash.Remove(aBookmarkId);
mBookmarkToKeywordHash.Put(aBookmarkId, keyword);
rv = updateBookmarkStmt->BindStringByName(NS_LITERAL_CSTRING("keyword"), keyword);
}
NS_ENSURE_SUCCESS(rv, rv);
PRTime lastModified = PR_Now();
rv = updateKeywordStmnt->BindInt64ByName(NS_LITERAL_CSTRING("date"), lastModified);
rv = updateBookmarkStmt->BindInt64ByName(NS_LITERAL_CSTRING("date"),
lastModified);
NS_ENSURE_SUCCESS(rv, rv);
rv = updateKeywordStmnt->BindInt64ByName(NS_LITERAL_CSTRING("item_id"), aBookmarkId);
rv = updateBookmarkStmt->BindInt64ByName(NS_LITERAL_CSTRING("item_id"),
aBookmarkId);
NS_ENSURE_SUCCESS(rv, rv);
rv = updateKeywordStmnt->Execute();
rv = updateBookmarkStmt->Execute();
NS_ENSURE_SUCCESS(rv, rv);
rv = transaction.Commit();
NS_ENSURE_SUCCESS(rv, rv);
// Pass the new keyword to OnItemChanged.
NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
nsINavBookmarkObserver,
OnItemChanged(aBookmarkId, NS_LITERAL_CSTRING("keyword"),
PR_FALSE, NS_ConvertUTF16toUTF8(aKeyword),
PR_FALSE, NS_ConvertUTF16toUTF8(keyword),
lastModified, TYPE_BOOKMARK));
return NS_OK;
@ -2872,21 +2883,17 @@ nsNavBookmarks::GetKeywordForBookmark(PRInt64 aBookmarkId, nsAString& aKeyword)
NS_ENSURE_ARG_MIN(aBookmarkId, 1);
aKeyword.Truncate(0);
DECLARE_AND_ASSIGN_SCOPED_LAZY_STMT(stmt, mDBGetKeywordForBookmark);
nsresult rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("item_id"),
aBookmarkId);
nsresult rv = EnsureKeywordsHash();
NS_ENSURE_SUCCESS(rv, rv);
PRBool hasMore = PR_FALSE;
rv = stmt->ExecuteStep(&hasMore);
if (NS_FAILED(rv) || ! hasMore) {
nsAutoString keyword;
if (!mBookmarkToKeywordHash.Get(aBookmarkId, &keyword)) {
aKeyword.SetIsVoid(PR_TRUE);
return NS_OK; // not found: return void keyword string
}
else {
aKeyword.Assign(keyword);
}
// found, get the keyword
rv = stmt->GetString(0, aKeyword);
NS_ENSURE_SUCCESS(rv, rv);
return NS_OK;
}
@ -2899,24 +2906,57 @@ nsNavBookmarks::GetURIForKeyword(const nsAString& aKeyword, nsIURI** aURI)
*aURI = nsnull;
// Shortcuts are always lowercased internally.
nsAutoString kwd(aKeyword);
ToLowerCase(kwd);
nsAutoString keyword(aKeyword);
ToLowerCase(keyword);
DECLARE_AND_ASSIGN_SCOPED_LAZY_STMT(stmt, mDBGetURIForKeyword);
nsresult rv = stmt->BindStringByName(NS_LITERAL_CSTRING("keyword"), kwd);
nsresult rv = EnsureKeywordsHash();
NS_ENSURE_SUCCESS(rv, rv);
PRBool hasMore = PR_FALSE;
rv = stmt->ExecuteStep(&hasMore);
if (NS_FAILED(rv) || ! hasMore)
return NS_OK; // not found: leave URI null
keywordSearchData searchData;
searchData.keyword.Assign(aKeyword);
searchData.itemId = -1;
mBookmarkToKeywordHash.EnumerateRead(SearchBookmarkForKeyword, &searchData);
// found, get the URI
nsCAutoString spec;
rv = stmt->GetUTF8String(0, spec);
if (searchData.itemId == -1) {
// Not found.
return NS_OK;
}
rv = GetBookmarkURI(searchData.itemId, aURI);
NS_ENSURE_SUCCESS(rv, rv);
rv = NS_NewURI(aURI, spec);
return NS_OK;
}
nsresult
nsNavBookmarks::EnsureKeywordsHash() {
if (mBookmarkToKeywordHash.IsInitialized())
return NS_OK;
mBookmarkToKeywordHash.Init(BOOKMARKS_TO_KEYWORDS_INITIAL_CACHE_SIZE);
nsCOMPtr<mozIStorageStatement> stmt;
nsresult rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING(
"SELECT b.id, k.keyword "
"FROM moz_bookmarks b "
"JOIN moz_keywords k ON k.id = b.keyword_id "
), getter_AddRefs(stmt));
NS_ENSURE_SUCCESS(rv, rv);
PRBool hasMore;
while (NS_SUCCEEDED(stmt->ExecuteStep(&hasMore)) && hasMore) {
PRInt64 itemId;
rv = stmt->GetInt64(0, &itemId);
NS_ENSURE_SUCCESS(rv, rv);
nsAutoString keyword;
rv = stmt->GetString(1, keyword);
NS_ENSURE_SUCCESS(rv, rv);
rv = mBookmarkToKeywordHash.Put(itemId, keyword);
NS_ENSURE_SUCCESS(rv, rv);
}
return NS_OK;
}

View File

@ -381,8 +381,7 @@ private:
nsCOMPtr<mozIStorageStatement> mDBSetItemLastModified;
nsCOMPtr<mozIStorageStatement> mDBSetItemIndex;
nsCOMPtr<mozIStorageStatement> mDBGetKeywordForURI;
nsCOMPtr<mozIStorageStatement> mDBGetKeywordForBookmark;
nsCOMPtr<mozIStorageStatement> mDBGetURIForKeyword;
nsCOMPtr<mozIStorageStatement> mDBGetBookmarksToKeywords;
nsCOMPtr<mozIStorageStatement> mDBAdjustPosition;
nsCOMPtr<mozIStorageStatement> mDBRemoveItem;
nsCOMPtr<mozIStorageStatement> mDBGetLastChildId;
@ -449,6 +448,21 @@ private:
nsCategoryCache<nsINavBookmarkObserver> mCacheObservers;
bool mShuttingDown;
/**
* Always call EnsureKeywordsHash() and check it for errors before actually
* using the hash. Internal keyword methods are already doing that.
*/
nsresult EnsureKeywordsHash();
nsDataHashtable<nsTrimInt64HashKey, nsString> mBookmarkToKeywordHash;
/**
* This function must be called every time a bookmark is removed.
*
* @param aURI
* Uri to test.
*/
nsresult UpdateKeywordsHashForRemovedBookmark(PRInt64 aItemId);
};
struct nsBookmarksUpdateBatcher

View File

@ -0,0 +1,125 @@
/* -*- Mode: Java; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
/* vim:set ts=2 sw=2 sts=2 et: */
/* ***** BEGIN LICENSE BLOCK *****
* Version: MPL 1.1/GPL 2.0/LGPL 2.1
*
* The contents of this file are subject to the Mozilla Public License Version
* 1.1 (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
* http://www.mozilla.org/MPL/
*
* Software distributed under the License is distributed on an "AS IS" basis,
* WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
* for the specific language governing rights and limitations under the
* License.
*
* The Original Code is Places Test Code.
*
* The Initial Developer of the Original Code is the Mozilla Foundation.
* Portions created by the Initial Developer are Copyright (C) 2010
* the Initial Developer. All Rights Reserved.
*
* Contributor(s):
* Marco Bonardo <mak77@bonardo.net> (Original Author)
*
* Alternatively, the contents of this file may be used under the terms of
* either the GNU General Public License Version 2 or later (the "GPL"), or
* the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
* in which case the provisions of the GPL or the LGPL are applicable instead
* of those above. If you wish to allow use of your version of this file only
* under the terms of either the GPL or the LGPL, and not to allow others to
* 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 ***** */
let bs = PlacesUtils.bookmarks;
let db = DBConn();
function check_keyword(aItemId, aExpectedBookmarkKeyword, aExpectedURIKeyword) {
if (aItemId) {
print("Check keyword for bookmark");
do_check_eq(bs.getKeywordForBookmark(aItemId), aExpectedBookmarkKeyword);
print("Check keyword for uri");
let uri = bs.getBookmarkURI(aItemId);
do_check_eq(bs.getKeywordForURI(uri), aExpectedURIKeyword);
print("Check uri for keyword");
// This API can't tell which uri the user wants, so it returns a random one.
if (aExpectedURIKeyword)
do_check_true(/http:\/\/test[0-9]\.mozilla\.org/.test(bs.getURIForKeyword(aExpectedURIKeyword).spec));
}
else {
stmt = db.createStatement(
"SELECT id FROM moz_keywords WHERE keyword = :keyword"
);
stmt.params.keyword = aExpectedBookmarkKeyword;
try {
do_check_false(stmt.executeStep());
} finally {
stmt.finalize();
}
}
print("Check there are no orphan database entries");
let stmt = db.createStatement(
"SELECT b.id FROM moz_bookmarks b "
+ "LEFT JOIN moz_keywords k ON b.keyword_id = k.id "
+ "WHERE keyword_id NOTNULL AND k.id ISNULL"
);
try {
do_check_false(stmt.executeStep());
} finally {
stmt.finalize();
}
}
function run_test() {
print("Check that leyword does not exist");
do_check_eq(bs.getURIForKeyword("keyword"), null);
let folderId = bs.createFolder(PlacesUtils.unfiledBookmarksFolderId,
"folder", bs.DEFAULT_INDEX);
print("Add a bookmark with a keyword");
let itemId1 = bs.insertBookmark(folderId,
uri("http://test1.mozilla.org/"),
bs.DEFAULT_INDEX,
"test1");
check_keyword(itemId1, null, null);
bs.setKeywordForBookmark(itemId1, "keyword");
check_keyword(itemId1, "keyword", "keyword");
print("Add another bookmark with the same uri, should not inherit keyword.");
let itemId1_bis = bs.insertBookmark(folderId,
uri("http://test1.mozilla.org/"),
bs.DEFAULT_INDEX,
"test1_bis");
check_keyword(itemId1_bis, null, "keyword");
print("Set same keyword on another bookmark with a different uri.");
let itemId2 = bs.insertBookmark(folderId,
uri("http://test2.mozilla.org/"),
bs.DEFAULT_INDEX,
"test2");
check_keyword(itemId2, null, null);
bs.setKeywordForBookmark(itemId2, "keyword");
check_keyword(itemId1, "keyword", "keyword");
check_keyword(itemId1_bis, null, "keyword");
check_keyword(itemId2, "keyword", "keyword");
print("Remove a bookmark with a keyword, it should not be removed from others");
bs.removeItem(itemId2);
check_keyword(itemId1, "keyword", "keyword");
print("Remove a folder containing bookmarks with keywords");
// Keyword should be removed as well.
bs.removeItem(folderId);
check_keyword(null, "keyword");
}