Bug 1623973 - Make FinalizationRegistryObject active record set and registrations map values weak r=sfink

This removes tracing for the values of the registration map and the active record set and sweeps them instead. This requires maintaining a map of FinalizationRegistryObjects on the zone.

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

--HG--
extra : moz-landing-system : lando

js/src/builtin/FinalizationRegistryObject.cpp

@@ -151,7 +151,7 @@ const JSClassOps FinalizationRecordVectorObject::classOps_ = {
     nullptr,                                   // call
     nullptr,                                   // hasInstance
     nullptr,                                   // construct
-    FinalizationRecordVectorObject::trace,     // trace
+    nullptr,                                   // trace
 };
 
 /* static */
@@ -174,14 +174,6 @@ FinalizationRecordVectorObject* FinalizationRecordVectorObject::create(
   return object;
 }
 
-/* static */
-void FinalizationRecordVectorObject::trace(JSTracer* trc, JSObject* obj) {
-  auto rv = &obj->as<FinalizationRecordVectorObject>();
-  if (auto* records = rv->records()) {
-    records->trace(trc);
-  }
-}
-
 /* static */
 void FinalizationRecordVectorObject::finalize(JSFreeOp* fop, JSObject* obj) {
   auto rv = &obj->as<FinalizationRecordVectorObject>();
@@ -224,6 +216,11 @@ inline void FinalizationRecordVectorObject::remove(
   records()->eraseIfEqual(record);
 }
 
+inline void FinalizationRecordVectorObject::sweep() {
+  MOZ_ASSERT(records());
+  return records()->sweep();
+}
+
 ///////////////////////////////////////////////////////////////////////////
 // FinalizationRegistryObject
 
@@ -329,6 +326,10 @@ bool FinalizationRegistryObject::construct(JSContext* cx, unsigned argc,
   registry->initReservedSlot(IsQueuedForCleanupSlot, BooleanValue(false));
   registry->initReservedSlot(IsCleanupJobActiveSlot, BooleanValue(false));
 
+  if (!cx->runtime()->gc.addFinalizationRegistry(cx, registry)) {
+    return false;
+  }
+
   args.rval().setObject(*registry);
   return true;
 }
@@ -336,17 +337,40 @@ bool FinalizationRegistryObject::construct(JSContext* cx, unsigned argc,
 /* static */
 void FinalizationRegistryObject::trace(JSTracer* trc, JSObject* obj) {
   auto registry = &obj->as<FinalizationRegistryObject>();
+
+  // Trace the registrations weak map. At most this traces the
+  // FinalizationRecordVectorObject values of the map; the contents of those
+  // objects are weakly held and are not traced.
   if (ObjectWeakMap* registrations = registry->registrations()) {
     registrations->trace(trc);
   }
-  if (FinalizationRecordSet* records = registry->activeRecords()) {
-    records->trace(trc);
-  }
+
+  // The active record set is weakly held and is not traced.
+
   if (FinalizationRecordVector* records = registry->recordsToBeCleanedUp()) {
     records->trace(trc);
   }
 }
 
+void FinalizationRegistryObject::sweep() {
+  // Sweep the set of active records. These may die if CCWs to record objects
+  // get nuked.
+  MOZ_ASSERT(activeRecords());
+  activeRecords()->sweep();
+
+  // Sweep the contents of the registrations weak map's values.
+  MOZ_ASSERT(registrations());
+  for (ObjectValueWeakMap::Enum e(registrations()->valueMap()); !e.empty();
+       e.popFront()) {
+    auto registrations =
+        &e.front().value().toObject().as<FinalizationRecordVectorObject>();
+    registrations->sweep();
+    if (registrations->isEmpty()) {
+      e.removeFront();
+    }
+  }
+}
+
 /* static */
 void FinalizationRegistryObject::finalize(JSFreeOp* fop, JSObject* obj) {
   auto registry = &obj->as<FinalizationRegistryObject>();
@@ -354,7 +378,7 @@ void FinalizationRegistryObject::finalize(JSFreeOp* fop, JSObject* obj) {
   // Clear the weak pointer to the registry in all remaining records.
 
   // FinalizationRegistries are foreground finalized whereas record objects are
-  // background finalized, so record objects and guaranteed to still be
+  // background finalized, so record objects are guaranteed to still be
   // accessible at this point.
   MOZ_ASSERT(registry->getClass()->flags & JSCLASS_FOREGROUND_FINALIZE);
 

js/src/builtin/FinalizationRegistryObject.h

@@ -135,8 +135,10 @@ class FinalizationRecordObject : public NativeObject {
 using FinalizationRecordVector =
     GCVector<HeapPtr<FinalizationRecordObject*>, 1, js::ZoneAllocPolicy>;
 
-// A JS object that wraps a FinalizationRecordVector. Used as the values in the
-// registration weakmap.
+// A JS object containing a vector of FinalizationRecordObjects, which holds the
+// records corresponding to the registrations for a particular registration
+// token. These are used as the values in the registration weakmap. The contents
+// of the vector are weak references and are not traced.
 class FinalizationRecordVectorObject : public NativeObject {
   enum { RecordsSlot = 0, SlotCount };
 
@@ -153,6 +155,8 @@ class FinalizationRecordVectorObject : public NativeObject {
   bool append(HandleFinalizationRecordObject record);
   void remove(HandleFinalizationRecordObject record);
 
+  void sweep();
+
  private:
   static const JSClassOps classOps_;
 
@@ -192,6 +196,8 @@ class FinalizationRegistryObject : public NativeObject {
   void setQueuedForCleanup(bool value);
   void setCleanupJobActive(bool value);
 
+  void sweep();
+
   static bool cleanupQueuedRecords(JSContext* cx,
                                    HandleFinalizationRegistryObject registry,
                                    HandleObject callback = nullptr);

js/src/gc/FinalizationRegistry.cpp

@@ -11,12 +11,23 @@
 #include "builtin/FinalizationRegistryObject.h"
 #include "gc/GCRuntime.h"
 #include "gc/Zone.h"
+#include "vm/JSContext.h"
 
 #include "gc/PrivateIterators-inl.h"
 
 using namespace js;
 using namespace js::gc;
 
+bool GCRuntime::addFinalizationRegistry(JSContext* cx,
+                                        FinalizationRegistryObject* registry) {
+  if (!cx->zone()->finalizationRegistries().put(registry)) {
+    ReportOutOfMemory(cx);
+    return false;
+  }
+
+  return true;
+}
+
 bool GCRuntime::registerWithFinalizationRegistry(JSContext* cx,
                                                  HandleObject target,
                                                  HandleObject record) {
@@ -67,6 +78,12 @@ void GCRuntime::sweepFinalizationRegistries(Zone* zone) {
   // Sweep finalization registry data and queue finalization records for cleanup
   // for any entries whose target is dying and remove them from the map.
 
+  Zone::FinalizationRegistrySet& set = zone->finalizationRegistries();
+  set.sweep();
+  for (auto r = set.all(); !r.empty(); r.popFront()) {
+    r.front()->as<FinalizationRegistryObject>().sweep();
+  }
+
   Zone::FinalizationRecordMap& map = zone->finalizationRecordMap();
   for (Zone::FinalizationRecordMap::Enum e(map); !e.empty(); e.popFront()) {
     FinalizationRecordVector& records = e.front().value();

js/src/gc/GC.cpp

@@ -4956,8 +4956,9 @@ void GCRuntime::sweepWeakRefs() {
 void GCRuntime::sweepFinalizationRegistriesOnMainThread() {
   // This calls back into the browser which expects to be called from the main
   // thread.
-  gcstats::AutoPhase ap(stats(),
-                        gcstats::PhaseKind::SWEEP_FINALIZATION_REGISTRIES);
+  gcstats::AutoPhase ap1(stats(), gcstats::PhaseKind::SWEEP_COMPARTMENTS);
+  gcstats::AutoPhase ap2(stats(),
+                         gcstats::PhaseKind::SWEEP_FINALIZATION_REGISTRIES);
   for (SweepGroupZonesIter zone(this); !zone.done(); zone.next()) {
     sweepFinalizationRegistries(zone);
   }
@@ -5242,7 +5243,6 @@ IncrementalProgress GCRuntime::beginSweepingSweepGroup(JSFreeOp* fop,
     {
       AutoUnlockHelperThreadState unlock(lock);
       sweepJitDataOnMainThread(fop);
-      sweepFinalizationRegistriesOnMainThread();
     }
 
     for (auto& task : sweepCacheTasks) {
@@ -5254,6 +5254,10 @@ IncrementalProgress GCRuntime::beginSweepingSweepGroup(JSFreeOp* fop,
     startSweepingAtomsTable();
   }
 
+  // FinalizationRegistry sweeping touches weak maps and so must not run in
+  // parallel with that.
+  sweepFinalizationRegistriesOnMainThread();
+
   // Queue all GC things in all zones for sweeping, either on the foreground
   // or on the background thread.
 

js/src/gc/GCRuntime.h

@@ -436,6 +436,8 @@ class GCRuntime {
   JS::DoCycleCollectionCallback setDoCycleCollectionCallback(
       JS::DoCycleCollectionCallback callback);
 
+  bool addFinalizationRegistry(JSContext* cx,
+                               FinalizationRegistryObject* registry);
   bool registerWithFinalizationRegistry(JSContext* cx, HandleObject target,
                                         HandleObject record);
   bool cleanupQueuedFinalizationRegistry(

js/src/gc/WeakMap.h

@@ -313,6 +313,8 @@ class ObjectWeakMap {
     return mallocSizeOf(this) + sizeOfExcludingThis(mallocSizeOf);
   }
 
+  ObjectValueWeakMap& valueMap() { return map; }
+
 #ifdef JSGC_HASH_TABLE_CHECKS
   void checkAfterMovingGC();
 #endif

js/src/gc/Zone.cpp

@@ -162,6 +162,7 @@ JS::Zone::Zone(JSRuntime* rt)
       baseShapes_(this, this),
       initialShapes_(this, this),
       nurseryShapes_(this),
+      finalizationRegistries_(this, this),
       finalizationRecordMap_(this, this),
       jitZone_(this, nullptr),
       gcScheduled_(false),

js/src/gc/Zone.h

@@ -319,6 +319,12 @@ class Zone : public js::ZoneAllocator, public js::gc::GraphNodeBase<JS::Zone> {
       js::Vector<js::AccessorShape*, 0, js::SystemAllocPolicy>;
   js::ZoneData<NurseryShapeVector> nurseryShapes_;
 
+  // The set of all finalization registries in this zone.
+  using FinalizationRegistrySet =
+      GCHashSet<js::HeapPtrObject, js::MovableCellHasher<js::HeapPtrObject>,
+                js::ZoneAllocPolicy>;
+  js::ZoneOrGCTaskData<FinalizationRegistrySet> finalizationRegistries_;
+
   // A map from finalization registry targets to a list of finalization records
   // representing registries that the target is registered with and their
   // associated held values.
@@ -685,6 +691,10 @@ class Zone : public js::ZoneAllocator, public js::gc::GraphNodeBase<JS::Zone> {
 
   void sweepWeakKeysAfterMinorGC();
 
+  FinalizationRegistrySet& finalizationRegistries() {
+    return finalizationRegistries_.ref();
+  }
+
   FinalizationRecordMap& finalizationRecordMap() {
     return finalizationRecordMap_.ref();
   }