Bug 1371094 part 1 - Move the nsClassHashtable::LookupForAdd() method to nsBaseHashtable. r=froydnj
authorMats Palmgren <mats@mozilla.com>
Wed, 14 Jun 2017 01:03:38 +0200
changeset 363851 1b01e6d83f2c0f8a2865abf86b13d7c74f3467f8
parent 363850 3e54dcfc694028aeec0601fec6d00ca97a36a8bb
child 363852 fb2197af9bfcf7b3e95f876c60b5b0533e5b15e7
push id32027
push usercbook@mozilla.com
push dateWed, 14 Jun 2017 12:45:45 +0000
treeherdermozilla-central@45fde181a497 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1371094
milestone56.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 1371094 part 1 - Move the nsClassHashtable::LookupForAdd() method to nsBaseHashtable. r=froydnj Also change nsClassHashtable::LookupOrAdd to not regard existing entries with a nullptr value as non-existent. This is to make it consistent with nsBaseHashtable::LookupForAdd() and other methods. MozReview-Commit-ID: 1wYqK8XQbyW
xpcom/ds/nsBaseHashtable.h
xpcom/ds/nsClassHashtable.h
--- a/xpcom/ds/nsBaseHashtable.h
+++ b/xpcom/ds/nsBaseHashtable.h
@@ -189,16 +189,102 @@ public:
     bool shouldRemove = aFunction(ent->mData);
     MOZ_ASSERT(tableGeneration == GetGeneration(),
                "hashtable was modified by the LookupRemoveIf callback!");
     if (shouldRemove) {
       this->RemoveEntry(ent);
     }
   }
 
+  struct EntryPtr {
+  private:
+    EntryType& mEntry;
+    bool mExistingEntry;
+    // For debugging purposes
+#ifdef DEBUG
+    nsBaseHashtable& mTable;
+    uint32_t mTableGeneration;
+    bool mDidInitNewEntry;
+#endif
+
+  public:
+    EntryPtr(nsBaseHashtable& aTable, EntryType* aEntry, bool aExistingEntry)
+      : mEntry(*aEntry)
+      , mExistingEntry(aExistingEntry)
+#ifdef DEBUG
+      , mTable(aTable)
+      , mTableGeneration(aTable.GetGeneration())
+      , mDidInitNewEntry(false)
+#endif
+    {}
+    ~EntryPtr()
+    {
+      MOZ_ASSERT(mExistingEntry || mDidInitNewEntry,
+                 "Forgot to call OrInsert() on a new entry");
+    }
+
+    // Is there something stored in the table already?
+    explicit operator bool() const
+    {
+      MOZ_ASSERT(mTableGeneration == mTable.GetGeneration());
+      return mExistingEntry;
+    }
+
+    template <class F>
+    UserDataType OrInsert(F func)
+    {
+      MOZ_ASSERT(mTableGeneration == mTable.GetGeneration());
+      if (!mExistingEntry) {
+        mEntry.mData = func();
+#ifdef DEBUG
+        mDidInitNewEntry = true;
+#endif
+      }
+      return mEntry.mData;
+    }
+
+    MOZ_MUST_USE DataType& Data()
+    {
+      MOZ_ASSERT(mTableGeneration == mTable.GetGeneration());
+      return mEntry.mData;
+    }
+  };
+
+  /**
+   * Looks up aKey in the hashtable and returns an object that allows you to
+   * insert a new entry into the hashtable for that key if an existing entry
+   * isn't found for it.
+   *
+   * A typical usage of this API looks like this:
+   *
+   *   auto insertedValue = table.LookupForAdd(key).OrInsert([]() {
+   *     return newValue;
+   *   });
+   *
+   *   auto p = table.LookupForAdd(key);
+   *   if (p) {
+   *     // The entry already existed in the table.
+   *     DoSomething(p.Data());
+   *   } else {
+   *     // An existing entry wasn't found, store a new entry in the hashtable.
+   *     p.OrInsert([]() { return newValue; });
+   *   }
+   *
+   * We ensure that the hashtable isn't modified before EntryPtr method calls.
+   * This is useful for cases where you want to insert a new entry into the
+   * hashtable if one doesn't exist before but would like to avoid two hashtable
+   * lookups.
+   */
+  MOZ_MUST_USE EntryPtr LookupForAdd(KeyType aKey)
+  {
+    auto count = Count();
+    EntryType* ent = this->PutEntry(aKey);
+    return EntryPtr(*this, ent, count == Count());
+  }
+
   // This is an iterator that also allows entry removal. Example usage:
   //
   //   for (auto iter = table.Iter(); !iter.Done(); iter.Next()) {
   //     const KeyType key = iter.Key();
   //     const UserDataType data = iter.UserData();
   //     // or
   //     const DataType& data = iter.Data();
   //     // ... do stuff with |key| and/or |data| ...
