Bug 1051102 - Make mPackagesHash be a more modern hashtable. r=bsmedberg
authorBlake Kaplan <mrbkap@gmail.com>
Mon, 11 Aug 2014 11:13:36 -0700
changeset 198962 7f5a7dc734207085c2e8a7b8c4103285307b8f8f
parent 198961 d20cfaa4220b62da01767efde0e6df99e1145cdb
child 198963 74aae2dec3c9f2a50a70fa101f2d5986d7c6048f
push id27293
push useremorley@mozilla.com
push dateTue, 12 Aug 2014 14:29:39 +0000
treeherderautoland@ee1ad12a3939 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbsmedberg
bugs1051102
milestone34.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1051102 - Make mPackagesHash be a more modern hashtable. r=bsmedberg
chrome/nsChromeRegistryChrome.cpp
chrome/nsChromeRegistryChrome.h
--- a/chrome/nsChromeRegistryChrome.cpp
+++ b/chrome/nsChromeRegistryChrome.cpp
@@ -44,23 +44,17 @@
 #define SELECTED_LOCALE_PREF "general.useragent.locale"
 #define SELECTED_SKIN_PREF   "general.skins.selectedSkin"
 #define PACKAGE_OVERRIDE_BRANCH "chrome.override_package."
 
 using namespace mozilla;
 using mozilla::dom::ContentParent;
 using mozilla::dom::PContentParent;
 
