Reland "[heap] Refactor marking weak object worklists"

This is a reland of ff61743fb0

Original change's description:
> [heap] Refactor marking weak object worklists
>
> This CL extracts weak object worklist related code into separate files
> and uses a macro to specify all weak object worklists in a generic way.
>
> The motivation of the refactoring is twofold:
> 1) We can now enforce that each weak object worklist is updated after
>    Scavenge. (Forgetting to define the update function causes a link
>    time error.)
> 2) The reduced boilerplate will be useful for transitioning to the
>    new ::heap::base::Worklist.
>
> Change-Id: Ic80a7ccca010c09370d6525f43d78de24192f8ea
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2442624
> Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
> Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#70308}

Change-Id: I8a9f39e53ef4123dd28a1da6f7992cdff341f694
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2461741
Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70568}

BUILD.gn

@@ -2705,6 +2705,8 @@ v8_source_set("v8_base_without_compiler") {
     "src/heap/stress-scavenge-observer.h",
     "src/heap/sweeper.cc",
     "src/heap/sweeper.h",
+    "src/heap/weak-object-worklists.cc",
+    "src/heap/weak-object-worklists.h",
     "src/heap/worklist.h",
     "src/ic/call-optimization.cc",
     "src/ic/call-optimization.h",

src/heap/concurrent-marking.h

@@ -29,7 +29,7 @@ class Heap;
 class Isolate;
 class MajorNonAtomicMarkingState;
 class MemoryChunk;
-struct WeakObjects;
+class WeakObjects;
 
 struct MemoryChunkData {
   intptr_t live_bytes;

src/heap/incremental-marking.cc

@@ -501,109 +501,7 @@ void IncrementalMarking::UpdateMarkingWorklistAfterScavenge() {
         }
       });
 
