[weakrefs] Clean up FinalizationGroups in FIFO order

Currently dirty FinalizationGroups are processed by the cleanup task in
LIFO order. This results in starvation when FinalizationGroups are added
to the dirty list faster than the cleanup task is run.

R=ulan@chromium.org

Bug: v8:8179
Change-Id: I6e4a5bbd490396120b07ca6053176beded7cef6e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2051619Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Shu-yu Guo <syg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66296}

src/api/api.cc

@@ -884,6 +884,8 @@ StartupData SnapshotCreator::CreateBlob(
   startup_serializer.SerializeWeakReferencesAndDeferred();
   can_be_rehashed = can_be_rehashed && startup_serializer.can_be_rehashed();
 
+  startup_serializer.CheckNoDirtyFinalizationGroups();
+
   read_only_serializer.FinalizeSerialization();
   can_be_rehashed = can_be_rehashed && read_only_serializer.can_be_rehashed();
 

src/heap/finalization-group-cleanup-task.cc

@@ -47,7 +47,7 @@ void FinalizationGroupCleanupTask::RunInternal() {
   Handle<JSFinalizationGroup> finalization_group;
   // There could be no dirty FinalizationGroups. When a context is disposed by
   // the embedder, its FinalizationGroups are removed from the dirty list.
-  if (!heap_->TakeOneDirtyJSFinalizationGroup().ToHandle(&finalization_group)) {
+  if (!heap_->DequeueDirtyJSFinalizationGroup().ToHandle(&finalization_group)) {
     return;
   }
   finalization_group->set_scheduled_for_cleanup(false);

src/heap/heap.cc

@@ -207,6 +207,7 @@ Heap::Heap()
   set_native_contexts_list(Smi::zero());
   set_allocation_sites_list(Smi::zero());
   set_dirty_js_finalization_groups_list(Smi::zero());
+  set_dirty_js_finalization_groups_list_tail(Smi::zero());
   // Put a dummy entry in the remembered pages so we can find the list the
   // minidump even if there are no real unmapped pages.
   RememberUnmappedPage(kNullAddress, false);
@@ -1203,7 +1204,7 @@ void Heap::GarbageCollectionEpilogue() {
       isolate()->host_cleanup_finalization_group_callback()) {
     HandleScope handle_scope(isolate());
     Handle<JSFinalizationGroup> finalization_group;
-    while (TakeOneDirtyJSFinalizationGroup().ToHandle(&finalization_group)) {
+    while (DequeueDirtyJSFinalizationGroup().ToHandle(&finalization_group)) {
       isolate()->RunHostCleanupFinalizationGroupCallback(finalization_group);
     }
   }
@@ -2678,6 +2679,11 @@ void Heap::ProcessDirtyJSFinalizationGroups(WeakObjectRetainer* retainer) {
   Object head = VisitWeakList<JSFinalizationGroup>(
       this, dirty_js_finalization_groups_list(), retainer);
   set_dirty_js_finalization_groups_list(head);
+  // If the list is empty, set the tail to undefined. Otherwise the tail is set
+  // by WeakListVisitor<JSFinalizationGroup>::VisitLiveObject.
+  if (head.IsUndefined(isolate())) {
+    set_dirty_js_finalization_groups_list_tail(head);
+  }
 }
 
 void Heap::ProcessWeakListRoots(WeakObjectRetainer* retainer) {
@@ -2685,6 +2691,8 @@ void Heap::ProcessWeakListRoots(WeakObjectRetainer* retainer) {
   set_allocation_sites_list(retainer->RetainAs(allocation_sites_list()));
   set_dirty_js_finalization_groups_list(
       retainer->RetainAs(dirty_js_finalization_groups_list()));
+  set_dirty_js_finalization_groups_list_tail(
+      retainer->RetainAs(dirty_js_finalization_groups_list_tail()));
 }
 
 void Heap::ForeachAllocationSite(
@@ -6069,33 +6077,47 @@ void Heap::PostFinalizationGroupCleanupTaskIfNeeded() {
   is_finalization_group_cleanup_task_posted_ = true;
 }
 
-void Heap::AddDirtyJSFinalizationGroup(
+void Heap::EnqueueDirtyJSFinalizationGroup(
     JSFinalizationGroup finalization_group,
     std::function<void(HeapObject object, ObjectSlot slot, Object target)>
         gc_notify_updated_slot) {
+  // Add a FinalizationGroup to the tail of the dirty list.
   DCHECK(!HasDirtyJSFinalizationGroups() ||
          dirty_js_finalization_groups_list().IsJSFinalizationGroup());
   DCHECK(finalization_group.next_dirty().IsUndefined(isolate()));
   DCHECK(!finalization_group.scheduled_for_cleanup());
   finalization_group.set_scheduled_for_cleanup(true);
-  finalization_group.set_next_dirty(dirty_js_finalization_groups_list());
-  gc_notify_updated_slot(
-      finalization_group,
-      finalization_group.RawField(JSFinalizationGroup::kNextDirtyOffset),
-      dirty_js_finalization_groups_list());
-  set_dirty_js_finalization_groups_list(finalization_group);
-  // dirty_js_finalization_groups_list is rescanned by ProcessWeakListRoots.
+  if (dirty_js_finalization_groups_list_tail().IsUndefined(isolate())) {
+    DCHECK(dirty_js_finalization_groups_list().IsUndefined(isolate()));
+    set_dirty_js_finalization_groups_list(finalization_group);
+    // dirty_js_finalization_groups_list_ is rescanned by ProcessWeakListRoots.
+  } else {
+    JSFinalizationGroup tail =
+        JSFinalizationGroup::cast(dirty_js_finalization_groups_list_tail());
+    tail.set_next_dirty(finalization_group);
+    gc_notify_updated_slot(
+        tail,
+        finalization_group.RawField(JSFinalizationGroup::kNextDirtyOffset),
+        finalization_group);
+  }
+  set_dirty_js_finalization_groups_list_tail(finalization_group);
+  // dirty_js_finalization_groups_list_tail_ is rescanned by
+  // ProcessWeakListRoots.
 }
 
-MaybeHandle<JSFinalizationGroup> Heap::TakeOneDirtyJSFinalizationGroup() {
+MaybeHandle<JSFinalizationGroup> Heap::DequeueDirtyJSFinalizationGroup() {
+  // Take a FinalizationGroup from the head of the dirty list for fairness.
   if (HasDirtyJSFinalizationGroups()) {
-    Handle<JSFinalizationGroup> finalization_group(
+    Handle<JSFinalizationGroup> head(
         JSFinalizationGroup::cast(dirty_js_finalization_groups_list()),
         isolate());
-    set_dirty_js_finalization_groups_list(finalization_group->next_dirty());
-    finalization_group->set_next_dirty(
-        ReadOnlyRoots(isolate()).undefined_value());
-    return finalization_group;
+    set_dirty_js_finalization_groups_list(head->next_dirty());
+    head->set_next_dirty(ReadOnlyRoots(this).undefined_value());
+    if (*head == dirty_js_finalization_groups_list_tail()) {
+      set_dirty_js_finalization_groups_list_tail(
+          ReadOnlyRoots(this).undefined_value());
+    }
+    return head;
   }
   return {};
 }
@@ -6127,6 +6149,7 @@ void Heap::RemoveDirtyFinalizationGroupsOnContext(NativeContext context) {
       current = finalization_group.next_dirty();
     }
   }
+  set_dirty_js_finalization_groups_list_tail(prev);
 }
 
 void Heap::KeepDuringJob(Handle<JSReceiver> target) {

src/heap/heap.h

@@ -508,6 +508,12 @@ class Heap {
   Object dirty_js_finalization_groups_list() {
     return dirty_js_finalization_groups_list_;
   }
+  void set_dirty_js_finalization_groups_list_tail(Object object) {
+    dirty_js_finalization_groups_list_tail_ = object;
+  }
+  Object dirty_js_finalization_groups_list_tail() {
+    return dirty_js_finalization_groups_list_tail_;
+  }
 
   // Used in CreateAllocationSiteStub and the (de)serializer.
   Address allocation_sites_list_address() {
@@ -807,14 +813,12 @@ class Heap {
   // See also: FLAG_interpreted_frames_native_stack.
   void SetInterpreterEntryTrampolineForProfiling(Code code);
 
-  // Add finalization_group to the end of the dirty_js_finalization_groups list.
-  void AddDirtyJSFinalizationGroup(
+  void EnqueueDirtyJSFinalizationGroup(
       JSFinalizationGroup finalization_group,
       std::function<void(HeapObject object, ObjectSlot slot, Object target)>
           gc_notify_updated_slot);
 
-  // Pop and return the head of the dirty_js_finalization_groups list.
-  MaybeHandle<JSFinalizationGroup> TakeOneDirtyJSFinalizationGroup();
+  MaybeHandle<JSFinalizationGroup> DequeueDirtyJSFinalizationGroup();
 
   // Called from Heap::NotifyContextDisposed to remove all FinalizationGroups
   // with {context} from the dirty list when the context e.g. navigates away or
@@ -2050,6 +2054,8 @@ class Heap {
   Object native_contexts_list_;
   Object allocation_sites_list_;
   Object dirty_js_finalization_groups_list_;
+  // Weak list tails.
+  Object dirty_js_finalization_groups_list_tail_;
 
   std::vector<GCCallbackTuple> gc_epilogue_callbacks_;
   std::vector<GCCallbackTuple> gc_prologue_callbacks_;

src/heap/mark-compact.cc

@@ -2497,8 +2497,8 @@ void MarkCompactCollector::ClearJSWeakRefs() {
       JSFinalizationGroup finalization_group =
           JSFinalizationGroup::cast(weak_cell.finalization_group());
       if (!finalization_group.scheduled_for_cleanup()) {
-        heap()->AddDirtyJSFinalizationGroup(finalization_group,
-                                            gc_notify_updated_slot);
+        heap()->EnqueueDirtyJSFinalizationGroup(finalization_group,
+                                                gc_notify_updated_slot);
       }
       // We're modifying the pointers in WeakCell and JSFinalizationGroup during
       // GC; thus we need to record the slots it writes. The normal write

src/heap/objects-visiting.cc

@@ -197,7 +197,9 @@ struct WeakListVisitor<JSFinalizationGroup> {
 
   static int WeakNextOffset() { return JSFinalizationGroup::kNextDirtyOffset; }
 
-  static void VisitLiveObject(Heap*, JSFinalizationGroup, WeakObjectRetainer*) {
+  static void VisitLiveObject(Heap* heap, JSFinalizationGroup obj,
+                              WeakObjectRetainer*) {
+    heap->set_dirty_js_finalization_groups_list_tail(obj);
   }
 
   static void VisitPhantomObject(Heap*, JSFinalizationGroup) {}

src/heap/setup-heap-internal.cc

@@ -68,6 +68,8 @@ bool Heap::CreateHeapObjects() {
   set_native_contexts_list(ReadOnlyRoots(this).undefined_value());
   set_allocation_sites_list(ReadOnlyRoots(this).undefined_value());
   set_dirty_js_finalization_groups_list(ReadOnlyRoots(this).undefined_value());
+  set_dirty_js_finalization_groups_list_tail(
+      ReadOnlyRoots(this).undefined_value());
 
   return true;
 }

src/snapshot/startup-deserializer.cc

@@ -56,6 +56,8 @@ void StartupDeserializer::DeserializeInto(Isolate* isolate) {
   }
   isolate->heap()->set_dirty_js_finalization_groups_list(
       ReadOnlyRoots(isolate).undefined_value());
+  isolate->heap()->set_dirty_js_finalization_groups_list_tail(
+      ReadOnlyRoots(isolate).undefined_value());
 
   isolate->builtins()->MarkInitialized();
 

src/snapshot/startup-serializer.cc

@@ -169,6 +169,14 @@ void StartupSerializer::SerializeUsingPartialSnapshotCache(
   sink->PutInt(cache_index, "partial_snapshot_cache_index");
 }
 
+void StartupSerializer::CheckNoDirtyFinalizationGroups() {
+  Isolate* isolate = this->isolate();
+  CHECK(isolate->heap()->dirty_js_finalization_groups_list().IsUndefined(
+      isolate));
+  CHECK(isolate->heap()->dirty_js_finalization_groups_list_tail().IsUndefined(
+      isolate));
+}
+
 void SerializedHandleChecker::AddToSet(FixedArray serialized) {
   int length = serialized.length();
   for (int i = 0; i < length; i++) serialized_.insert(serialized.get(i));

src/snapshot/startup-serializer.h

@@ -41,6 +41,10 @@ class V8_EXPORT_PRIVATE StartupSerializer : public RootsSerializer {
   void SerializeUsingPartialSnapshotCache(SnapshotByteSink* sink,
                                           HeapObject obj);
 
+  // The per-heap dirty FinalizationGroup list is weak and not serialized. No
+  // JSFinalizationGroups should be used during startup.
+  void CheckNoDirtyFinalizationGroups();
+
  private:
   void SerializeObject(HeapObject o) override;