-static PLDHashOperator
-RemoveAll(PLDHashTable *table, PLDHashEntryHdr *entry, uint32_t number, void *arg)
-{
-  return (PLDHashOperator) (PL_DHASH_NEXT | PL_DHASH_REMOVE);
-}
-
-// We use a "best-fit" algorithm for matching locales and themes. 
+// We use a "best-fit" algorithm for matching locales and themes.
 // 1) the exact selected locale/theme
 // 2) (locales only) same language, different country
 //    e.g. en-GB is the selected locale, only en-US is available
 // 3) any available locale/theme
 
 /**
  * Match the language-part of two lang-COUNTRY codes, hopefully but
  * not guaranteed to be in the form ab-CD or abz-CD. "ab" should also
@@ -77,17 +71,17 @@ LanguagesMatch(const nsACString& a, cons
   a.BeginReading(as);
   a.EndReading(ae);
   b.BeginReading(bs);
   b.EndReading(be);
 
   while (*as == *bs) {
     if (*as == '-')
       return true;
- 
+
     ++as; ++bs;
 
     // reached the end
     if (as == ae && bs == be)
       return true;
 
     // "a" is short
     if (as == ae)
@@ -100,37 +94,32 @@ LanguagesMatch(const nsACString& a, cons
 
   return false;
 }
 
 nsChromeRegistryChrome::nsChromeRegistryChrome()
   : mProfileLoaded(false)
   , mDynamicRegistration(true)
 {
-  mPackagesHash.ops = nullptr;
 }
 
 nsChromeRegistryChrome::~nsChromeRegistryChrome()
 {
-  if (mPackagesHash.ops)
-    PL_DHashTableFinish(&mPackagesHash);
 }
 
 nsresult
 nsChromeRegistryChrome::Init()
 {
   nsresult rv = nsChromeRegistry::Init();
   if (NS_FAILED(rv))
     return rv;
 
   mSelectedLocale = NS_LITERAL_CSTRING("en-US");
   mSelectedSkin = NS_LITERAL_CSTRING("classic/1.0");
 
-  PL_DHashTableInit(&mPackagesHash, &kTableOps, nullptr, sizeof(PackageEntry));
-
   bool safeMode = false;
   nsCOMPtr<nsIXULRuntime> xulrun (do_GetService(XULAPPINFO_SERVICE_CONTRACTID));
   if (xulrun)
     xulrun->GetInSafeMode(&safeMode);
 
   nsCOMPtr<nsIPrefService> prefserv (do_GetService(NS_PREFSERVICE_CONTRACTID));
   nsCOMPtr<nsIPrefBranch> prefs;
 
@@ -194,22 +183,18 @@ nsChromeRegistryChrome::GetLocalesForPac
   nsresult rv = OverrideLocalePackage(aPackage, realpackage);
   if (NS_FAILED(rv))
     return rv;
 
   nsTArray<nsCString> *a = new nsTArray<nsCString>;
   if (!a)
     return NS_ERROR_OUT_OF_MEMORY;
 
-  PackageEntry* entry =
-      static_cast<PackageEntry*>(PL_DHashTableOperate(&mPackagesHash,
-                                                      & realpackage,
-                                                      PL_DHASH_LOOKUP));
-
-  if (PL_DHASH_ENTRY_IS_BUSY(entry)) {
+  PackageEntry* entry;
+  if (mPackagesHash.Get(realpackage, &entry)) {
     entry->locales.EnumerateToArray(a);
   }
 
   rv = NS_NewAdoptingUTF8StringEnumerator(aResult, a);
   if (NS_FAILED(rv))
     delete a;
 
   return rv;
@@ -243,17 +228,17 @@ nsChromeRegistryChrome::IsLocaleRTL(cons
 
   // first check the intl.uidirection.<locale> preference, and if that is not
   // set, check the same preference but with just the first two characters of
   // the locale. If that isn't set, default to left-to-right.
   nsAutoCString prefString = NS_LITERAL_CSTRING("intl.uidirection.") + locale;
   nsCOMPtr<nsIPrefBranch> prefBranch (do_GetService(NS_PREFSERVICE_CONTRACTID));
   if (!prefBranch)
     return NS_OK;
-  
+
   nsXPIDLCString dir;
   prefBranch->GetCharPref(prefString.get(), getter_Copies(dir));
   if (dir.IsEmpty()) {
     int32_t hyphen = prefString.FindChar('-');
     if (hyphen >= 1) {
       nsAutoCString shortPref(Substring(prefString, 0, hyphen));
       prefBranch->GetCharPref(shortPref.get(), getter_Copies(dir));
     }
@@ -265,22 +250,18 @@ nsChromeRegistryChrome::IsLocaleRTL(cons
 nsresult
 nsChromeRegistryChrome::GetSelectedLocale(const nsACString& aPackage,
                                           nsACString& aLocale)
 {
   nsCString realpackage;
   nsresult rv = OverrideLocalePackage(aPackage, realpackage);
   if (NS_FAILED(rv))
     return rv;
-  PackageEntry* entry =
-      static_cast<PackageEntry*>(PL_DHashTableOperate(&mPackagesHash,
-                                                      & realpackage,
-                                                      PL_DHASH_LOOKUP));
-
-  if (PL_DHASH_ENTRY_IS_FREE(entry))
+  PackageEntry* entry;
+  if (!mPackagesHash.Get(realpackage, &entry))
     return NS_ERROR_FILE_NOT_FOUND;
 
   aLocale = entry->locales.GetSelected(mSelectedLocale, nsProviderArray::LOCALE);
   if (aLocale.IsEmpty())
     return NS_ERROR_FAILURE;
 
   return NS_OK;
 }
@@ -383,17 +364,17 @@ nsChromeRegistryChrome::Observe(nsISuppo
   }
 
   return rv;
 }
 
 NS_IMETHODIMP
 nsChromeRegistryChrome::CheckForNewChrome()
 {
-  PL_DHashTableEnumerate(&mPackagesHash, RemoveAll, nullptr);
+  mPackagesHash.Clear();
   mOverlayHash.Clear();
   mStyleHash.Clear();
   mOverrideTable.Clear();
 
   mDynamicRegistration = false;
 
   nsComponentManagerImpl::gComponentManager->RereadChromeManifests();
 
@@ -439,17 +420,17 @@ EnumerateOverride(nsIURI* aURIKey,
 {
   nsTArray<OverrideMapping>* overrides =
       static_cast<nsTArray<OverrideMapping>*>(aArg);
 
   SerializedURI chromeURI, overrideURI;
 
   SerializeURI(aURIKey, chromeURI);
   SerializeURI(aURI, overrideURI);
-        
+
   OverrideMapping override = {
     chromeURI, overrideURI
   };
   overrides->AppendElement(override);
   return (PLDHashOperator)PL_DHASH_NEXT;
 }
 
 struct EnumerationArgs
@@ -465,17 +446,17 @@ nsChromeRegistryChrome::SendRegisteredCh
 {
   InfallibleTArray<ChromePackage> packages;
   InfallibleTArray<ResourceMapping> resources;
   InfallibleTArray<OverrideMapping> overrides;
 
   EnumerationArgs args = {
     packages, mSelectedLocale, mSelectedSkin
   };
-  PL_DHashTableEnumerate(&mPackagesHash, CollectPackages, &args);
+  mPackagesHash.EnumerateRead(CollectPackages, &args);
 
   nsCOMPtr<nsIIOService> io (do_GetIOService());
   NS_ENSURE_TRUE_VOID(io);
 
   nsCOMPtr<nsIProtocolHandler> ph;
   nsresult rv = io->GetProtocolHandler("resource", getter_AddRefs(ph));
   NS_ENSURE_SUCCESS_VOID(rv);
 
@@ -501,45 +482,44 @@ nsChromeRegistryChrome::SendRegisteredCh
         parents[i]->SendRegisterChrome(packages, resources, overrides,
                                        mSelectedLocale, true);
       NS_WARN_IF_FALSE(success, "couldn't reset a child's registered chrome");
     }
   }
 }
 
 /* static */ void