-  UpdateWeakReferencesAfterScavenge();
-}
-
-void IncrementalMarking::UpdateWeakReferencesAfterScavenge() {
-  weak_objects_->weak_references.Update(
-      [](std::pair<HeapObject, HeapObjectSlot> slot_in,
-         std::pair<HeapObject, HeapObjectSlot>* slot_out) -> bool {
-        HeapObject heap_obj = slot_in.first;
-        HeapObject forwarded = ForwardingAddress(heap_obj);
-
-        if (!forwarded.is_null()) {
-          ptrdiff_t distance_to_slot =
-              slot_in.second.address() - slot_in.first.ptr();
-          Address new_slot = forwarded.ptr() + distance_to_slot;
-          slot_out->first = forwarded;
-          slot_out->second = HeapObjectSlot(new_slot);
-          return true;
-        }
-
-        return false;
-      });
-  weak_objects_->weak_objects_in_code.Update(
-      [](std::pair<HeapObject, Code> slot_in,
-         std::pair<HeapObject, Code>* slot_out) -> bool {
-        HeapObject heap_obj = slot_in.first;
-        HeapObject forwarded = ForwardingAddress(heap_obj);
-
-        if (!forwarded.is_null()) {
-          slot_out->first = forwarded;
-          slot_out->second = slot_in.second;
-          return true;
-        }
-
-        return false;
-      });
-  weak_objects_->ephemeron_hash_tables.Update(
-      [](EphemeronHashTable slot_in, EphemeronHashTable* slot_out) -> bool {
-        EphemeronHashTable forwarded = ForwardingAddress(slot_in);
-
-        if (!forwarded.is_null()) {
-          *slot_out = forwarded;
-          return true;
-        }
-
-        return false;
-      });
-
-  auto ephemeron_updater = [](Ephemeron slot_in, Ephemeron* slot_out) -> bool {
-    HeapObject key = slot_in.key;
-    HeapObject value = slot_in.value;
-    HeapObject forwarded_key = ForwardingAddress(key);
-    HeapObject forwarded_value = ForwardingAddress(value);
-
-    if (!forwarded_key.is_null() && !forwarded_value.is_null()) {
-      *slot_out = Ephemeron{forwarded_key, forwarded_value};
-      return true;
-    }
-
-    return false;
-  };
-
-  weak_objects_->current_ephemerons.Update(ephemeron_updater);
-  weak_objects_->next_ephemerons.Update(ephemeron_updater);
-  weak_objects_->discovered_ephemerons.Update(ephemeron_updater);
-
-  weak_objects_->flushed_js_functions.Update(
-      [](JSFunction slot_in, JSFunction* slot_out) -> bool {
-        JSFunction forwarded = ForwardingAddress(slot_in);
-
-        if (!forwarded.is_null()) {
-          *slot_out = forwarded;
-          return true;
-        }
-
-        return false;
-      });
-#ifdef DEBUG
-  weak_objects_->bytecode_flushing_candidates.Iterate(
-      [](SharedFunctionInfo candidate) {
-        DCHECK(!Heap::InYoungGeneration(candidate));
-      });
-#endif
-
-  if (FLAG_harmony_weak_refs) {
-    weak_objects_->js_weak_refs.Update(
-        [](JSWeakRef js_weak_ref_in, JSWeakRef* js_weak_ref_out) -> bool {
-          JSWeakRef forwarded = ForwardingAddress(js_weak_ref_in);
-
-          if (!forwarded.is_null()) {
-            *js_weak_ref_out = forwarded;
-            return true;
-          }
-
-          return false;
-        });
-
-#ifdef DEBUG
-    // TODO(syg, marja): Support WeakCells in the young generation.
-    weak_objects_->weak_cells.Iterate([](WeakCell weak_cell) {
-      DCHECK(!Heap::InYoungGeneration(weak_cell));
-    });
-#endif
-  }
+  weak_objects_->UpdateAfterScavenge();
 }
 
 void IncrementalMarking::UpdateMarkedBytesAfterScavenge(

src/heap/incremental-marking.h

@@ -146,7 +146,6 @@ class V8_EXPORT_PRIVATE IncrementalMarking final {
   void FinalizeIncrementally();
 
   void UpdateMarkingWorklistAfterScavenge();
-  void UpdateWeakReferencesAfterScavenge();
   void UpdateMarkedBytesAfterScavenge(size_t dead_bytes_in_new_space);
 
   void Hurry();

src/heap/marking-visitor.h

@@ -11,58 +11,12 @@
 #include "src/heap/memory-chunk.h"
 #include "src/heap/objects-visiting.h"
 #include "src/heap/spaces.h"
+#include "src/heap/weak-object-worklists.h"
 #include "src/heap/worklist.h"
-#include "src/objects/heap-object.h"   // For Worklist<HeapObject, ...>
-#include "src/objects/js-weak-refs.h"  // For Worklist<WeakCell, ...>
 
 namespace v8 {
 namespace internal {
 
-struct Ephemeron {
-  HeapObject key;
-  HeapObject value;
-};
-
-using EphemeronWorklist = Worklist<Ephemeron, 64>;
-
-// Weak objects encountered during marking.
-struct WeakObjects {
-  Worklist<TransitionArray, 64> transition_arrays;
-
-  // Keep track of all EphemeronHashTables in the heap to process
-  // them in the atomic pause.
-  Worklist<EphemeronHashTable, 64> ephemeron_hash_tables;
-
-  // Keep track of all ephemerons for concurrent marking tasks. Only store
-  // ephemerons in these Worklists if both key and value are unreachable at the
-  // moment.
-  //
-  // MarkCompactCollector::ProcessEphemeronsUntilFixpoint drains and fills these
-  // worklists.
-  //
-  // current_ephemerons is used as draining worklist in the current fixpoint
-  // iteration.
-  EphemeronWorklist current_ephemerons;
-
-  // Stores ephemerons to visit in the next fixpoint iteration.
-  EphemeronWorklist next_ephemerons;
-
-  // When draining the marking worklist new discovered ephemerons are pushed
-  // into this worklist.
-  EphemeronWorklist discovered_ephemerons;
-
-  // TODO(marja): For old space, we only need the slot, not the host
-  // object. Optimize this by adding a different storage for old space.
-  Worklist<std::pair<HeapObject, HeapObjectSlot>, 64> weak_references;
-  Worklist<std::pair<HeapObject, Code>, 64> weak_objects_in_code;
-
-  Worklist<JSWeakRef, 64> js_weak_refs;
-  Worklist<WeakCell, 64> weak_cells;
-
-  Worklist<SharedFunctionInfo, 64> bytecode_flushing_candidates;
-  Worklist<JSFunction, 64> flushed_js_functions;
-};
-
 struct EphemeronMarking {
   std::vector<HeapObject> newly_discovered;
   bool newly_discovered_overflowed;

src/heap/weak-object-worklists.cc

@@ -0,0 +1,172 @@
+// Copyright 2020 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "src/heap/weak-object-worklists.h"
+
+#include "src/heap/heap-inl.h"
+#include "src/heap/heap.h"
+#include "src/heap/worklist.h"
+#include "src/objects/hash-table.h"
+#include "src/objects/heap-object.h"
+#include "src/objects/js-function.h"
+#include "src/objects/js-weak-refs-inl.h"
+#include "src/objects/js-weak-refs.h"
+#include "src/objects/shared-function-info.h"
+#include "src/objects/transitions.h"
+
+namespace v8 {
+
+namespace internal {
+
+void WeakObjects::UpdateAfterScavenge() {
+#define INVOKE_UPDATE(_, name, Name) Update##Name(name);
+  WEAK_OBJECT_WORKLISTS(INVOKE_UPDATE)
+#undef INVOKE_UPDATE
+}
+
+void WeakObjects::UpdateTransitionArrays(
+    WeakObjectWorklist<TransitionArray>& transition_arrays) {
+  DCHECK(!ContainsYoungObjects(transition_arrays));
+}
+
+void WeakObjects::UpdateEphemeronHashTables(
+    WeakObjectWorklist<EphemeronHashTable>& ephemeron_hash_tables) {
+  ephemeron_hash_tables.Update(
+      [](EphemeronHashTable slot_in, EphemeronHashTable* slot_out) -> bool {
+        EphemeronHashTable forwarded = ForwardingAddress(slot_in);
+
+        if (!forwarded.is_null()) {
+          *slot_out = forwarded;
+          return true;
+        }
+
+        return false;
+      });
+}
+
+namespace {
+bool EphemeronUpdater(Ephemeron slot_in, Ephemeron* slot_out) {
+  HeapObject key = slot_in.key;
+  HeapObject value = slot_in.value;
+  HeapObject forwarded_key = ForwardingAddress(key);
+  HeapObject forwarded_value = ForwardingAddress(value);
+
+  if (!forwarded_key.is_null() && !forwarded_value.is_null()) {
+    *slot_out = Ephemeron{forwarded_key, forwarded_value};
+    return true;
+  }
+
+  return false;
+}
+}  // anonymous namespace
+
+void WeakObjects::UpdateCurrentEphemerons(
+    WeakObjectWorklist<Ephemeron>& current_ephemerons) {
+  current_ephemerons.Update(EphemeronUpdater);
+}
+
+void WeakObjects::UpdateNextEphemerons(
+    WeakObjectWorklist<Ephemeron>& next_ephemerons) {
+  next_ephemerons.Update(EphemeronUpdater);
+}
+
+void WeakObjects::UpdateDiscoveredEphemerons(
+    WeakObjectWorklist<Ephemeron>& discovered_ephemerons) {
+  discovered_ephemerons.Update(EphemeronUpdater);
+}
+
+void WeakObjects::UpdateWeakReferences(
+    WeakObjectWorklist<HeapObjectAndSlot>& weak_references) {
+  weak_references.Update(
+      [](HeapObjectAndSlot slot_in, HeapObjectAndSlot* slot_out) -> bool {
+        HeapObject heap_obj = slot_in.first;
+        HeapObject forwarded = ForwardingAddress(heap_obj);
+
+        if (!forwarded.is_null()) {
+          ptrdiff_t distance_to_slot =
+              slot_in.second.address() - slot_in.first.ptr();
+          Address new_slot = forwarded.ptr() + distance_to_slot;
+          slot_out->first = forwarded;
+          slot_out->second = HeapObjectSlot(new_slot);
+          return true;
+        }
+
+        return false;
+      });
+}
+
+void WeakObjects::UpdateWeakObjectsInCode(
+    WeakObjectWorklist<HeapObjectAndCode>& weak_objects_in_code) {
+  weak_objects_in_code.Update(
+      [](HeapObjectAndCode slot_in, HeapObjectAndCode* slot_out) -> bool {
+        HeapObject heap_obj = slot_in.first;
+        HeapObject forwarded = ForwardingAddress(heap_obj);
+
+        if (!forwarded.is_null()) {
+          slot_out->first = forwarded;
+          slot_out->second = slot_in.second;
+          return true;
+        }
+
+        return false;
+      });
+}
+
+void WeakObjects::UpdateJSWeakRefs(
+    WeakObjectWorklist<JSWeakRef>& js_weak_refs) {
+  if (FLAG_harmony_weak_refs) {
+    js_weak_refs.Update(
+        [](JSWeakRef js_weak_ref_in, JSWeakRef* js_weak_ref_out) -> bool {
+          JSWeakRef forwarded = ForwardingAddress(js_weak_ref_in);
+
+          if (!forwarded.is_null()) {
+            *js_weak_ref_out = forwarded;
+            return true;
+          }
+
+          return false;
+        });
+  }
+}
+
+void WeakObjects::UpdateWeakCells(WeakObjectWorklist<WeakCell>& weak_cells) {
+  // TODO(syg, marja): Support WeakCells in the young generation.
+  DCHECK(!ContainsYoungObjects(weak_cells));
+}
+
+void WeakObjects::UpdateBytecodeFlushingCandidates(
+    WeakObjectWorklist<SharedFunctionInfo>& bytecode_flushing_candidates) {
+  DCHECK(!ContainsYoungObjects(bytecode_flushing_candidates));
+}
+
+void WeakObjects::UpdateFlushedJSFunctions(
+    WeakObjectWorklist<JSFunction>& flushed_js_functions) {
+  flushed_js_functions.Update(
+      [](JSFunction slot_in, JSFunction* slot_out) -> bool {
+        JSFunction forwarded = ForwardingAddress(slot_in);
+
+        if (!forwarded.is_null()) {
+          *slot_out = forwarded;
+          return true;
+        }
+
+        return false;
+      });
+}
+
+#ifdef DEBUG
+template <typename Type>
+bool WeakObjects::ContainsYoungObjects(WeakObjectWorklist<Type>& worklist) {
+  bool result = false;
+  worklist.Iterate([&result](Type candidate) {
+    if (Heap::InYoungGeneration(candidate)) {
+      result = true;
+    }
+  });
+  return result;
+}
+#endif
+
+}  // namespace internal
+}  // namespace v8

src/heap/weak-object-worklists.h

@@ -0,0 +1,90 @@
+// Copyright 2020 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef V8_HEAP_WEAK_OBJECT_WORKLISTS_H_
+#define V8_HEAP_WEAK_OBJECT_WORKLISTS_H_
+
+#include "src/common/globals.h"
+#include "src/heap/worklist.h"
+#include "src/objects/heap-object.h"
+#include "src/objects/js-weak-refs.h"
+
+namespace v8 {
+namespace internal {
+
+struct Ephemeron {
+  HeapObject key;
+  HeapObject value;
+};
+
+using HeapObjectAndSlot = std::pair<HeapObject, HeapObjectSlot>;
+using HeapObjectAndCode = std::pair<HeapObject, Code>;
+class EphemeronHashTable;
+class JSFunction;
+class SharedFunctionInfo;
+class TransitionArray;
+
+// Weak objects and weak references discovered during incremental/concurrent
+// marking. They are processed in ClearNonLiveReferences after marking.
+// Each entry in this list specifies:
+// 1) Type of the worklist entry.
+// 2) Lower-case name of the worklsit.
+// 3) Capitalized name of the worklist.
+//
+// If you add a new entry, then you also need to implement the corresponding
+// Update*() function in the cc file for updating pointers after Scavenge.
+#define WEAK_OBJECT_WORKLISTS(F)                                             \
+  F(TransitionArray, transition_arrays, TransitionArrays)                    \
+  /* Keep track of all EphemeronHashTables in the heap to process            \
+     them in the atomic pause. */                                            \
+  F(EphemeronHashTable, ephemeron_hash_tables, EphemeronHashTables)          \
+  /* Keep track of all ephemerons for concurrent marking tasks. Only store   \
+     ephemerons in these worklists if both (key, value) are unreachable at   \
+     the moment.                                                             \
+     MarkCompactCollector::ProcessEphemeronsUntilFixpoint drains/fills       \
+     these worklists. current_ephemerons is used as draining worklist in     \
+     the current fixpoint iteration. */                                      \
+  F(Ephemeron, current_ephemerons, CurrentEphemerons)                        \
+  /* Stores ephemerons to visit in the next fixpoint iteration. */           \
+  F(Ephemeron, next_ephemerons, NextEphemerons)                              \
+  /* When draining the marking worklist new discovered ephemerons are pushed \
+      into this worklist. */                                                 \
+  F(Ephemeron, discovered_ephemerons, DiscoveredEphemerons)                  \
+  /* TODO(marja): For old space, we only need the slot, not the host object. \
+     Optimize this by adding a different storage for old space. */           \
+  F(HeapObjectAndSlot, weak_references, WeakReferences)                      \
+  F(HeapObjectAndCode, weak_objects_in_code, WeakObjectsInCode)              \
+  F(JSWeakRef, js_weak_refs, JSWeakRefs)                                     \
+  F(WeakCell, weak_cells, WeakCells)                                         \
+  F(SharedFunctionInfo, bytecode_flushing_candidates,                        \
+    BytecodeFlushingCandidates)                                              \
+  F(JSFunction, flushed_js_functions, FlushedJSFunctions)
+
+class WeakObjects {
+ public:
+  template <typename Type>
+  using WeakObjectWorklist = Worklist<Type, 64>;
+
+#define DECLARE_WORKLIST(Type, name, _) WeakObjectWorklist<Type> name;
+  WEAK_OBJECT_WORKLISTS(DECLARE_WORKLIST)
+#undef DECLARE_WORKLIST
+
+  void UpdateAfterScavenge();
+
+ private:
+#define DECLARE_UPDATE_METHODS(Type, _, Name) \
+  void Update##Name(WeakObjectWorklist<Type>&);
+  WEAK_OBJECT_WORKLISTS(DECLARE_UPDATE_METHODS)
+#undef DECLARE_UPDATE_METHODS
+
+#ifdef DEBUG
+  template <typename Type>
+  bool ContainsYoungObjects(WeakObjectWorklist<Type>& worklist);
+#endif
+};
+
+}  // namespace internal
+}  // namespace v8
+
+#endif  // V8_HEAP_WEAK_OBJECT_WORKLISTS_H_