Bug 1730534 - Part 3: Add an iterator for JSHolderMap r=mccr8

This adds an iterator over JSHolderMap and replaces use of the ForEach method
with it.

Differential Revision: https://phabricator.services.mozilla.com/D125429

xpcom/base/CycleCollectedJSRuntime.cpp

@@ -498,35 +498,37 @@ void JSHolderMap::EntryVectorIter::Settle() {
   }
 }
 
-JSHolderMap::JSHolderMap() : mJSHolderMap(256) {}
-
-template <typename F>
-inline void JSHolderMap::ForEach(F&& f, WhichHolders aWhich) {
-  // Multi-zone JS holders must always be considered.
-  for (EntryVectorIter entry(*this, mAnyZoneJSHolders); !entry.Done();
-       entry.Next()) {
-    f(entry->mHolder, entry->mTracer, nullptr);
-  }
-
-  for (auto i = mPerZoneJSHolders.modIter(); !i.done(); i.next()) {
-    if (aWhich == HoldersRequiredForGrayMarking &&
-        !JS::NeedGrayRootsForZone(i.get().key())) {
-      continue;
-    }
-
+inline JSHolderMap::Iter::Iter(JSHolderMap& aMap, WhichHolders aWhich)
+    : mHolderMap(aMap), mIter(aMap, aMap.mAnyZoneJSHolders) {
+  // Populate vector of zones to iterate after the any-zone holders.
+  for (auto i = aMap.mPerZoneJSHolders.iter(); !i.done(); i.next()) {
     JS::Zone* zone = i.get().key();
-    EntryVector* holders = i.get().value().get();
-    for (EntryVectorIter entry(*this, *holders); !entry.Done(); entry.Next()) {
-      MOZ_ASSERT(entry->mZone == zone);
-      f(entry->mHolder, entry->mTracer, zone);
+    if (aWhich == AllHolders || JS::NeedGrayRootsForZone(i.get().key())) {
+      MOZ_ALWAYS_TRUE(mZones.append(zone));
+    }
   }
 
-    if (holders->IsEmpty()) {
-      i.remove();
+  Settle();
+}
+
+void JSHolderMap::Iter::Settle() {
+  while (mIter.Done()) {
+    if (mZone && mIter.Vector().IsEmpty()) {
+      mHolderMap.mPerZoneJSHolders.remove(mZone);
     }
+
+    if (mZones.empty()) {
+      break;
+    }
+
+    mZone = mZones.popCopy();
+    EntryVector& vector = *mHolderMap.mPerZoneJSHolders.lookup(mZone)->value();
+    new (&mIter) EntryVectorIter(mHolderMap, vector);
   }
 }
 
+JSHolderMap::JSHolderMap() : mJSHolderMap(256) {}
+
 bool JSHolderMap::RemoveEntry(EntryVector& aJSHolders, Entry* aEntry) {
   MOZ_ASSERT(aEntry);
   MOZ_ASSERT(!aEntry->mHolder);
@@ -765,8 +767,9 @@ size_t CycleCollectedJSRuntime::SizeOfExcludingThis(
 }
 
 void CycleCollectedJSRuntime::UnmarkSkippableJSHolders() {
-  mJSHolders.ForEach([](void* holder, nsScriptObjectTracer* tracer,
-                        JS::Zone* zone) { tracer->CanSkip(holder, true); });
+  for (JSHolderMap::Iter entry(mJSHolders); !entry.Done(); entry.Next()) {
+    entry->mTracer->CanSkip(entry->mHolder, true);
+  }
 }
 
 void CycleCollectedJSRuntime::DescribeGCThing(
@@ -949,8 +952,10 @@ void CycleCollectedJSRuntime::TraverseNativeRoots(
   // would hurt to do this after the JS holders.
   TraverseAdditionalNativeRoots(aCb);
 
-  mJSHolders.ForEach(
-      [&aCb](void* holder, nsScriptObjectTracer* tracer, JS::Zone* zone) {
+  for (JSHolderMap::Iter entry(mJSHolders); !entry.Done(); entry.Next()) {
+    void* holder = entry->mHolder;
+    nsScriptObjectTracer* tracer = entry->mTracer;
+
     bool noteRoot = false;
     if (MOZ_UNLIKELY(aCb.WantAllTraces())) {
       noteRoot = true;
@@ -963,7 +968,7 @@ void CycleCollectedJSRuntime::TraverseNativeRoots(
     if (noteRoot) {
       aCb.NoteNativeRoot(holder, tracer);
     }
-      });
+  }
 }
 
 /* static */
@@ -1398,19 +1403,21 @@ void CycleCollectedJSRuntime::TraceNativeGrayRoots(
   TraceAdditionalNativeGrayRoots(aTracer);
 
   bool checkSingleZoneHolders = ShouldCheckSingleZoneHolders();
-  mJSHolders.ForEach(
-      [aTracer, checkSingleZoneHolders](
-          void* holder, nsScriptObjectTracer* tracer, JS::Zone* zone) {
+  for (JSHolderMap::Iter entry(mJSHolders, aWhich); !entry.Done();
+       entry.Next()) {
+    void* holder = entry->mHolder;
+    nsScriptObjectTracer* tracer = entry->mTracer;
+
 #ifdef CHECK_SINGLE_ZONE_JS_HOLDERS
     if (checkSingleZoneHolders && !tracer->IsMultiZoneJSHolder()) {
-          CheckHolderIsSingleZone(holder, tracer, zone);
+      CheckHolderIsSingleZone(holder, tracer, entry.Zone());
     }
 #else
     Unused << checkSingleZoneHolders;
 #endif
+
     tracer->Trace(holder, JsGcTracer(), aTracer);
-      },
-      aWhich);
+  }
 }
 
 void CycleCollectedJSRuntime::AddJSHolder(void* aHolder,

xpcom/base/CycleCollectedJSRuntime.h

@@ -90,11 +90,9 @@ class JSHolderMap {
  public:
   enum WhichHolders { AllHolders, HoldersRequiredForGrayMarking };
 
-  JSHolderMap();
+  class Iter;
 
-  // Call functor |f| for each holder.
-  template <typename F>
-  void ForEach(F&& f, WhichHolders aWhich = AllHolders);
+  JSHolderMap();
 
   bool Has(void* aHolder) const;
   nsScriptObjectTracer* Get(void* aHolder) const;
@@ -172,6 +170,31 @@ class JSHolderMap::EntryVectorIter {
   EntryVector::IterImpl mIter;
 };
 
+class JSHolderMap::Iter {
+ public:
+  explicit Iter(JSHolderMap& aMap, WhichHolders aWhich = AllHolders);
+
+  bool Done() const { return mIter.Done(); }
+  const Entry& Get() const { return mIter.Get(); }
+  void Next() {
+    mIter.Next();
+    Settle();
+  }
+
+  operator const Entry*() const { return &Get(); }
+  const Entry* operator->() const { return &Get(); }
+
+  JS::Zone* Zone() const { return mZone; }
+
+ private:
+  void Settle();
+
+  JSHolderMap& mHolderMap;
+  Vector<JS::Zone*, 1, InfallibleAllocPolicy> mZones;
+  JS::Zone* mZone = nullptr;
+  EntryVectorIter mIter;
+};
+
 class CycleCollectedJSRuntime {
   friend class JSGCThingParticipant;
   friend class JSZoneParticipant;