-nsChromeRegistryChrome::ChromePackageFromPackageEntry(PackageEntry* aPackage,
+nsChromeRegistryChrome::ChromePackageFromPackageEntry(const nsACString& aPackageName,
+                                                      PackageEntry* aPackage,
                                                       ChromePackage* aChromePackage,
                                                       const nsCString& aSelectedLocale,
                                                       const nsCString& aSelectedSkin)
 {
   SerializeURI(aPackage->baseURI, aChromePackage->contentBaseURI);
   SerializeURI(aPackage->locales.GetBase(aSelectedLocale,
                                          nsProviderArray::LOCALE),
                aChromePackage->localeBaseURI);
   SerializeURI(aPackage->skins.GetBase(aSelectedSkin, nsProviderArray::ANY),
                aChromePackage->skinBaseURI);
-  aChromePackage->package = aPackage->package;
+  aChromePackage->package = aPackageName;
   aChromePackage->flags = aPackage->flags;
 }
 
 PLDHashOperator
-nsChromeRegistryChrome::CollectPackages(PLDHashTable *table,
-                                  PLDHashEntryHdr *entry,
-                                  uint32_t number,
-                                  void *arg)
+nsChromeRegistryChrome::CollectPackages(const nsACString &aKey,
+                                        PackageEntry *package,
+                                        void *arg)
 {
   EnumerationArgs* args = static_cast<EnumerationArgs*>(arg);
-  PackageEntry* package = static_cast<PackageEntry*>(entry);
 
   ChromePackage chromePackage;
-  ChromePackageFromPackageEntry(package, &chromePackage,
+  ChromePackageFromPackageEntry(aKey, package, &chromePackage,
                                 args->selectedLocale, args->selectedSkin);
   args->packages.AppendElement(chromePackage);
-  return (PLDHashOperator)PL_DHASH_NEXT;
+  return PL_DHASH_NEXT;
 }
 
 static bool
 CanLoadResource(nsIURI* aResourceURI)
 {
   bool isLocalResource = false;
   (void)NS_URIChainHasFlags(aResourceURI,
                             nsIProtocolHandler::URI_IS_LOCAL_RESOURCE,
@@ -547,22 +527,18 @@ CanLoadResource(nsIURI* aResourceURI)
   return isLocalResource;
 }
 
 nsIURI*
 nsChromeRegistryChrome::GetBaseURIFromPackage(const nsCString& aPackage,
                                               const nsCString& aProvider,
                                               const nsCString& aPath)
 {
-  PackageEntry* entry =
-      static_cast<PackageEntry*>(PL_DHashTableOperate(&mPackagesHash,
-                                                      &aPackage,
-                                                      PL_DHASH_LOOKUP));
-
-  if (PL_DHASH_ENTRY_IS_FREE(entry)) {
+  PackageEntry* entry;
+  if (!mPackagesHash.Get(aPackage, &entry)) {
     if (!mInitialized)
       return nullptr;
 
     LogMessage("No chrome package registered for chrome://%s/%s/%s",
                aPackage.get(), aProvider.get(), aPath.get());
 
     return nullptr;
   }
@@ -578,72 +554,24 @@ nsChromeRegistryChrome::GetBaseURIFromPa
   }
   return nullptr;
 }
 
 nsresult
 nsChromeRegistryChrome::GetFlagsFromPackage(const nsCString& aPackage,
                                             uint32_t* aFlags)
 {
-  PackageEntry* entry =
-      static_cast<PackageEntry*>(PL_DHashTableOperate(&mPackagesHash,
-                                                      & (nsACString&) aPackage,
-                                                      PL_DHASH_LOOKUP));
-  if (PL_DHASH_ENTRY_IS_FREE(entry))
+  PackageEntry* entry;
+  if (!mPackagesHash.Get(aPackage, &entry))
     return NS_ERROR_FILE_NOT_FOUND;
 
   *aFlags = entry->flags;
   return NS_OK;
 }
 
-PLHashNumber
-nsChromeRegistryChrome::HashKey(PLDHashTable *table, const void *key)
-{
-  const nsACString& str = *reinterpret_cast<const nsACString*>(key);
-  return HashString(str);
-}
-
-bool
-nsChromeRegistryChrome::MatchKey(PLDHashTable *table, const PLDHashEntryHdr *entry,
-                           const void *key)
-{
-  const nsACString& str = *reinterpret_cast<const nsACString*>(key);
-  const PackageEntry* pentry = static_cast<const PackageEntry*>(entry);
-  return str.Equals(pentry->package);
-}
-
-void
-nsChromeRegistryChrome::ClearEntry(PLDHashTable *table, PLDHashEntryHdr *entry)
-{
-  PackageEntry* pentry = static_cast<PackageEntry*>(entry);
-  pentry->~PackageEntry();
-}
-
-bool
-nsChromeRegistryChrome::InitEntry(PLDHashTable *table, PLDHashEntryHdr *entry,
-                            const void *key)
-{
-  const nsACString& str = *reinterpret_cast<const nsACString*>(key);
-
-  new (entry) PackageEntry(str);
-  return true;
-}
-
-const PLDHashTableOps
-nsChromeRegistryChrome::kTableOps = {
-  PL_DHashAllocTable,
-  PL_DHashFreeTable,
-  HashKey,
-  MatchKey,
-  PL_DHashMoveEntryStub,
-  ClearEntry,
-  PL_DHashFinalizeStub,
-  InitEntry
-};
-
 nsChromeRegistryChrome::ProviderEntry*
 nsChromeRegistryChrome::nsProviderArray::GetProvider(const nsACString& aPreferred, MatchType aType)
 {
   size_t i = mArray.Length();
   if (!i)
     return nullptr;
 
   ProviderEntry* found = nullptr;  // Only set if we find a partial-match locale
@@ -849,33 +777,29 @@ nsChromeRegistryChrome::ManifestContent(
 
   if (!CanLoadResource(resolved)) {
     LogMessageWithContext(resolved, lineno, nsIScriptError::warningFlag,
                           "During chrome registration, cannot register non-local URI '%s' as content.",
                           uri);
     return;
   }
 
-  PackageEntry* entry =
-    static_cast<PackageEntry*>(PL_DHashTableOperate(&mPackagesHash,
-                                                    & (const nsACString&) nsDependentCString(package),
-                                                    PL_DHASH_ADD));
-  if (!entry)
-    return;
-
+  nsDependentCString packageName(package);
+  PackageEntry* entry = mPackagesHash.LookupOrAdd(packageName);
   entry->baseURI = resolved;
 
   if (platform)
     entry->flags |= PLATFORM_PACKAGE;
   if (contentaccessible)
     entry->flags |= CONTENT_ACCESSIBLE;
 
   if (mDynamicRegistration) {
     ChromePackage chromePackage;
-    ChromePackageFromPackageEntry(entry, &chromePackage, mSelectedLocale, mSelectedSkin);
+    ChromePackageFromPackageEntry(packageName, entry, &chromePackage,
+                                  mSelectedLocale, mSelectedSkin);
     SendManifestEntry(chromePackage);
   }
 }
 
 void
 nsChromeRegistryChrome::ManifestLocale(ManifestProcessingContext& cx, int lineno,
                                        char *const * argv, bool platform,
                                        bool contentaccessible)
@@ -895,28 +819,24 @@ nsChromeRegistryChrome::ManifestLocale(M
 
   if (!CanLoadResource(resolved)) {
     LogMessageWithContext(resolved, lineno, nsIScriptError::warningFlag,
                           "During chrome registration, cannot register non-local URI '%s' as content.",
                           uri);
     return;
   }
 
-  PackageEntry* entry =
-    static_cast<PackageEntry*>(PL_DHashTableOperate(&mPackagesHash,
-                                                    & (const nsACString&) nsDependentCString(package),
-                                                    PL_DHASH_ADD));
-  if (!entry)
-    return;
-
+  nsDependentCString packageName(package);
+  PackageEntry* entry = mPackagesHash.LookupOrAdd(packageName);
   entry->locales.SetBase(nsDependentCString(provider), resolved);
 
   if (mDynamicRegistration) {
     ChromePackage chromePackage;
-    ChromePackageFromPackageEntry(entry, &chromePackage, mSelectedLocale, mSelectedSkin);
+    ChromePackageFromPackageEntry(packageName, entry, &chromePackage,
+                                  mSelectedLocale, mSelectedSkin);
     SendManifestEntry(chromePackage);
   }
 }
 
 void
 nsChromeRegistryChrome::ManifestSkin(ManifestProcessingContext& cx, int lineno,
                                      char *const * argv, bool platform,
                                      bool contentaccessible)
@@ -936,28 +856,24 @@ nsChromeRegistryChrome::ManifestSkin(Man
 
   if (!CanLoadResource(resolved)) {
     LogMessageWithContext(resolved, lineno, nsIScriptError::warningFlag,
                           "During chrome registration, cannot register non-local URI '%s' as content.",
                           uri);
     return;
   }
 
-  PackageEntry* entry =
-    static_cast<PackageEntry*>(PL_DHashTableOperate(&mPackagesHash,
-                                                    & (const nsACString&) nsDependentCString(package),
-                                                    PL_DHASH_ADD));
-  if (!entry)
-    return;
-
+  nsDependentCString packageName(package);
+  PackageEntry* entry = mPackagesHash.LookupOrAdd(packageName);
   entry->skins.SetBase(nsDependentCString(provider), resolved);
 
   if (mDynamicRegistration) {
     ChromePackage chromePackage;
-    ChromePackageFromPackageEntry(entry, &chromePackage, mSelectedLocale, mSelectedSkin);
+    ChromePackageFromPackageEntry(packageName, entry, &chromePackage,
+                                  mSelectedLocale, mSelectedSkin);
     SendManifestEntry(chromePackage);
   }
 }
 
 void
 nsChromeRegistryChrome::ManifestOverlay(ManifestProcessingContext& cx, int lineno,
                                         char *const * argv, bool platform,
                                         bool contentaccessible)
@@ -1058,17 +974,17 @@ nsChromeRegistryChrome::ManifestResource
     NS_WARNING("No IO service trying to process chrome manifests");
     return;
   }
 
   nsCOMPtr<nsIProtocolHandler> ph;
   nsresult rv = io->GetProtocolHandler("resource", getter_AddRefs(ph));
   if (NS_FAILED(rv))
     return;
-  
+
   nsCOMPtr<nsIResProtocolHandler> rph = do_QueryInterface(ph);
 
   bool exists = false;
   rv = rph->HasSubstitution(host, &exists);
   if (exists) {
     LogMessageWithContext(cx.GetManifestURI(), lineno, nsIScriptError::warningFlag,
                           "Duplicate resource declaration for '%s' ignored.", package);
     return;
--- a/chrome/nsChromeRegistryChrome.h
+++ b/chrome/nsChromeRegistryChrome.h
@@ -5,16 +5,17 @@
 
 #ifndef nsChromeRegistryChrome_h
 #define nsChromeRegistryChrome_h
 
 #include "nsCOMArray.h"
 #include "nsChromeRegistry.h"
 #include "nsTArray.h"
 #include "mozilla/Move.h"
+#include "nsClassHashtable.h"
 
 namespace mozilla {
 namespace dom {
 class PContentParent;
 }
 }
 
 class nsIPrefBranch;
@@ -48,42 +49,35 @@ class nsChromeRegistryChrome : public ns
 
   // If aChild is non-null then it is a new child to notify. If aChild is
   // null, then we have installed new chrome and we are resetting all of our
   // children's registered chrome.
   void SendRegisteredChrome(mozilla::dom::PContentParent* aChild);
 
  private:
   struct PackageEntry;
-  static void ChromePackageFromPackageEntry(PackageEntry* aPackage,
+  static void ChromePackageFromPackageEntry(const nsACString& aPackageName,
+                                            PackageEntry* aPackage,
                                             ChromePackage* aChromePackage,
                                             const nsCString& aSelectedLocale,
                                             const nsCString& aSelectedSkin);
-  static PLDHashOperator CollectPackages(PLDHashTable *table,
-                                         PLDHashEntryHdr *entry,
-                                         uint32_t number, void *arg);
+  static PLDHashOperator CollectPackages(const nsACString &aKey,
+                                         PackageEntry *package,
+                                         void *arg);
 
   nsresult OverrideLocalePackage(const nsACString& aPackage,
                                  nsACString& aOverride);
   nsresult SelectLocaleFromPref(nsIPrefBranch* prefs);
   nsresult UpdateSelectedLocale() MOZ_OVERRIDE;
   nsIURI* GetBaseURIFromPackage(const nsCString& aPackage,
                                  const nsCString& aProvider,
                                  const nsCString& aPath) MOZ_OVERRIDE;
   nsresult GetFlagsFromPackage(const nsCString& aPackage,
                                uint32_t* aFlags) MOZ_OVERRIDE;
 
-  static const PLDHashTableOps kTableOps;
-  static PLDHashNumber HashKey(PLDHashTable *table, const void *key);
-  static bool          MatchKey(PLDHashTable *table, const PLDHashEntryHdr *entry,
-                                const void *key);
-  static void          ClearEntry(PLDHashTable *table, PLDHashEntryHdr *entry);
-  static bool          InitEntry(PLDHashTable *table, PLDHashEntryHdr *entry,
-                                 const void *key);
-
   struct ProviderEntry
   {
     ProviderEntry(const nsACString& aProvider, nsIURI* aBase) :
     provider(aProvider),
     baseURI(aBase) { }
 
     nsCString        provider;
     nsCOMPtr<nsIURI> baseURI;
@@ -112,21 +106,20 @@ class nsChromeRegistryChrome : public ns
    private:
     ProviderEntry* GetProvider(const nsACString& aPreferred, MatchType aType);
 
     nsTArray<ProviderEntry> mArray;
   };
 
   struct PackageEntry : public PLDHashEntryHdr
   {
-    PackageEntry(const nsACString& package)
-    : package(package), flags(0) { }
+    PackageEntry()
+    : flags(0) { }
     ~PackageEntry() { }
 
-    nsCString        package;
     nsCOMPtr<nsIURI> baseURI;
     uint32_t         flags;
     nsProviderArray  locales;
     nsProviderArray  skins;
   };
 
   class OverlayListEntry : public nsURIHashKey
   {
@@ -165,17 +158,17 @@ class nsChromeRegistryChrome : public ns
 
   bool mProfileLoaded;
   bool mDynamicRegistration;
 
   nsCString mSelectedLocale;
   nsCString mSelectedSkin;
 
   // Hash of package names ("global") to PackageEntry objects
-  PLDHashTable mPackagesHash;
+  nsClassHashtable<nsCStringHashKey, PackageEntry> mPackagesHash;
 
   virtual void ManifestContent(ManifestProcessingContext& cx, int lineno,
                                char *const * argv, bool platform,
                                bool contentaccessible);
   virtual void ManifestLocale(ManifestProcessingContext& cx, int lineno,
                               char *const * argv, bool platform,
                               bool contentaccessible);
   virtual void ManifestSkin(ManifestProcessingContext& cx, int lineno,