Bug 1488649 - Convert the prefs table from PLDHashTable to mozilla::HashSet. r=glandium

Because it's hot, and mozilla::HashSet is much faster than PLDHashTable.

--HG--
extra : rebase_source : 65efb5c25a01d03201eb31845f627b503efe7c9b

modules/libpref/Preferences.cpp

@@ -19,6 +19,7 @@
 #include "mozilla/Attributes.h"
 #include "mozilla/dom/PContent.h"
 #include "mozilla/HashFunctions.h"
+#include "mozilla/HashTable.h"
 #include "mozilla/Logging.h"
 #include "mozilla/Maybe.h"
 #include "mozilla/MemoryReporting.h"
@@ -538,15 +539,6 @@ public:
 
   // Other operations.
 
-  bool MatchEntry(const char* aPrefName)
-  {
-    if (!mName || !aPrefName) {
-      return false;
-    }
-
-    return strcmp(mName, aPrefName) == 0;
-  }
-
   bool GetBoolValue(PrefValueKind aKind = PrefValueKind::User) const
   {
     MOZ_ASSERT(IsTypeBool());
@@ -955,33 +947,20 @@ private:
   PrefValue mUserValue;
 };
 
-class PrefEntry : public PLDHashEntryHdr
+struct PrefHasher
 {
-public:
+  using Key = mozilla::UniquePtr<Pref>;
-  Pref* mPref; // Note: this is never null in a live entry.
+  using Lookup = const char*;
 
-  static bool MatchEntry(const PLDHashEntryHdr* aEntry, const void* aKey)
+  static HashNumber hash(const Lookup& aLookup) { return HashString(aLookup); }
+
+  static bool match(const Key& aKey, const Lookup& aLookup)
   {
-    auto entry = static_cast<const PrefEntry*>(aEntry);
+    if (!aLookup || !aKey->Name()) {
-    auto prefName = static_cast<const char*>(aKey);
+      return false;
+    }
 
-    return entry->mPref->MatchEntry(prefName);
+    return strcmp(aLookup, aKey->Name()) == 0;
-  }
-
-  static void InitEntry(PLDHashEntryHdr* aEntry, const void* aKey)
-  {
-    auto entry = static_cast<PrefEntry*>(aEntry);
-    auto prefName = static_cast<const char*>(aKey);
-
-    entry->mPref = new Pref(prefName);
-  }
-
-  static void ClearEntry(PLDHashTable* aTable, PLDHashEntryHdr* aEntry)
-  {
-    auto entry = static_cast<PrefEntry*>(aEntry);
-
-    delete entry->mPref;
-    entry->mPref = nullptr;
   }
 };
 
@@ -1301,7 +1280,9 @@ private:
   uintptr_t mNextAndMatchKind;
 };
 
-static PLDHashTable* gHashTable;
+using PrefsHashTable = mozilla::HashSet<mozilla::UniquePtr<Pref>, PrefHasher>;
+
+static PrefsHashTable* gHashTable;
 
 // The callback list contains all the priority callbacks followed by the
 // non-priority callbacks. gLastPriorityNode records where the first part ends.
@@ -1349,22 +1330,16 @@ AddAccessCount(const char* aPrefName)
 static bool gCallbacksInProgress = false;
 static bool gShouldCleanupDeadNodes = false;
 
-static PLDHashTableOps pref_HashTableOps = {
-  PLDHashTable::HashStringKey, PrefEntry::MatchEntry,
-  PLDHashTable::MoveEntryStub, PrefEntry::ClearEntry,
-  PrefEntry::InitEntry,
-};
-
 class PrefsHashIter
 {
-  using Iterator = decltype(gHashTable->Iter());
+  using Iterator = decltype(gHashTable->modIter());
   using ElemType = Pref*;
 
   Iterator mIter;
 
 public:
-  explicit PrefsHashIter(PLDHashTable* aTable)
+  explicit PrefsHashIter(PrefsHashTable* aTable)
-    : mIter(aTable->Iter())
+    : mIter(aTable->modIter())
   {
   }
 
@@ -1391,7 +1366,7 @@ public:
       if (mDone) {
         return nullptr;
       }
-      return static_cast<PrefEntry*>(Iter().Get())->mPref;
+      return Iter().get().get();
     }
     ElemType get() const { return const_cast<Elem*>(this)->get(); }
 
@@ -1400,13 +1375,13 @@ public:
 
     operator ElemType() { return get(); }
 