--- a/xpcom/ds/nsClassHashtable.h
+++ b/xpcom/ds/nsClassHashtable.h
@@ -63,112 +63,37 @@ public:
    *
    * Normally, an entry is deleted when it's removed from an nsClassHashtable,
    * but this function transfers ownership of the entry back to the caller
    * through aOut -- the entry will be deleted when aOut goes out of scope.
    *
    * @param aKey the key to get and remove from the hashtable
    */
   void RemoveAndForget(KeyType aKey, nsAutoPtr<T>& aOut);
-
-  struct EntryPtr {
-  private:
-    typename base_type::EntryType& mEntry;
-    // For debugging purposes
-#ifdef DEBUG
-    base_type& mTable;
-    uint32_t mTableGeneration;
-#endif
-
-  public:
-    EntryPtr(base_type& aTable, typename base_type::EntryType* aEntry)
-      : mEntry(*aEntry)
-#ifdef DEBUG
-      , mTable(aTable)
-      , mTableGeneration(aTable.GetGeneration())
-#endif
-    {
-    }
-    ~EntryPtr()
-    {
-      MOZ_ASSERT(mEntry.mData, "Entry should have been added by now");
-    }
-
-    explicit operator bool() const
-    {
-      // Is there something stored in the table already?
-      MOZ_ASSERT(mTableGeneration == mTable.GetGeneration());
-      return !!mEntry.mData;
-    }
-
-    template <class F>
-    T* OrInsert(F func)
-    {
-      MOZ_ASSERT(mTableGeneration == mTable.GetGeneration());
-      if (!mEntry.mData) {
-        mEntry.mData = func();
-      }
-      return mEntry.mData;
-    }
-  };
-
-  /**
-   * Looks up aKey in the hashtable and returns an object that allows you to
-   * insert a new entry into the hashtable for that key if an existing entry
-   * isn't found for it.
-   *
-   * A typical usage of this API looks like this:
-   *
-   *   auto insertedValue = table.LookupForAdd(key).OrInsert([]() {
-   *     return newValue;
-   *   });
-   *
-   *   auto p = table.LookupForAdd(key);
-   *   if (p) {
-   *     // The entry already existed in the table.
-   *     DoSomething();
-   *   } else {
-   *     // An existing entry wasn't found, store a new entry in the hashtable.
-   *     p.OrInsert([]() { return newValue; });
-   *   }
-   *
-   * We ensure that the hashtable isn't modified before OrInsert() is called.
-   * This is useful for cases where you want to insert a new entry into the
-   * hashtable if one doesn't exist before but would like to avoid two hashtable
-   * lookups.
-   */
-  MOZ_MUST_USE EntryPtr LookupForAdd(KeyType aKey);
 };
 
 //
 // nsClassHashtable definitions
 //
 
 template<class KeyClass, class T>
 template<typename... Args>
 T*
 nsClassHashtable<KeyClass, T>::LookupOrAdd(KeyType aKey,
                                            Args&&... aConstructionArgs)
 {
+  auto count = this->Count();
   typename base_type::EntryType* ent = this->PutEntry(aKey);
-  if (!ent->mData) {
+  if (count != this->Count()) {
     ent->mData = new T(mozilla::Forward<Args>(aConstructionArgs)...);
   }
   return ent->mData;
 }
 
 template<class KeyClass, class T>
-typename nsClassHashtable<KeyClass, T>::EntryPtr
-nsClassHashtable<KeyClass, T>::LookupForAdd(KeyType aKey)
-{
-  typename base_type::EntryType* ent = this->PutEntry(aKey);
-  return EntryPtr(*this, ent);
-}
-
-template<class KeyClass, class T>
 bool
 nsClassHashtable<KeyClass, T>::Get(KeyType aKey, T** aRetVal) const
 {
   typename base_type::EntryType* ent = this->GetEntry(aKey);
 
   if (ent) {
     if (aRetVal) {
       *aRetVal = ent->mData;