Bug 1872982 - Part 2: Partition inner view vector based on whether the view is in the nursery r=jandem

This patch changes the inner view vector to store all nursery views at the end,
and records the index of the first nursery view. This allows us to sweep only
the nursery views after minor GC.

For the page in question this reduces the proportion of time spent in GC from
100% to around 35%.

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

js/public/GCVector.h

@@ -174,11 +174,14 @@ class GCVector {
     return !empty();
   }
 
-  // Like eraseIf, but may mutate the contents of the vector.
+  // Like eraseIf, but may mutate the contents of the vector. Iterates from
+  // |startIndex| to the last element of the vector.
   template <typename Pred>
-  void mutableEraseIf(Pred pred) {
-    T* src = begin();
-    T* dst = begin();
+  void mutableEraseIf(Pred pred, size_t startIndex = 0) {
+    MOZ_ASSERT(startIndex <= length());
+
+    T* src = begin() + startIndex;
+    T* dst = src;
     while (src != end()) {
       if (!pred(*src)) {
         if (src != dst) {

js/src/vm/ArrayBufferObject.cpp

@@ -2092,54 +2092,112 @@ bool ArrayBufferObject::addView(JSContext* cx, ArrayBufferViewObject* view) {
  * InnerViewTable
  */
 
-constexpr size_t VIEW_LIST_MAX_LENGTH = 500;
+inline bool InnerViewTable::Views::empty() { return views.empty(); }
+
+inline bool InnerViewTable::Views::hasNurseryViews() {
+  return firstNurseryView < views.length();
+}
+
+bool InnerViewTable::Views::addView(ArrayBufferViewObject* view) {
+  // Add the view to the list, ensuring that all nursery views are at end.
+
+  if (!views.append(view)) {
+    return false;
+  }
+
+  if (!gc::IsInsideNursery(view)) {
+    // Move tenured views before |firstNurseryView|.
+    if (firstNurseryView != views.length() - 1) {
+      std::swap(views[firstNurseryView], views.back());
+    }
+    firstNurseryView++;
+  }
+
+  check();
+
+  return true;
+}
+
+bool InnerViewTable::Views::sweepAfterMinorGC(JSTracer* trc) {
+  return traceWeak(trc, firstNurseryView);
+}
+
+bool InnerViewTable::Views::traceWeak(JSTracer* trc, size_t startIndex) {
+  // Use |trc| to trace the view vector from |startIndex| to the end, removing
+  // dead views and updating |firstNurseryView|.
+
+  size_t index = startIndex;
+  bool sawNurseryView = false;
+  views.mutableEraseIf(
+      [&](auto& view) {
+        if (!JS::GCPolicy<ViewVector::ElementType>::traceWeak(trc, &view)) {
+          return true;
+        }
+
+        if (!sawNurseryView && gc::IsInsideNursery(view)) {
+          sawNurseryView = true;
+          firstNurseryView = index;
+        }
+
+        index++;
+        return false;
+      },
+      startIndex);
+
+  if (!sawNurseryView) {
+    firstNurseryView = views.length();
+  }
+
+  check();
+
+  return !views.empty();
+}
+
+inline void InnerViewTable::Views::check() {
+#ifdef DEBUG
+  MOZ_ASSERT(firstNurseryView <= views.length());
+  if (views.length() < 100) {
+    for (size_t i = 0; i < views.length(); i++) {
+      MOZ_ASSERT(gc::IsInsideNursery(views[i]) == (i >= firstNurseryView));
+    }
+  }
+#endif
+}
 
 bool InnerViewTable::addView(JSContext* cx, ArrayBufferObject* buffer,
                              ArrayBufferViewObject* view) {
   // ArrayBufferObject entries are only added when there are multiple views.
   MOZ_ASSERT(buffer->firstView());
-
-  auto ptr = map.lookupForAdd(buffer);
-
   MOZ_ASSERT(!gc::IsInsideNursery(buffer));
-  bool addToNursery = nurseryKeysValid && gc::IsInsideNursery(view);
 
-  if (ptr) {
-    ViewVector& views = ptr->value();
-    MOZ_ASSERT(!views.empty());
+  // Ensure the buffer is present in the map, getting the list of views.
+  auto ptr = map.lookupForAdd(buffer);
+  if (!ptr && !map.add(ptr, buffer, Views(cx->zone()))) {
+    ReportOutOfMemory(cx);
+    return false;
+  }
+  Views& views = ptr->value();
 
-    if (addToNursery) {
-      // Only add the entry to |nurseryKeys| if it isn't already there.
-      if (views.length() >= VIEW_LIST_MAX_LENGTH) {
-        // To avoid quadratic blowup, skip the loop below if we end up
-        // adding enormous numbers of views for the same object.
-        nurseryKeysValid = false;
-      } else {
-        for (size_t i = 0; i < views.length(); i++) {
-          if (gc::IsInsideNursery(views[i])) {
-            addToNursery = false;
-            break;
-          }
-        }
-      }
-    }
-
-    if (!views.append(view)) {
-      ReportOutOfMemory(cx);
-      return false;
-    }
-  } else {
-    if (!map.add(ptr, buffer, ViewVector(cx->zone()))) {
-      ReportOutOfMemory(cx);
-      return false;
-    }
-    // ViewVector has one inline element, so the first insertion is
-    // guaranteed to succeed.
-    MOZ_ALWAYS_TRUE(ptr->value().append(view));
+  bool isNurseryView = gc::IsInsideNursery(view);
+  bool hadNurseryViews = views.hasNurseryViews();
+  if (!views.addView(view)) {
+    ReportOutOfMemory(cx);
+    return false;
   }
 
-  if (addToNursery && !nurseryKeys.append(buffer)) {
-    nurseryKeysValid = false;
+  // If we added the first nursery view, add the buffer to the list of buffers
+  // which have nursery views.
+  if (isNurseryView && !hadNurseryViews && nurseryKeysValid) {
+#ifdef DEBUG
+    if (nurseryKeys.length() < 100) {
+      for (auto* key : nurseryKeys) {
+        MOZ_ASSERT(key != buffer);
+      }
+    }
+#endif
+    if (!nurseryKeys.append(buffer)) {
+      nurseryKeysValid = false;
+    }
   }
 
   return true;
@@ -2149,7 +2207,7 @@ InnerViewTable::ViewVector* InnerViewTable::maybeViewsUnbarriered(
     ArrayBufferObject* buffer) {
   auto ptr = map.lookup(buffer);
   if (ptr) {
-    return &ptr->value();
+    return &ptr->value().views;
   }
   return nullptr;
 }
@@ -2168,16 +2226,20 @@ void InnerViewTable::sweepAfterMinorGC(JSTracer* trc) {
 
   if (nurseryKeysValid) {
     for (size_t i = 0; i < nurseryKeys.length(); i++) {
-      ArrayBufferObject* buffer = MaybeForwarded(nurseryKeys[i]);
+      ArrayBufferObject* buffer = nurseryKeys[i];
+      MOZ_ASSERT(!gc::IsInsideNursery(buffer));
       auto ptr = map.lookup(buffer);
-      using Policy = ArrayBufferViewMap::EntryGCPolicy;
-      if (ptr && !Policy::traceWeak(trc, &ptr->mutableKey(), &ptr->value())) {
+      if (ptr && !ptr->value().sweepAfterMinorGC(trc)) {
         map.remove(ptr);
       }
     }
   } else {
-    // Do the required sweeping by looking at every map entry.
-    map.traceWeak(trc);
+    for (ArrayBufferViewMap::Enum e(map); !e.empty(); e.popFront()) {
+      MOZ_ASSERT(!gc::IsInsideNursery(e.front().key()));
+      if (!e.front().value().sweepAfterMinorGC(trc)) {
+        e.removeFront();
+      }
+    }
   }
 
   nurseryKeys.clear();
@@ -2187,7 +2249,7 @@ void InnerViewTable::sweepAfterMinorGC(JSTracer* trc) {
 size_t InnerViewTable::sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf) {
   size_t vectorSize = 0;
   for (auto r = map.all(); !r.empty(); r.popFront()) {
-    vectorSize += r.front().value().sizeOfExcludingThis(mallocSizeOf);
+    vectorSize += r.front().value().views.sizeOfExcludingThis(mallocSizeOf);
   }
 
   return vectorSize + map.shallowSizeOfExcludingThis(mallocSizeOf) +

js/src/vm/ArrayBufferObject.h

@@ -551,8 +551,25 @@ ArrayBufferObjectMaybeShared* CreateWasmBuffer(JSContext* cx,
 // Per-compartment table that manages the relationship between array buffers
 // and the views that use their storage.
 class InnerViewTable {
+  // Store views in a vector such that all the tenured views come before any
+  // nursery views. Maintain the index of the first nursery view so there is an
+  // efficient way to access only the nursery views.
   using ViewVector =
       GCVector<UnsafeBarePtr<ArrayBufferViewObject*>, 1, ZoneAllocPolicy>;
+  struct Views {
+    ViewVector views;  // List of views with tenured views at the front.
+    size_t firstNurseryView = 0;
+
+    explicit Views(JS::Zone* zone) : views(zone) {}
+    bool empty();
+    bool hasNurseryViews();
+    bool addView(ArrayBufferViewObject* view);
+
+    bool traceWeak(JSTracer* trc, size_t startIndex = 0);
+    bool sweepAfterMinorGC(JSTracer* trc);
+
+    void check();
+  };
 
   // For all objects sharing their storage with some other view, this maps
   // the object to the list of such views. All entries in this map are weak.
@@ -566,7 +583,7 @@ class InnerViewTable {
   // live. Special support is required in the minor GC, implemented in
   // sweepAfterMinorGC.
   using ArrayBufferViewMap =
-      GCHashMap<UnsafeBarePtr<ArrayBufferObject*>, ViewVector,
+      GCHashMap<UnsafeBarePtr<ArrayBufferObject*>, Views,
                 StableCellHasher<JSObject*>, ZoneAllocPolicy>;
   ArrayBufferViewMap map;