Bug 1478879 - Define Range/Enum in terms of Iterator/ModIterator. r=luke
authorNicholas Nethercote <nnethercote@mozilla.com>
Tue, 31 Jul 2018 10:23:03 +1000
changeset 429718 2c3b5f4eda625adf917db7402bc1d655d1c1e3bc
parent 429717 1e45ce266b665543f4388b96cd2a2ccc15359bda
child 429719 3bd2221b034d4a57fd97df85fc64a60c5ba3478b
push id105976
push usernnethercote@mozilla.com
push dateThu, 02 Aug 2018 01:55:00 +0000
treeherdermozilla-inbound@3c7c754ce8fd [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke
bugs1478879
milestone63.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 1478879 - Define Range/Enum in terms of Iterator/ModIterator. r=luke To reduce the code duplication.
mfbt/HashTable.h
--- a/mfbt/HashTable.h
+++ b/mfbt/HashTable.h
@@ -1361,159 +1361,71 @@ public:
         mTable.compactIfUnderloaded();
       }
     }
   };
 
   // Range is similar to Iterator, but uses different terminology.
   class Range
   {
-  protected:
     friend class HashTable;
 
-    Range(const HashTable& aTable, Entry* aCur, Entry* aEnd)
-      : mCur(aCur)
-      , mEnd(aEnd)
-#ifdef DEBUG
-      , mTable(aTable)
-      , mMutationCount(aTable.mMutationCount)
-      , mGeneration(aTable.generation())
-      , mValidEntry(true)
-#endif
+    Iterator mIter;
+
+  protected:
+    explicit Range(const HashTable& table)
+      : mIter(table)
     {
-      while (mCur < mEnd && !mCur->isLive()) {
-        ++mCur;
-      }
     }
 
-    Entry* mCur;
-    Entry* mEnd;
-#ifdef DEBUG
-    const HashTable& mTable;
-    uint64_t mMutationCount;
-    Generation mGeneration;
-    bool mValidEntry;
-#endif
-
   public:
-    bool empty() const
-    {
-#ifdef DEBUG
-      MOZ_ASSERT(mGeneration == mTable.generation());
-      MOZ_ASSERT(mMutationCount == mTable.mMutationCount);
-#endif
-      return mCur == mEnd;
-    }
+    bool empty() const { return mIter.done(); }
 
-    T& front() const
-    {
-      MOZ_ASSERT(!empty());
-#ifdef DEBUG
-      MOZ_ASSERT(mValidEntry);
-      MOZ_ASSERT(mGeneration == mTable.generation());
-      MOZ_ASSERT(mMutationCount == mTable.mMutationCount);
-#endif
-      return mCur->get();
-    }
+    T& front() const { return mIter.get(); }
 
-    void popFront()
-    {
-      MOZ_ASSERT(!empty());
-#ifdef DEBUG
-      MOZ_ASSERT(mGeneration == mTable.generation());
-      MOZ_ASSERT(mMutationCount == mTable.mMutationCount);
-#endif
-      while (++mCur < mEnd && !mCur->isLive()) {
-        continue;
-      }
-#ifdef DEBUG
-      mValidEntry = true;
-#endif
-    }
+    void popFront() { return mIter.next(); }
   };
 
   // Enum is similar to ModIterator, but uses different terminology.
-  class Enum : public Range
+  class Enum
   {
-    friend class HashTable;
-
-    HashTable& mTable;
-    bool mRekeyed;
-    bool mRemoved;
+    ModIterator mIter;
 
     // Enum is movable but not copyable.
     Enum(const Enum&) = delete;
     void operator=(const Enum&) = delete;
 
   public:
     template<class Map>
     explicit Enum(Map& map)
-      : Range(map.all())
-      , mTable(map.mImpl)
-      , mRekeyed(false)
-      , mRemoved(false)
+      : mIter(map.mImpl)
+    {
+    }
+
+    MOZ_IMPLICIT Enum(Enum&& other)
+      : mIter(std::move(other.mIter))
     {
     }
 
-    MOZ_IMPLICIT Enum(Enum&& aOther)
-      : Range(aOther)
-      , mTable(aOther.mTable)
-      , mRekeyed(aOther.mRekeyed)
-      , mRemoved(aOther.mRemoved)
-    {
-      aOther.mRekeyed = false;
-      aOther.mRemoved = false;
-    }
+    bool empty() const { return mIter.done(); }
+
+    T& front() const { return mIter.get(); }
 
-    void removeFront()
-    {
-      mTable.remove(*this->mCur);
-      mRemoved = true;
-#ifdef DEBUG
-      this->mValidEntry = false;
-      this->mMutationCount = mTable.mMutationCount;
-#endif
-    }
+    void popFront() { return mIter.next(); }
 
-    NonConstT& mutableFront()
-    {
-      MOZ_ASSERT(!this->empty());
-#ifdef DEBUG
-      MOZ_ASSERT(this->mValidEntry);
-      MOZ_ASSERT(this->mGeneration == this->Range::mTable.generation());
-      MOZ_ASSERT(this->mMutationCount == this->Range::mTable.mMutationCount);
-#endif
-      return this->mCur->getMutable();
-    }
+    void removeFront() { mIter.remove(); }
+
+    NonConstT& mutableFront() { return mIter.getMutable(); }
 
     void rekeyFront(const Lookup& aLookup, const Key& aKey)
     {
-      MOZ_ASSERT(&aKey != &HashPolicy::getKey(this->mCur->get()));
-      Ptr p(*this->mCur, mTable);
-      mTable.rekeyWithoutRehash(p, aLookup, aKey);
-      mRekeyed = true;
-#ifdef DEBUG
-      this->mValidEntry = false;
-      this->mMutationCount = mTable.mMutationCount;
-#endif
+      mIter.rekey(aLookup, aKey);
     }
 
-    void rekeyFront(const Key& aKey) { rekeyFront(aKey, aKey); }
-
-    ~Enum()
-    {
-      if (mRekeyed) {
-        mTable.mGen++;
-        mTable.checkOverRemoved();
-      }
-
-      if (mRemoved) {
-        mTable.compactIfUnderloaded();
-      }
-    }
+    void rekeyFront(const Key& aKey) { mIter.rekey(aKey); }
   };
 
   // HashTable is movable
   HashTable(HashTable&& aRhs)
     : AllocPolicy(aRhs)
   {
     PodAssign(this, &aRhs);
     aRhs.mTable = nullptr;
@@ -2112,17 +2024,17 @@ public:
   {
     MOZ_ASSERT(mTable);
     return ModIterator(*this);
   }
 
   Range all() const
   {
     MOZ_ASSERT(mTable);
-    return Range(*this, mTable, mTable + capacity());
+    return Range(*this);
   }
 
   bool empty() const
   {
     MOZ_ASSERT(mTable);
     return !mEntryCount;
   }