Bug 1460674 - part 0a - store the hash table entry size in iterators; r=njn
authorNathan Froyd <froydnj@mozilla.com>
Mon, 26 Nov 2018 16:24:50 -0500
changeset 504495 9869912ed6d74ea4ce63f976e93ce1b3b361f884
parent 504494 81cde2d265ab24055b77d12899e51f979cfc2043
child 504496 927af324b93d51639dc24c09e6bb0eb9ace40613
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnjn
bugs1460674
milestone65.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 1460674 - part 0a - store the hash table entry size in iterators; r=njn This change is satisfying insofar as it removes a load from every iteration step over the hashtable, but it doesn't seem to affect our collection benchmarks in any way. The change does not make iterators any larger, as there is padding at the end of the iterator structure on both 32- and 64-bit platforms.
xpcom/ds/PLDHashTable.cpp
xpcom/ds/PLDHashTable.h
--- a/xpcom/ds/PLDHashTable.cpp
+++ b/xpcom/ds/PLDHashTable.cpp
@@ -723,33 +723,36 @@ PLDHashTable::ShallowSizeOfIncludingThis
 
 PLDHashTable::Iterator::Iterator(Iterator&& aOther)
   : mTable(aOther.mTable)
   , mLimit(aOther.mLimit)
   , mCurrent(aOther.mCurrent)
   , mNexts(aOther.mNexts)
   , mNextsLimit(aOther.mNextsLimit)
   , mHaveRemoved(aOther.mHaveRemoved)
+  , mEntrySize(aOther.mEntrySize)
 {
   // No need to change |mChecker| here.
   aOther.mTable = nullptr;
   aOther.mLimit = nullptr;
   aOther.mCurrent = nullptr;
   aOther.mNexts = 0;
   aOther.mNextsLimit = 0;
   aOther.mHaveRemoved = false;
+  aOther.mEntrySize = 0;
 }
 
 PLDHashTable::Iterator::Iterator(PLDHashTable* aTable)
   : mTable(aTable)
   , mLimit(mTable->mEntryStore.Get() + mTable->Capacity() * mTable->mEntrySize)
   , mCurrent(mTable->mEntryStore.Get())
   , mNexts(0)
   , mNextsLimit(mTable->EntryCount())
   , mHaveRemoved(false)
+  , mEntrySize(aTable->mEntrySize)
 {
 #ifdef DEBUG
   mTable->mChecker.StartReadOp();
 #endif
 
   if (ChaosMode::isActive(ChaosFeature::HashTableIteration) &&
       mTable->Capacity() > 0) {
     // Start iterating at a random entry. It would be even more chaotic to
@@ -783,17 +786,17 @@ PLDHashTable::Iterator::IsOnNonLiveEntry
 {
   MOZ_ASSERT(!Done());
   return !EntryIsLive(reinterpret_cast<PLDHashEntryHdr*>(mCurrent));
 }
 
 MOZ_ALWAYS_INLINE void
 PLDHashTable::Iterator::MoveToNextEntry()
 {
-  mCurrent += mTable->mEntrySize;
+  mCurrent += mEntrySize;
   if (mCurrent == mLimit) {
     mCurrent = mTable->mEntryStore.Get();  // Wrap-around. Possible due to Chaos Mode.
   }
 }
 
 void
 PLDHashTable::Iterator::Next()
 {
--- a/xpcom/ds/PLDHashTable.h
+++ b/xpcom/ds/PLDHashTable.h
@@ -486,16 +486,17 @@ public:
 
   private:
     char* mLimit;                     // One past the last entry.
     char* mCurrent;                   // Pointer to the current entry.
     uint32_t mNexts;                  // Number of Next() calls.
     uint32_t mNextsLimit;             // Next() call limit.
 
     bool mHaveRemoved;                // Have any elements been removed?
+    uint8_t mEntrySize;               // Size of entries.
 
     bool IsOnNonLiveEntry() const;
     void MoveToNextEntry();
 
     Iterator() = delete;
     Iterator(const Iterator&) = delete;
     Iterator& operator=(const Iterator&) = delete;
     Iterator& operator=(const Iterator&&) = delete;