Bug 683316 - DOMStorageImpl::GetKey performance regression, r=bz

dom/src/storage/Makefile.in

@@ -48,6 +48,7 @@ LIBXUL_LIBRARY = 1
 
 CPPSRCS = \
        nsDOMStorage.cpp \
+       nsDOMStorageBaseDB.cpp \
        nsDOMStorageDBWrapper.cpp \
        nsDOMStoragePersistentDB.cpp \
        nsDOMStorageMemoryDB.cpp \

dom/src/storage/nsDOMStorage.cpp

@@ -726,7 +726,7 @@ DOMStorageImpl::DOMStorageImpl(nsDOMStorage* aStorage, DOMStorageImpl& aThat)
 void
 DOMStorageImpl::Init(nsDOMStorage* aStorage)
 {
-  mItemsCached = PR_FALSE;
+  mItemsCachedVersion = 0;
   mItems.Init(8);
   mOwner = aStorage;
   if (nsDOMStorageManager::gStorageManager)
@@ -892,8 +892,6 @@ DOMStorageImpl::SetDBValue(const nsAString& aKey,
                          &usage);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  // Before bug 536544 got fixed we were dropping mItemsCached flag here
-
   if (warnQuota >= 0 && usage > warnQuota) {
     // try to include the window that exceeded the warn quota
     nsCOMPtr<nsIDOMWindow> window;
@@ -947,7 +945,7 @@ void
 DOMStorageImpl::ClearAll()
 {
   mItems.EnumerateEntries(ClearStorageItem, nsnull);
-  mItemsCached = PR_FALSE;
+  mItemsCachedVersion = 0;
 }
 
 struct CopyArgs {
@@ -997,7 +995,7 @@ DOMStorageImpl::CacheKeysFromDB()
   // cache all the keys in the hash. This is used by the Length and Key methods
   // use this cache for better performance. The disadvantage is that the
   // order may break if someone changes the keys in the database directly.
-  if (!mItemsCached) {
+  if (gStorageDB->IsScopeDirty(this)) {
     nsresult rv = InitDB();
     NS_ENSURE_SUCCESS(rv, rv);
 
@@ -1006,7 +1004,7 @@ DOMStorageImpl::CacheKeysFromDB()
     rv = gStorageDB->GetAllKeys(this, &mItems);
     NS_ENSURE_SUCCESS(rv, rv);
 
-    mItemsCached = PR_TRUE;
+    gStorageDB->MarkScopeCached(this);
   }
 
   return NS_OK;
@@ -1075,7 +1073,6 @@ DOMStorageImpl::GetLength(bool aCallerSecure, PRUint32* aLength)
 {
   // Force reload of items from database.  This ensures sync localStorages for
   // same origins among different windows.
-  mItemsCached = PR_FALSE;
   if (UseDB())
     CacheKeysFromDB();
 
@@ -1135,7 +1132,6 @@ DOMStorageImpl::GetKey(bool aCallerSecure, PRUint32 aIndex, nsAString& aKey)
   // something?
 
   if (UseDB()) {
-    mItemsCached = PR_FALSE;
     CacheKeysFromDB();
   }
 
@@ -1255,8 +1251,6 @@ DOMStorageImpl::RemoveValue(bool aCallerSecure, const nsAString& aKey,
     rv = gStorageDB->RemoveKey(this, aKey, !IsOfflineAllowed(mDomain),
                                aKey.Length() + value.Length());
     NS_ENSURE_SUCCESS(rv, rv);
-
-    // Before bug 536544 got fixed we were dropping mItemsCached flag here
   }
   else if (entry) {
     // clear string as StorageItems may be referencing this item

dom/src/storage/nsDOMStorage.h

@@ -270,6 +270,9 @@ public:
 
   // cache the keys from the database for faster lookup
   nsresult CacheKeysFromDB();
+
+  PRUint64 CachedVersion() { return mItemsCachedVersion; }
+  void SetCachedVersion(PRUint64 version) { mItemsCachedVersion = version; }
   
   // Some privileged internal pages can use a persistent storage even in
   // session-only or private-browsing modes.
@@ -328,8 +331,9 @@ private:
 
   static nsresult InitDB();
 
-  // true if items from the database are cached
-  PRPackedBool mItemsCached;
+  // 0 initially or a positive data version number assigned by gStorageDB
+  // after keys have been cached from the database
+  PRUint64 mItemsCachedVersion;
 
   // the key->value item pairs
   nsTHashtable<nsSessionStorageEntry> mItems;

dom/src/storage/nsDOMStorageBaseDB.cpp

@@ -0,0 +1,103 @@
+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* ***** 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 mozilla.org code.
+ *
+ * The Initial Developer of the Original Code is
+ * Mozilla Corporation.
+ * Portions created by the Initial Developer are Copyright (C) 2006
+ * the Initial Developer. All Rights Reserved.
+ *
+ * Contributor(s):
+ *   Honza Bambas <honzab@firemni.cz>
+ *
+ * Alternatively, the contents of this file may be used under the terms of
+ * either of 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 ***** */
+
+#include "nsDOMStorageBaseDB.h"
+#include "nsDOMStorage.h"
+
+PRUint64 nsDOMStorageBaseDB::sGlobalVersion = 1;
+
+nsDOMStorageBaseDB::nsDOMStorageBaseDB()
+{
+  mScopesVersion.Init(8);
+}
+
+// public
+
+void
+nsDOMStorageBaseDB::MarkScopeCached(DOMStorageImpl* aStorage)
+{
+  aStorage->SetCachedVersion(CachedScopeVersion(aStorage));
+}
+
+bool
+nsDOMStorageBaseDB::IsScopeDirty(DOMStorageImpl* aStorage)
+{
+  return !aStorage->CachedVersion() ||
+         (aStorage->CachedVersion() != CachedScopeVersion(aStorage));
+}
+
+// protected
+
+// static
+PRUint64
+nsDOMStorageBaseDB::NextGlobalVersion()
+{
+  sGlobalVersion++;
+  if (sGlobalVersion == 0) // Control overlap, never return 0
+    sGlobalVersion = 1;
+  return sGlobalVersion;
+}
+
+PRUint64
+nsDOMStorageBaseDB::CachedScopeVersion(DOMStorageImpl* aStorage)
+{
+  PRUint64 currentVersion;
+  if (mScopesVersion.Get(aStorage->GetScopeDBKey(), &currentVersion))
+    return currentVersion;
+
+  mScopesVersion.Put(aStorage->GetScopeDBKey(), sGlobalVersion);
+  return sGlobalVersion;
+}
+
+void
+nsDOMStorageBaseDB::MarkScopeDirty(DOMStorageImpl* aStorage)
+{
+  PRUint64 nextVersion = NextGlobalVersion();
+  mScopesVersion.Put(aStorage->GetScopeDBKey(), nextVersion);
+
+  // We may do this because the storage updates its cache along with
+  // updating the database.
+  aStorage->SetCachedVersion(nextVersion);
+}
+
+void
+nsDOMStorageBaseDB::MarkAllScopesDirty()
+{
+  mScopesVersion.Clear();
+  NextGlobalVersion();
+}

dom/src/storage/nsDOMStorageBaseDB.h

@@ -0,0 +1,83 @@
+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* ***** 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 mozilla.org code.
+ *
+ * The Initial Developer of the Original Code is
+ * Mozilla Corporation.
+ * Portions created by the Initial Developer are Copyright (C) 2006
+ * the Initial Developer. All Rights Reserved.
+ *
+ * Contributor(s):
+ *   Honza Bambas <honzab@firemni.cz>
+ *
+ * Alternatively, the contents of this file may be used under the terms of
+ * either of 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 ***** */
+
+#ifndef nsDOMStorageBaseDB_h___
+#define nsDOMStorageBaseDB_h___
+
+#include "nscore.h"
+#include "nsDataHashtable.h"
+
+class DOMStorageImpl;
+
+class nsDOMStorageBaseDB
+{
+public:
+  nsDOMStorageBaseDB();
+  virtual ~nsDOMStorageBaseDB() {}
+
+  /**
+   * Marks the storage as "cached" after the DOMStorageImpl object has loaded
+   * all items to its memory copy of the entries - IsScopeDirty returns false
+   * after call of this method for this storage.
+   *
+   * When a key is changed or deleted in the storage, the storage scope is
+   * marked as "dirty" again and makes the DOMStorageImpl object recache its
+   * keys on next access, because IsScopeDirty returns true again.
+   */
+  void MarkScopeCached(DOMStorageImpl* aStorage);
+
+  /**
+   * Test whether the storage for the scope (i.e. origin or host) has been
+   * changed since the last MarkScopeCached call.
+   */
+  bool IsScopeDirty(DOMStorageImpl* aStorage);
+
+protected:
+  nsDataHashtable<nsCStringHashKey, PRUint64> mScopesVersion;
+
+  static PRUint64 NextGlobalVersion();
+  PRUint64 CachedScopeVersion(DOMStorageImpl* aStorage);
+
+  void MarkScopeDirty(DOMStorageImpl* aStorage);
+  void MarkAllScopesDirty();
+
+private:
+  static PRUint64 sGlobalVersion;
+};
+
+#endif /* nsDOMStorageDB_h___ */

dom/src/storage/nsDOMStorageDBWrapper.cpp

@@ -113,17 +113,23 @@ nsDOMStorageDBWrapper::FlushAndDeleteTemporaryTables(bool force)
   return NS_FAILED(rv1) ? rv1 : rv2;
 }
 
-#define IMPL_FORWARDER(_code)                                         \
+#define IMPL_FORWARDER_GUTS(_return, _code)                                \
   PR_BEGIN_MACRO                                                      \
   if (aStorage->CanUseChromePersist())                                \
-    return mChromePersistentDB._code;                                 \
+    _return mChromePersistentDB._code;                                \
   if (nsDOMStorageManager::gStorageManager->InPrivateBrowsingMode())  \
-    return mPrivateBrowsingDB._code;                                  \
+    _return mPrivateBrowsingDB._code;                                 \
   if (aStorage->SessionOnly())                                        \
-    return mSessionOnlyDB._code;                                      \
-  return mPersistentDB._code;                                         \
+    _return mSessionOnlyDB._code;                                     \
+  _return mPersistentDB._code;                                        \
   PR_END_MACRO
 
+#define IMPL_FORWARDER(_code)                                  \
+  IMPL_FORWARDER_GUTS(return, _code)
+
+#define IMPL_VOID_FORWARDER(_code)                                    \
+  IMPL_FORWARDER_GUTS((void), _code)
+
 nsresult
 nsDOMStorageDBWrapper::GetAllKeys(DOMStorageImpl* aStorage,
                                   nsTHashtable<nsSessionStorageEntry>* aKeys)
@@ -176,6 +182,18 @@ nsDOMStorageDBWrapper::ClearStorage(DOMStorageImpl* aStorage)
   IMPL_FORWARDER(ClearStorage(aStorage));
 }
 
+void
+nsDOMStorageDBWrapper::MarkScopeCached(DOMStorageImpl* aStorage)
+{
+  IMPL_VOID_FORWARDER(MarkScopeCached(aStorage));
+}
+
+bool
+nsDOMStorageDBWrapper::IsScopeDirty(DOMStorageImpl* aStorage)
+{
+  IMPL_FORWARDER(IsScopeDirty(aStorage));
+}
+
 nsresult
 nsDOMStorageDBWrapper::DropSessionOnlyStoragesForHost(const nsACString& aHostName)
 {

dom/src/storage/nsDOMStorageDBWrapper.h

@@ -190,6 +190,25 @@ public:
   nsresult
   GetUsage(const nsACString& aDomain, PRBool aIncludeSubDomains, PRInt32 *aUsage);
 
+  /**
+   * Marks the storage as "cached" after the DOMStorageImpl object has loaded
+   * all items to its memory copy of the entries - IsScopeDirty returns false
+   * after call of this method for this storage.
+   *
+   * When a key is changed or deleted in the storage, the storage scope is
+   * marked as "dirty" again and makes the DOMStorageImpl object recache its
+   * keys on next access, because IsScopeDirty returns true again.
+   */
+  void
+  MarkScopeCached(DOMStorageImpl* aStorage);
+
+  /**
+   * Test whether the storage for the scope (i.e. origin or host) has been
+   * changed since the last MarkScopeCached call.
+   */
+  bool
+  IsScopeDirty(DOMStorageImpl* aStorage);
+
   /**
     * Turns "http://foo.bar.com:80" to "moc.rab.oof.:http:80",
     * i.e. reverses the host, appends a dot, appends the schema

dom/src/storage/nsDOMStorageMemoryDB.cpp

@@ -235,6 +235,8 @@ nsDOMStorageMemoryDB::SetKey(DOMStorageImpl* aStorage,
 
   *aNewUsage = usage;
 
+  MarkScopeDirty(aStorage);
+
   return NS_OK;
 }
 
@@ -255,6 +257,8 @@ nsDOMStorageMemoryDB::SetSecure(DOMStorageImpl* aStorage,
 
   item->mSecure = aSecure;
 
+  MarkScopeDirty(aStorage);
+
   return NS_OK;
 }
 
@@ -277,6 +281,8 @@ nsDOMStorageMemoryDB::RemoveKey(DOMStorageImpl* aStorage,
   storage->mUsageDelta -= aKey.Length() + item->mValue.Length();
   storage->mTable.Remove(aKey);
 
+  MarkScopeDirty(aStorage);
+
   return NS_OK;
 }
 
@@ -302,6 +308,9 @@ nsDOMStorageMemoryDB::ClearStorage(DOMStorageImpl* aStorage)
   NS_ENSURE_SUCCESS(rv, rv);
 
   storage->mTable.Enumerate(RemoveAllKeysEnum, storage);
+
+  MarkScopeDirty(aStorage);
+
   return NS_OK;
 }
 
@@ -309,6 +318,7 @@ nsresult
 nsDOMStorageMemoryDB::DropStorage(DOMStorageImpl* aStorage)
 {
   mData.Remove(aStorage->GetScopeDBKey());
+  MarkScopeDirty(aStorage);
   return NS_OK;
 }
 
@@ -346,6 +356,8 @@ nsDOMStorageMemoryDB::RemoveOwner(const nsACString& aOwner,
   struc.mMatch = PR_TRUE;
   mData.Enumerate(RemoveOwnersEnum, &struc);
 
+  MarkAllScopesDirty();
+
   return NS_OK;
 }
 
@@ -378,6 +390,8 @@ nsDOMStorageMemoryDB::RemoveOwners(const nsTArray<nsString> &aOwners,
     mData.Enumerate(RemoveOwnersEnum, &struc);
   }
 
+  MarkAllScopesDirty();
+
   return NS_OK;
 }
 
@@ -385,6 +399,9 @@ nsresult
 nsDOMStorageMemoryDB::RemoveAll()
 {
   mData.Clear(); // XXX Check this releases all instances
+
+  MarkAllScopesDirty();
+
   return NS_OK;
 }
 

dom/src/storage/nsDOMStorageMemoryDB.h

@@ -40,12 +40,13 @@
 #define nsDOMStorageMemoryDB_h___
 
 #include "nscore.h"
+#include "nsDOMStorageBaseDB.h"
 #include "nsClassHashtable.h"
 #include "nsDataHashtable.h"
 
 class nsDOMStoragePersistentDB;
 
-class nsDOMStorageMemoryDB
+class nsDOMStorageMemoryDB : public nsDOMStorageBaseDB
 {
 public:
   nsDOMStorageMemoryDB() : mPreloading(PR_FALSE) {}

dom/src/storage/nsDOMStoragePersistentDB.cpp

@@ -726,6 +726,8 @@ nsDOMStoragePersistentDB::SetKey(DOMStorageImpl* aStorage,
 
   *aNewUsage = usage;
 
+  MarkScopeDirty(aStorage);
+
   return NS_OK;
 }
 
@@ -760,7 +762,12 @@ nsDOMStoragePersistentDB::SetSecure(DOMStorageImpl* aStorage,
   rv = binder.Add();
   NS_ENSURE_SUCCESS(rv, rv);
 
-  return mSetSecureStatement->Execute();
+  rv = mSetSecureStatement->Execute();
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  MarkScopeDirty(aStorage);
+
+  return NS_OK;
 }
 
 nsresult
@@ -796,6 +803,8 @@ nsDOMStoragePersistentDB::RemoveKey(DOMStorageImpl* aStorage,
   rv = mRemoveKeyStatement->Execute();
   NS_ENSURE_SUCCESS(rv, rv);
 
+  MarkScopeDirty(aStorage);
+
   return NS_OK;
 }
 
@@ -825,6 +834,8 @@ nsDOMStoragePersistentDB::ClearStorage(DOMStorageImpl* aStorage)
   rv = mRemoveStorageStatement->Execute();
   NS_ENSURE_SUCCESS(rv, rv);
 
+  MarkScopeDirty(aStorage);
+
   return NS_OK;
 }
 
@@ -864,6 +875,8 @@ nsDOMStoragePersistentDB::RemoveOwner(const nsACString& aOwner,
   rv = mRemoveOwnerStatement->Execute();
   NS_ENSURE_SUCCESS(rv, rv);
 
+  MarkAllScopesDirty();
+
   return NS_OK;
 }
 
@@ -941,6 +954,8 @@ nsDOMStoragePersistentDB::RemoveOwners(const nsTArray<nsString> &aOwners,
   rv = statement->Execute();
   NS_ENSURE_SUCCESS(rv, rv);
 
+  MarkAllScopesDirty();
+
   return NS_OK;
 }
 
@@ -957,6 +972,8 @@ nsDOMStoragePersistentDB::RemoveAll()
   rv = mRemoveAllStatement->Execute();
   NS_ENSURE_SUCCESS(rv, rv);
 
+  MarkAllScopesDirty();
+
   return NS_OK;
 }
 

dom/src/storage/nsDOMStoragePersistentDB.h

@@ -40,6 +40,7 @@
 #define nsDOMStoragePersistentDB_h___
 
 #include "nscore.h"
+#include "nsDOMStorageBaseDB.h"
 #include "mozIStorageConnection.h"
 #include "mozIStorageStatement.h"
 #include "nsTHashtable.h"
@@ -52,7 +53,7 @@ class nsSessionStorageEntry;
 using mozilla::TimeStamp;
 using mozilla::TimeDuration;
 
-class nsDOMStoragePersistentDB
+class nsDOMStoragePersistentDB : public nsDOMStorageBaseDB
 {
 public:
   nsDOMStoragePersistentDB();