-    void Remove() { Iter().Remove(); }
+    void Remove() { Iter().remove(); }
 
     Elem& operator++()
     {
       MOZ_ASSERT(!mDone);
-      Iter().Next();
+      Iter().next();
-      mDone = Iter().Done();
+      mDone = Iter().done();
       return *this;
     }
 
@@ -1416,14 +1391,14 @@ public:
     }
   };
 
-  Elem begin() { return Elem(*this, mIter.Done()); }
+  Elem begin() { return Elem(*this, mIter.done()); }
 
   Elem end() { return Elem(*this, true); }
 };
 
 class PrefsIter
 {
-  using Iterator = decltype(gHashTable->Iter());
+  using Iterator = decltype(gHashTable->iter());
   using ElemType = PrefWrapper;
 
   using HashElem = PrefsHashIter::Elem;
@@ -1432,7 +1407,7 @@ class PrefsIter
   using ElemTypeVariant = Variant<HashElem, SharedElem>;
 
   SharedPrefMap* mSharedMap;
-  PLDHashTable* mHashTable;
+  PrefsHashTable* mHashTable;
   PrefsHashIter mIter;
 
   ElemTypeVariant mPos;
@@ -1441,7 +1416,7 @@ class PrefsIter
   Maybe<PrefWrapper> mEntry;
 
 public:
-  PrefsIter(PLDHashTable* aHashTable, SharedPrefMap* aSharedMap)
+  PrefsIter(PrefsHashTable* aHashTable, SharedPrefMap* aSharedMap)
     : mSharedMap(aSharedMap)
     , mHashTable(aHashTable)
     , mIter(aHashTable)
@@ -1531,9 +1506,9 @@ public:
 
     void SkipDuplicates()
     {
-      while (!mDone && (mParent.IteratingBase()
+      while (!mDone &&
-                          ? !!mParent.mHashTable->Search(ref().Name())
+             (mParent.IteratingBase() ? mParent.mHashTable->has(ref().Name())
-                          : ref().IsTypeNone())) {
+                                      : ref().IsTypeNone())) {
         Next();
       }
     }
@@ -1621,7 +1596,7 @@ pref_savePrefs()
 {
   MOZ_ASSERT(NS_IsMainThread());
 
-  PrefSaveData savedPrefs(gHashTable->EntryCount());
+  PrefSaveData savedPrefs(gHashTable->count());
 
   for (auto& pref : PrefsIter(gHashTable, gSharedMap)) {
     nsAutoCString prefValueStr;
@@ -1649,25 +1624,19 @@ static bool gContentProcessPrefsAreInited = false;
 
 #endif // DEBUG
 
-static PrefEntry*
+static Pref*
-pref_HashTableLookupInner(const char* aPrefName)
+pref_HashTableLookup(const char* aPrefName)
 {
   MOZ_ASSERT(NS_IsMainThread() || mozilla::ServoStyleSet::IsInServoTraversal());
 
   MOZ_ASSERT_IF(!XRE_IsParentProcess(), gContentProcessPrefsAreInited);
 
-  return static_cast<PrefEntry*>(gHashTable->Search(aPrefName));
+  // We use readonlyThreadsafeLookup() because we often have concurrent lookups
-}
+  // from multiple Stylo threads. This is safe because those threads cannot
-
+  // modify gHashTable, and the main thread is blocked while Stylo threads are
-static Pref*
+  // doing these lookups.
-pref_HashTableLookup(const char* aPrefName)
+  auto p = gHashTable->readonlyThreadsafeLookup(aPrefName);
-{
+  return p ? p->get() : nullptr;
-  PrefEntry* entry = pref_HashTableLookupInner(aPrefName);
-  if (!entry) {
-    return nullptr;
-  }
-
-  return entry->mPref;
 }
 
 // While notifying preference callbacks, this holds the wrapper for the
@@ -1718,11 +1687,11 @@ pref_LookupForModify(const char* aPrefName,
     return wrapper->as<Pref*>();
   }
 
-  auto entry = static_cast<PrefEntry*>(gHashTable->Add(aPrefName, fallible));
+  Pref* pref = new Pref(aPrefName);
-  if (!entry) {
+  if (!gHashTable->putNew(aPrefName, pref)) {
+    delete pref;
     return Err(NS_ERROR_OUT_OF_MEMORY);
   }
-  Pref* pref = entry->mPref;
   pref->FromWrapper(*wrapper);
   return pref;
 }
@@ -1755,15 +1724,16 @@ pref_SetPref(const char* aPrefName,
   }
 
   if (!pref) {
-    auto entry = static_cast<PrefEntry*>(gHashTable->Add(aPrefName, fallible));
+    auto p = gHashTable->lookupForAdd(aPrefName);
-    if (!entry) {
+    if (!p) {
-      return NS_ERROR_OUT_OF_MEMORY;
+      pref = new Pref(aPrefName);
-    }
-    pref = entry->mPref;
-
-    if (pref->IsTypeNone()) {
-      // New entry. Set the type.
       pref->SetType(aType);
+      if (!gHashTable->add(p, pref)) {
+        delete pref;
+        return NS_ERROR_OUT_OF_MEMORY;
+      }
+    } else {
+      pref = p->get();
     }
   }
 
@@ -2882,16 +2852,14 @@ nsPrefBranch::DeleteBranch(const char* aStartingAt)
   const nsACString& branchNameNoDot =
     Substring(branchName, 0, branchName.Length() - 1);
 
-  for (auto iter = gHashTable->Iter(); !iter.Done(); iter.Next()) {
+  for (auto iter = gHashTable->modIter(); !iter.done(); iter.next()) {
-    Pref* pref = static_cast<PrefEntry*>(iter.Get())->mPref;
-
     // The first disjunct matches branches: e.g. a branch name "foo.bar."
     // matches a name "foo.bar.baz" (but it won't match "foo.barrel.baz").
     // The second disjunct matches leaf nodes: e.g. a branch name "foo.bar."
     // matches a name "foo.bar" (by ignoring the trailing '.').
-    nsDependentCString name(pref->Name());
+    nsDependentCString name(iter.get()->Name());
     if (StringBeginsWith(name, branchName) || name.Equals(branchNameNoDot)) {
-      iter.Remove();
+      iter.remove();
       // The saved callback pref may be invalid now.
       gCallbackPref = nullptr;
     }
@@ -2943,8 +2911,8 @@ nsPrefBranch::GetChildList(const char* aStartingAt,
       // back to us because if they do we are going to add mPrefRoot again.
       const nsCString& element = prefArray[dwIndex];
       outArray[dwIndex] =
-        (char*) moz_xmemdup(element.get() + mPrefRoot.Length(),
+        (char*)moz_xmemdup(element.get() + mPrefRoot.Length(),
-                            element.Length() - mPrefRoot.Length() + 1);
+                           element.Length() - mPrefRoot.Length() + 1);
     }
     *aChildArray = outArray;
   }
@@ -3535,10 +3503,9 @@ PreferenceServiceReporter::CollectReports(
   Preferences::AddSizeOfIncludingThis(mallocSizeOf, sizes);
 
   if (gHashTable) {
-    sizes.mHashTable += gHashTable->ShallowSizeOfIncludingThis(mallocSizeOf);
+    sizes.mHashTable += gHashTable->shallowSizeOfIncludingThis(mallocSizeOf);
-    for (auto iter = gHashTable->Iter(); !iter.Done(); iter.Next()) {
+    for (auto iter = gHashTable->iter(); !iter.done(); iter.next()) {
-      Pref* pref = static_cast<PrefEntry*>(iter.Get())->mPref;
+      iter.get()->AddSizeOfIncludingThis(mallocSizeOf, sizes);
-      pref->AddSizeOfIncludingThis(mallocSizeOf, sizes);
     }
   }
 
@@ -3850,10 +3817,8 @@ Preferences::GetInstanceForService()
 
   MOZ_ASSERT(!gHashTable);
   gHashTable =
-    new PLDHashTable(&pref_HashTableOps,
+    new PrefsHashTable(XRE_IsParentProcess() ? kHashTableInitialLengthParent
-                     sizeof(PrefEntry),
+                                             : kHashTableInitialLengthContent);
-                     (XRE_IsParentProcess() ? kHashTableInitialLengthParent
-                                            : kHashTableInitialLengthContent));
 
   gTelemetryLoadData =
     new nsDataHashtable<nsCStringHashKey, TelemetryLoadData>();
@@ -4018,8 +3983,8 @@ Preferences::SerializePreferences(nsCString& aStr)
 
   aStr.Truncate();
 
-  for (auto iter = gHashTable->Iter(); !iter.Done(); iter.Next()) {
+  for (auto iter = gHashTable->iter(); !iter.done(); iter.next()) {
-    Pref* pref = static_cast<PrefEntry*>(iter.Get())->mPref;
+    Pref* pref = iter.get().get();
     if (!pref->IsTypeNone() && pref->HasAdvisablySizedValues()) {
       pref->SerializeAndAppend(aStr);
     }
@@ -4061,10 +4026,8 @@ Preferences::EnsureSnapshot(size_t* aSize)
   if (!gSharedMap) {
     SharedPrefMapBuilder builder;
 
-    for (auto iter = gHashTable->Iter(); !iter.Done(); iter.Next()) {
+    for (auto iter = gHashTable->iter(); !iter.done(); iter.next()) {
-      Pref* pref = static_cast<PrefEntry*>(iter.Get())->mPref;
+      iter.get()->AddToMap(builder);
-
-      pref->AddToMap(builder);
     }
 
     gSharedMap = new SharedPrefMap(std::move(builder));
@@ -4078,7 +4041,9 @@ Preferences::EnsureSnapshot(size_t* aSize)
     // we can initialize the hashtable with the expected number of per-session
     // changed preferences, rather than the expected total number of
     // preferences.
-    gHashTable->ClearAndPrepareForLength(kHashTableInitialLengthContent);
+    gHashTable->clearAndCompact();
+    Unused << gHashTable->reserve(kHashTableInitialLengthContent);
+
     gPrefNameArena.Clear();
   }
 
@@ -4197,7 +4162,9 @@ Preferences::ResetPrefs()
     return NS_ERROR_NOT_AVAILABLE;
   }
 
-  gHashTable->ClearAndPrepareForLength(kHashTableInitialLengthParent);
+  gHashTable->clearAndCompact();
+  Unused << gHashTable->reserve(kHashTableInitialLengthParent);
+
   gPrefNameArena.Clear();
 
   return InitInitialObjects(/* isStartup */ false).isOk() ? NS_OK
@@ -4212,8 +4179,8 @@ Preferences::ResetUserPrefs()
   MOZ_ASSERT(NS_IsMainThread());
 
   Vector<const char*> prefNames;
-  for (auto iter = gHashTable->Iter(); !iter.Done(); iter.Next()) {
+  for (auto iter = gHashTable->modIter(); !iter.done(); iter.next()) {
-    Pref* pref = static_cast<PrefEntry*>(iter.Get())->mPref;
+    Pref* pref = iter.get().get();
 
     if (pref->HasUserValue()) {
       if (!prefNames.append(pref->Name())) {
@@ -4222,7 +4189,7 @@ Preferences::ResetUserPrefs()
 
       pref->ClearUserValue();
       if (!pref->HasDefaultValue()) {
-        iter.Remove();
+        iter.remove();
       }
     }
   }
@@ -4290,13 +4257,18 @@ Preferences::SetPreference(const dom::Pref& aDomPref)
 
   const char* prefName = aDomPref.name().get();
 
-  auto entry = static_cast<PrefEntry*>(gHashTable->Add(prefName, fallible));
+  Pref* pref;
-  if (!entry) {
+  auto p = gHashTable->lookupForAdd(prefName);
-    return;
+  if (!p) {
+    pref = new Pref(prefName);
+    if (!gHashTable->add(p, pref)) {
+      delete pref;
+      return;
+    }
+  } else {
+    pref = p->get();
   }
 
-  Pref* pref = entry->mPref;
-
   bool valueChanged = false;
   pref->FromDomPref(aDomPref, &valueChanged);
 
@@ -4316,7 +4288,7 @@ Preferences::SetPreference(const dom::Pref& aDomPref)
     if (gSharedMap->Has(pref->Name())) {
       pref->SetType(PrefType::None);
     } else {
-      gHashTable->RemoveEntry(entry);
+      gHashTable->remove(prefName);
     }
     pref = nullptr;
   }
@@ -5269,7 +5241,7 @@ Preferences::ClearUser(const char* aPrefName)
 
     if (!pref->HasDefaultValue()) {
       if (!gSharedMap || !gSharedMap->Has(pref->Name())) {
-        gHashTable->Remove(aPrefName);
+        gHashTable->remove(aPrefName);
       } else {
         pref->SetType(PrefType::None);
       }

modules/libpref/SharedPrefMap.h

@@ -318,7 +318,8 @@ class SharedPrefMap
     uint8_t mIsSticky : 1;
     // True if the preference is locked, as defined by the preference service.
     uint8_t mIsLocked : 1;
-    // True if the preference's default value has changed since it was first set.
+    // True if the preference's default value has changed since it was first
+    // set.
     uint8_t mDefaultChanged : 1;
   };