Bug 1736021 - Replace InnerViewTable::sweepEntry with use of the standard sweep policy r=sfink
authorJon Coppeard <jcoppeard@mozilla.com>
Fri, 15 Oct 2021 16:21:25 +0000
changeset 596023 032f4f99161cfd4b8d64d3af21c5744101711207
parent 596022 3a02803e1d29b9124b3e0a95721b8eb9878f8059
child 596024 09ace42c5e87ccb3d892e9de4b9334023374e402
push id38881
push userimoraru@mozilla.com
push dateFri, 15 Oct 2021 21:35:21 +0000
treeherdermozilla-central@ff6d6594f7b0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1736021
milestone95.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 1736021 - Replace InnerViewTable::sweepEntry with use of the standard sweep policy r=sfink Now we can expose the map's sweep policy and use this to replace sweepEntry entirely. Differential Revision: https://phabricator.services.mozilla.com/D128606
js/public/GCHashTable.h
js/src/vm/ArrayBufferObject.cpp
js/src/vm/ArrayBufferObject.h
--- a/js/public/GCHashTable.h
+++ b/js/public/GCHashTable.h
@@ -60,16 +60,18 @@ struct DefaultMapSweepPolicy {
 template <typename Key, typename Value,
           typename HashPolicy = js::DefaultHasher<Key>,
           typename AllocPolicy = js::TempAllocPolicy,
           typename MapSweepPolicy = DefaultMapSweepPolicy<Key, Value>>
 class GCHashMap : public js::HashMap<Key, Value, HashPolicy, AllocPolicy> {
   using Base = js::HashMap<Key, Value, HashPolicy, AllocPolicy>;
 
  public:
+  using SweepPolicy = MapSweepPolicy;
+
   explicit GCHashMap(AllocPolicy a = AllocPolicy()) : Base(std::move(a)) {}
   explicit GCHashMap(size_t length) : Base(length) {}
   GCHashMap(AllocPolicy a, size_t length) : Base(std::move(a), length) {}
 
   void trace(JSTracer* trc) {
     for (typename Base::Enum e(*this); !e.empty(); e.popFront()) {
       GCPolicy<Value>::trace(trc, &e.front().value(), "hashmap value");
       GCPolicy<Key>::trace(trc, &e.front().mutableKey(), "hashmap key");
--- a/js/src/vm/ArrayBufferObject.cpp
+++ b/js/src/vm/ArrayBufferObject.cpp
@@ -1719,43 +1719,27 @@ InnerViewTable::ViewVector* InnerViewTab
 
 void InnerViewTable::removeViews(ArrayBufferObject* buffer) {
   Map::Ptr p = map.lookup(buffer);
   MOZ_ASSERT(p);
 
   map.remove(p);
 }
 
-/* static */
-bool InnerViewTable::sweepEntry(JSObject** pkey, ViewVector& views) {
-  if (IsAboutToBeFinalizedUnbarriered(pkey)) {
-    return true;
-  }
-
-  MOZ_ASSERT(!views.empty());
-  views.sweep();
-
-  return views.empty();
-}
-
 void InnerViewTable::sweep() { map.sweep(); }
 
 void InnerViewTable::sweepAfterMinorGC() {
   MOZ_ASSERT(needsSweepAfterMinorGC());
 
   if (nurseryKeysValid) {
     for (size_t i = 0; i < nurseryKeys.length(); i++) {
       JSObject* buffer = MaybeForwarded(nurseryKeys[i]);
       Map::Ptr p = map.lookup(buffer);
-      if (!p) {
-        continue;
-      }
-
-      if (sweepEntry(&p->mutableKey(), p->value())) {
-        map.remove(buffer);
+      if (p && Map::SweepPolicy::needsSweep(&p->mutableKey(), &p->value())) {
+        map.remove(p);
       }
     }
     nurseryKeys.clear();
   } else {
     // Do the required sweeping by looking at every map entry.
     nurseryKeys.clear();
     sweep();
 
--- a/js/src/vm/ArrayBufferObject.h
+++ b/js/src/vm/ArrayBufferObject.h
@@ -521,50 +521,41 @@ bool CreateWasmBuffer32(JSContext* cx, c
 // and the views that use their storage.
 class InnerViewTable {
  public:
   using ViewVector = GCVector<UnsafeBarePtr<JSObject*>, 1, ZoneAllocPolicy>;
 
   friend class ArrayBufferObject;
 
  private:
-  struct MapGCPolicy {
-    static bool needsSweep(JSObject** key, ViewVector* value) {
-      return InnerViewTable::sweepEntry(key, *value);
-    }
-  };
-
   // This key is a raw pointer and not a WeakHeapPtr because the post-barrier
   // would hold nursery-allocated entries live unconditionally. It is a very
   // common pattern in low-level and performance-oriented JavaScript to create
   // hundreds or thousands of very short lived temporary views on a larger
-  // buffer; having to tenured all of these would be a catastrophic performance
+  // buffer; having to tenure all of these would be a catastrophic performance
   // regression. Thus, it is vital that nursery pointers in this map not be held
   // live. Special support is required in the minor GC, implemented in
   // sweepAfterMinorGC.
-  using Map = GCHashMap<JSObject*, ViewVector, MovableCellHasher<JSObject*>,
-                        ZoneAllocPolicy, MapGCPolicy>;
+  using Map = GCHashMap<UnsafeBarePtr<JSObject*>, ViewVector,
+                        MovableCellHasher<JSObject*>, ZoneAllocPolicy>;
 
   // 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.
   Map map;
 
   // List of keys from innerViews where either the source or at least one
   // target is in the nursery. The raw pointer to a JSObject is allowed here
   // because this vector is cleared after every minor collection. Users in
   // sweepAfterMinorCollection must be careful to use MaybeForwarded before
   // touching these pointers.
   Vector<JSObject*, 0, SystemAllocPolicy> nurseryKeys;
 
   // Whether nurseryKeys is a complete list.
   bool nurseryKeysValid;
 
-  // Sweep an entry during GC, returning whether the entry should be removed.
-  static bool sweepEntry(JSObject** pkey, ViewVector& views);
-
   bool addView(JSContext* cx, ArrayBufferObject* buffer, JSObject* view);
   ViewVector* maybeViewsUnbarriered(ArrayBufferObject* obj);
   void removeViews(ArrayBufferObject* obj);
 
  public:
   explicit InnerViewTable(Zone* zone) : map(zone), nurseryKeysValid(true) {}
 
   // Remove references to dead objects in the table and update table entries