Bug 1460674 - part 3 - make PLDHashTable iteration faster; r=njn

The core loop of Iterator::Next() requires multiple branches, one to
check for entry liveness and one to check for wraparound.  We can
rewrite this to use masking instead, as well as iterating only over the
hashes, and reconstructing the entry pointer when we know we've reached
a live entry.  This change cuts the time taken on the collections
benchmark by the iteration portion in half.

Author: froydnj

xpcom/ds/PLDHashTable.cpp

@@ -746,7 +746,6 @@ PLDHashTable::ShallowSizeOfIncludingThis(MallocSizeOf aMallocSizeOf) const
 
 PLDHashTable::Iterator::Iterator(Iterator&& aOther)
   : mTable(aOther.mTable)
-  , mLimit(aOther.mLimit)
   , mCurrent(aOther.mCurrent)
   , mNexts(aOther.mNexts)
   , mNextsLimit(aOther.mNextsLimit)
@@ -755,7 +754,7 @@ PLDHashTable::Iterator::Iterator(Iterator&& aOther)
 {
   // No need to change |mChecker| here.
   aOther.mTable = nullptr;
-  // We don't really have the concept of a null slot, so leave mLimit/mCurrent.
+  // We don't really have the concept of a null slot, so leave mCurrent.
   aOther.mNexts = 0;
   aOther.mNextsLimit = 0;
   aOther.mHaveRemoved = false;
@@ -764,8 +763,6 @@ PLDHashTable::Iterator::Iterator(Iterator&& aOther)
 
 PLDHashTable::Iterator::Iterator(PLDHashTable* aTable)
   : mTable(aTable)
-  , mLimit(mTable->mEntryStore.SlotForIndex(mTable->Capacity(), mTable->mEntrySize,
-                                            mTable->Capacity()))
   , mCurrent(mTable->mEntryStore.SlotForIndex(0, mTable->mEntrySize,
                                               mTable->Capacity()))
   , mNexts(0)
@@ -787,10 +784,8 @@ PLDHashTable::Iterator::Iterator(PLDHashTable* aTable)
   }
 
   // Advance to the first live entry, if there is one.
-  if (!Done()) {
-    while (IsOnNonLiveEntry()) {
-      MoveToNextEntry();
-    }
+  if (!Done() && IsOnNonLiveEntry()) {
+    MoveToNextLiveEntry();
   }
 }
 
@@ -813,17 +808,6 @@ PLDHashTable::Iterator::IsOnNonLiveEntry() const
   return !mCurrent.IsLive();
 }
 
-MOZ_ALWAYS_INLINE void
-PLDHashTable::Iterator::MoveToNextEntry()
-{
-  mCurrent.Next(mEntrySize);
-  if (mCurrent == mLimit) {
-    // We wrapped around.  Possible due to chaos mode.
-    mCurrent = mTable->mEntryStore.SlotForIndex(0, mEntrySize,
-                                                mTable->CapacityFromHashShift());
-  }
-}
-
 void
 PLDHashTable::Iterator::Next()
 {
@@ -833,12 +817,46 @@ PLDHashTable::Iterator::Next()
 
   // Advance to the next live entry, if there is one.
   if (!Done()) {
-    do {
-      MoveToNextEntry();
-    } while (IsOnNonLiveEntry());
+    MoveToNextLiveEntry();
   }
 }
 
+MOZ_ALWAYS_INLINE void
+PLDHashTable::Iterator::MoveToNextLiveEntry()
+{
+  // Chaos mode requires wraparound to cover all possible entries, so we can't
+  // simply move to the next live entry and stop when we hit the end of the
+  // entry store. But we don't want to introduce extra branches into our inner
+  // loop. So we are going to exploit the structure of the entry store in this
+  // method to implement an efficient inner loop.
+  //
+  // The idea is that since we are really only iterating through the stored
+  // hashes and because we know that there are a power-of-two number of
+  // hashes, we can use masking to implement the wraparound for us. This
+  // method does have the downside of needing to recalculate where the
+  // associated entry is once we've found it, but that seems OK.
+
+  // Our current slot and its associated hash.
+  Slot slot = mCurrent;
+  PLDHashNumber* p = slot.HashPtr();
+  const uint32_t capacity = mTable->CapacityFromHashShift();
+  const uint32_t mask = capacity - 1;
+  auto hashes = reinterpret_cast<PLDHashNumber*>(mTable->mEntryStore.Get());
+  uint32_t slotIndex = p - hashes;
+
+  do {
+    slotIndex = (slotIndex + 1) & mask;
+  } while (!Slot::IsLiveHash(hashes[slotIndex]));
+
+  // slotIndex now indicates where a live slot is. Rematerialize the entry
+  // and the slot.
+  auto entries = reinterpret_cast<char*>(&hashes[capacity]);
+  char* entryPtr = entries + slotIndex * mEntrySize;
+  auto entry = reinterpret_cast<PLDHashEntryHdr*>(entryPtr);
+
+  mCurrent = Slot(entry, &hashes[slotIndex]);
+}
+
 void
 PLDHashTable::Iterator::Remove()
 {

xpcom/ds/PLDHashTable.h

@@ -247,7 +247,8 @@ class PLDHashTable
 
     bool IsFree() const { return KeyHash() == 0; }
     bool IsRemoved() const { return KeyHash() == 1; }
-    bool IsLive() const { return KeyHash() >= 2; }
+    bool IsLive() const { return IsLiveHash(KeyHash()); }
+    static bool IsLiveHash(uint32_t aHash) { return aHash >= 2; }
 
     void MarkFree() { *HashPtr() = 0; }
     void MarkRemoved() { *HashPtr() = 1; }
@@ -259,9 +260,9 @@ class PLDHashTable
       mEntry = reinterpret_cast<PLDHashEntryHdr*>(p);
       mKeyHash++;
     }
-  private:
     PLDHashNumber* HashPtr() const { return mKeyHash; }
 
+  private:
     PLDHashEntryHdr* mEntry;
     PLDHashNumber* mKeyHash;
   };
@@ -613,7 +614,6 @@ class PLDHashTable
     PLDHashTable* mTable;             // Main table pointer.
 
   private:
-    Slot mLimit;                      // One past the last entry.
     Slot mCurrent;                    // Pointer to the current entry.
     uint32_t mNexts;                  // Number of Next() calls.
     uint32_t mNextsLimit;             // Next() call limit.
@@ -622,7 +622,8 @@ class PLDHashTable
     uint8_t mEntrySize;               // Size of entries.
 
     bool IsOnNonLiveEntry() const;
-    void MoveToNextEntry();
+
+    void MoveToNextLiveEntry();
 
     Iterator() = delete;
     Iterator(const Iterator&) = delete;