Revert "[heap] Refactor marking weak object worklists"

This reverts commit ff61743f.

Reason for revert: speculative revert for crbug.com/1135472

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}

TBR=ulan@chromium.org,dinfuehr@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Change-Id: I552423106b516bcc79d067cda390c188a717b125
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2452711Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70351}

BUILD.gn

@@ -2705,8 +2705,6 @@ 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;
-class WeakObjects;
+struct WeakObjects;
 
 struct MemoryChunkData {
   intptr_t live_bytes;

src/heap/incremental-marking.cc

@@ -501,7 +501,109 @@ void IncrementalMarking::UpdateMarkingWorklistAfterScavenge() {
         }
       });
 
-  weak_objects_->UpdateAfterScavenge();
+  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
+  }
 }
 
 void IncrementalMarking::UpdateMarkedBytesAfterScavenge(

src/heap/incremental-marking.h

@@ -146,6 +146,7 @@ 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,12 +11,58 @@
 #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

-// 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

-// 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_