Bug 1477579: Part 2 - Use nsID* rather than nsID for mFactories hash keys. r=froydnj

Our factory registrations already require that we store nsID pointers, which
we generally handle by using pointers to static data, or arena allocating a
copy of a dynamic ID.

Since we already have viable pointers to these IDs, there's no reason to store
an entire second copy for our hash key. We can use the pointer, instead, which
saves 16 bytes per entry.

MozReview-Commit-ID: 6MgKrXRSHv4

--HG--
extra : source : 5fb0b7746a5d56563b471e3061ccca124ea45485
extra : absorb_source : 275f5d4dc2c02e3d0391ed16e8690dac1e601758

xpcom/components/nsComponentManager.cpp

@@ -575,7 +575,7 @@ nsComponentManagerImpl::RegisterCIDEntryLocked(
     return;
   }
 
-  if (auto entry = mFactories.LookupForAdd(*aEntry->cid)) {
+  if (auto entry = mFactories.LookupForAdd(aEntry->cid)) {
     nsFactoryEntry* f = entry.Data();
     NS_WARNING("Re-registering a CID?");
 
@@ -608,7 +608,7 @@ nsComponentManagerImpl::RegisterContractIDLocked(
     return;
   }
 
-  nsFactoryEntry* f = mFactories.Get(*aEntry->cid);
+  nsFactoryEntry* f = mFactories.Get(aEntry->cid);
   if (!f) {
     NS_WARNING("No CID found when attempting to map contract ID");
 
@@ -702,9 +702,8 @@ nsComponentManagerImpl::ManifestComponent(ManifestProcessingContext& aCx,
   fl.GetURIString(hash);
 
   MutexLock lock(mLock);
-  auto entry = mFactories.LookupForAdd(cid);
-  if (entry) {
-    nsFactoryEntry* f = entry.Data();
+  nsFactoryEntry* f = mFactories.Get(&cid);
+  if (f) {
     char idstr[NSID_LENGTH];
     cid.ToProvidedString(idstr);
 
@@ -740,7 +739,7 @@ nsComponentManagerImpl::ManifestComponent(ManifestProcessingContext& aCx,
   auto* e = new (KnownNotNull, place) mozilla::Module::CIDEntry();
   e->cid = permanentCID;
 
-  entry.OrInsert([e, km] () { return new nsFactoryEntry(e, km); });
+  mFactories.Put(permanentCID, new nsFactoryEntry(e, km));
 }
 
 void
@@ -760,7 +759,7 @@ nsComponentManagerImpl::ManifestContract(ManifestProcessingContext& aCx,
   }
 
   MutexLock lock(mLock);
-  nsFactoryEntry* f = mFactories.Get(cid);
+  nsFactoryEntry* f = mFactories.Get(&cid);
   if (!f) {
     lock.Unlock();
     LogMessageWithContext(aCx.mFile, aLineNo,
@@ -927,7 +926,7 @@ nsFactoryEntry*
 nsComponentManagerImpl::GetFactoryEntry(const nsCID& aClass)
 {
   SafeMutexAutoLock lock(mLock);
-  return mFactories.Get(aClass);
+  return mFactories.Get(&aClass);
 }
 
 already_AddRefed<nsIFactory>
@@ -1256,7 +1255,7 @@ nsComponentManagerImpl::GetService(const nsCID& aClass,
   nsCOMPtr<nsISupports> service;
   MutexLock lock(mLock);
 
-  nsFactoryEntry* entry = mFactories.Get(aClass);
+  nsFactoryEntry* entry = mFactories.Get(&aClass);
   if (!entry) {
     return NS_ERROR_FACTORY_NOT_REGISTERED;
   }
@@ -1372,7 +1371,7 @@ nsComponentManagerImpl::IsServiceInstantiated(const nsCID& aClass,
 
   {
     SafeMutexAutoLock lock(mLock);
-    entry = mFactories.Get(aClass);
+    entry = mFactories.Get(&aClass);
   }
 
   if (entry && entry->mServiceObject) {
@@ -1577,7 +1576,7 @@ nsComponentManagerImpl::RegisterFactory(const nsCID& aClass,
     }
 
     SafeMutexAutoLock lock(mLock);
-    nsFactoryEntry* oldf = mFactories.Get(aClass);
+    nsFactoryEntry* oldf = mFactories.Get(&aClass);
     if (!oldf) {
       return NS_ERROR_FACTORY_NOT_REGISTERED;
     }
@@ -1589,7 +1588,7 @@ nsComponentManagerImpl::RegisterFactory(const nsCID& aClass,
   nsAutoPtr<nsFactoryEntry> f(new nsFactoryEntry(aClass, aFactory));
 
   SafeMutexAutoLock lock(mLock);
-  if (auto entry = mFactories.LookupForAdd(aClass)) {
+  if (auto entry = mFactories.LookupForAdd(f->mCIDEntry->cid)) {
     return NS_ERROR_FACTORY_EXISTS;
   } else {
     if (aContractID) {
@@ -1612,7 +1611,7 @@ nsComponentManagerImpl::UnregisterFactory(const nsCID& aClass,
 
   {
     SafeMutexAutoLock lock(mLock);
-    auto entry = mFactories.Lookup(aClass);
+    auto entry = mFactories.Lookup(&aClass);
     nsFactoryEntry* f = entry ? entry.Data() : nullptr;
     if (!f || f->mFactory != aFactory) {
       return NS_ERROR_FACTORY_NOT_REGISTERED;
@@ -1700,9 +1699,9 @@ nsComponentManagerImpl::EnumerateCIDs(nsISimpleEnumerator** aEnumerator)
 {
   nsCOMArray<nsISupports> array;
   for (auto iter = mFactories.Iter(); !iter.Done(); iter.Next()) {
-    const nsID& id = iter.Key();
+    const nsID* id = iter.Key();
     nsCOMPtr<nsISupportsID> wrapper = new nsSupportsID();
-    wrapper->SetData(&id);
+    wrapper->SetData(id);
     array.AppendObject(wrapper);
   }
   return NS_NewArrayEnumerator(aEnumerator, array);

xpcom/components/nsComponentManager.h

@@ -167,7 +167,7 @@ public:
                                   uint32_t aContractIDLen);
   nsFactoryEntry* GetFactoryEntry(const nsCID& aClass);
 
-  nsDataHashtable<nsIDHashKey, nsFactoryEntry*> mFactories;
+  nsDataHashtable<nsIDPointerHashKey, nsFactoryEntry*> mFactories;
   nsDataHashtable<nsCStringHashKey, nsFactoryEntry*> mContractIDs;
 
   SafeMutex mLock;

xpcom/ds/nsHashKeys.h

@@ -473,6 +473,37 @@ private:
   const nsID mID;
 };
 
+/**
+ * hashkey wrapper using nsID* KeyType
+ *
+ * @see nsTHashtable::EntryType for specification
+ */
+class nsIDPointerHashKey : public PLDHashEntryHdr
+{
+public:
+  typedef const nsID* KeyType;
+  typedef const nsID* KeyTypePointer;
+
+  explicit nsIDPointerHashKey(const nsID* aInID) : mID(aInID) {}
+  nsIDPointerHashKey(const nsIDPointerHashKey& aToCopy) : mID(aToCopy.mID) {}
+  ~nsIDPointerHashKey() = default;
+
+  KeyType GetKey() const { return mID; }
+  bool KeyEquals(KeyTypePointer aKey) const { return aKey->Equals(*mID); }
+
+  static KeyTypePointer KeyToPointer(KeyType aKey) { return aKey; }
+  static PLDHashNumber HashKey(KeyTypePointer aKey)
+  {
+    // Hash the nsID object's raw bytes.
+    return mozilla::HashBytes(aKey, sizeof(*aKey));
+  }
+
+  enum { ALLOW_MEMMOVE = true };
+
+private:
+  const nsID* mID;
+};
+
 /**
  * hashkey wrapper for "dependent" const char*; this class does not "own"
  * its string pointer.