Reland "[heap] Refactor atomic marking phase"

This is a reland of commit 25e32252

Original change's description:
> Reland "[heap] Refactor atomic marking phase"
>
> This is a reland of commit a3f66927
>
> The reland addresses a few CHECKs that were too agressive and also
> properly adjusts Oilpan's marking configurations depending on V8's
> flags.
>
> Original change's description:
> > [heap] Refactor atomic marking phase
> >
> > The atomic marking phase was organized in many distinct smaller
> > phases. In particular, before http://crrev.com/c/3584115 the marking
> > phase split into two large separate phases.
> >
> > This CL reorganizes marking into two phases that perform regular V8
> > heap marking, Oilpan, and ephemerons:
> > - A parallel phase that likely drains all marking worklists;
> > - A single-threaded final phase to catch any left overs;
> >
> > This avoids artificial splitting in phases and also avoids repeated
> > starting and joining of jobs.
> >
> > Change-Id: I5cccfc5777837d9ece10d8f4925781bf2d07d9da
> > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3602507
> > Reviewed-by: Omer Katz <omerkatz@chromium.org>
> > Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
> > Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
> > Cr-Commit-Position: refs/heads/main@{#80265}
>
> Change-Id: I26648da361b92d787c173aa9d390100ce8958728
> Bug: chromium:1320896
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3616519
> Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
> Reviewed-by: Omer Katz <omerkatz@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#80301}

Bug: chromium:1320896
Change-Id: I7ebb3bde9f0d3497f46c728bfbc380c1bd4bc021
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3641167Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#80485}

src/heap/base/worklist.h

@@ -196,6 +196,8 @@ void Worklist<EntryType, SegmentSize>::Merge(
 template <typename EntryType, uint16_t SegmentSize>
 void Worklist<EntryType, SegmentSize>::Swap(
     Worklist<EntryType, SegmentSize>* other) {
+  v8::base::MutexGuard guard1(&lock_);
+  v8::base::MutexGuard guard2(&other->lock_);
   Segment* top = top_;
   set_top(other->top_);
   other->set_top(top);

src/heap/cppgc-js/cpp-heap.cc

@@ -428,6 +428,19 @@ v8::metrics::Recorder::ContextId CppHeap::MetricRecorderAdapter::GetContextId()
       GetIsolate()->native_context());
 }
 
+namespace {
+
+CppHeap::MarkingType GetSupportedMarkingType() {
+  // Keep the selection simple for now as production configurations do not turn
+  // off parallel and/or concurrent marking independently.
+  if (!FLAG_parallel_marking || !FLAG_concurrent_marking)
+    return CppHeap::MarkingType::kIncremental;
+
+  return CppHeap::MarkingType::kIncrementalAndConcurrent;
+}
+
+}  // namespace
+
 CppHeap::CppHeap(
     v8::Platform* platform,
     const std::vector<std::unique_ptr<cppgc::CustomSpaceBase>>& custom_spaces,
@@ -436,8 +449,7 @@ CppHeap::CppHeap(
           std::make_shared<CppgcPlatformAdapter>(platform), custom_spaces,
           cppgc::internal::HeapBase::StackSupport::
               kSupportsConservativeStackScan,
-          FLAG_single_threaded_gc ? MarkingType::kIncremental
-                                  : MarkingType::kIncrementalAndConcurrent,
+          GetSupportedMarkingType(),
           FLAG_single_threaded_gc ? SweepingType::kIncremental
                                   : SweepingType::kIncrementalAndConcurrent),
       wrapper_descriptor_(wrapper_descriptor) {
@@ -605,6 +617,10 @@ bool CppHeap::AdvanceTracing(double max_duration) {
                        : v8::base::TimeDelta::FromMillisecondsD(max_duration);
   const size_t marked_bytes_limit = in_atomic_pause_ ? SIZE_MAX : 0;
   DCHECK_NOT_NULL(marker_);
+  if (in_atomic_pause_) {
+    marker_->NotifyConcurrentMarkingOfWorkIfNeeded(
+        cppgc::TaskPriority::kUserBlocking);
+  }
   // TODO(chromium:1056170): Replace when unified heap transitions to
   // bytes-based deadline.
   marking_done_ =

src/heap/cppgc/concurrent-marker.cc

@@ -238,6 +238,13 @@ void ConcurrentMarkerBase::NotifyIncrementalMutatorStepCompleted() {
   }
 }
 
+void ConcurrentMarkerBase::NotifyOfWorkIfNeeded(cppgc::TaskPriority priority) {
+  if (HasWorkForConcurrentMarking(marking_worklists_)) {
+    concurrent_marking_handle_->UpdatePriority(priority);
+    concurrent_marking_handle_->NotifyConcurrencyIncrease();
+  }
+}
+
 void ConcurrentMarkerBase::IncreaseMarkingPriorityIfNeeded() {
   if (!concurrent_marking_handle_->UpdatePriorityEnabled()) return;
   if (concurrent_marking_priority_increased_) return;

src/heap/cppgc/concurrent-marker.h

@@ -30,6 +30,7 @@ class V8_EXPORT_PRIVATE ConcurrentMarkerBase {
   bool Cancel();
 
   void NotifyIncrementalMutatorStepCompleted();
+  void NotifyOfWorkIfNeeded(cppgc::TaskPriority priority);
 
   bool IsActive() const;
 

src/heap/cppgc/marker.cc

@@ -469,6 +469,13 @@ bool MarkerBase::JoinConcurrentMarkingIfNeeded() {
   return true;
 }
 
+void MarkerBase::NotifyConcurrentMarkingOfWorkIfNeeded(
+    cppgc::TaskPriority priority) {
+  if (concurrent_marker_->IsActive()) {
+    concurrent_marker_->NotifyOfWorkIfNeeded(priority);
+  }
+}
+
 bool MarkerBase::AdvanceMarkingWithLimits(v8::base::TimeDelta max_duration,
                                           size_t marked_bytes_limit) {
   bool is_done = false;

src/heap/cppgc/marker.h

@@ -8,6 +8,7 @@
 #include <memory>
 
 #include "include/cppgc/heap.h"
+#include "include/cppgc/platform.h"
 #include "include/cppgc/visitor.h"
 #include "src/base/macros.h"
 #include "src/base/platform/time.h"
@@ -112,6 +113,7 @@ class V8_EXPORT_PRIVATE MarkerBase {
   void ProcessWeakness();
 
   bool JoinConcurrentMarkingIfNeeded();
+  void NotifyConcurrentMarkingOfWorkIfNeeded(cppgc::TaskPriority);
 
   inline void WriteBarrierForInConstructionObject(HeapObjectHeader&);
 

src/heap/gc-tracer.cc

@@ -907,11 +907,10 @@ void GCTracer::PrintNVP() const {
           "mark=%.1f "
           "mark.finish_incremental=%.1f "
           "mark.roots=%.1f "
-          "mark.main=%.1f "
-          "mark.weak_closure=%.1f "
-          "mark.weak_closure.ephemeron=%.1f "
-          "mark.weak_closure.ephemeron.marking=%.1f "
-          "mark.weak_closure.ephemeron.linear=%.1f "
+          "mark.full_closure_parallel=%.1f "
+          "mark.full_closure=%.1f "
+          "mark.ephemeron.marking=%.1f "
+          "mark.ephemeron.linear=%.1f "
           "mark.embedder_prologue=%.1f "
           "mark.embedder_tracing=%.1f "
           "prologue=%.1f "
@@ -996,9 +995,8 @@ void GCTracer::PrintNVP() const {
           current_scope(Scope::MC_MARK),
           current_scope(Scope::MC_MARK_FINISH_INCREMENTAL),
           current_scope(Scope::MC_MARK_ROOTS),
-          current_scope(Scope::MC_MARK_MAIN),
-          current_scope(Scope::MC_MARK_WEAK_CLOSURE),
-          current_scope(Scope::MC_MARK_WEAK_CLOSURE_EPHEMERON),
+          current_scope(Scope::MC_MARK_FULL_CLOSURE_PARALLEL),
+          current_scope(Scope::MC_MARK_FULL_CLOSURE),
           current_scope(Scope::MC_MARK_WEAK_CLOSURE_EPHEMERON_MARKING),
           current_scope(Scope::MC_MARK_WEAK_CLOSURE_EPHEMERON_LINEAR),
           current_scope(Scope::MC_MARK_EMBEDDER_PROLOGUE),

src/heap/mark-compact.cc

@@ -2104,7 +2104,7 @@ void MarkCompactCollector::RevisitObject(HeapObject obj) {
   marking_visitor_->Visit(obj.map(marking_visitor_->cage_base()), obj);
 }
 
-bool MarkCompactCollector::ProcessEphemeronsUntilFixpoint() {
+bool MarkCompactCollector::MarkTransitiveClosureUntilFixpoint() {
   int iterations = 0;
   int max_iterations = FLAG_ephemeron_fixpoint_iterations;
 
@@ -2128,20 +2128,13 @@ bool MarkCompactCollector::ProcessEphemeronsUntilFixpoint() {
     {
       TRACE_GC(heap()->tracer(),
                GCTracer::Scope::MC_MARK_WEAK_CLOSURE_EPHEMERON_MARKING);
-
-      if (FLAG_parallel_marking) {
-        heap_->concurrent_marking()->RescheduleJobIfNeeded(
-            TaskPriority::kUserBlocking);
-      }
-
       another_ephemeron_iteration_main_thread = ProcessEphemerons();
-      FinishConcurrentMarking();
     }
 
-    CHECK(
-        local_weak_objects()->current_ephemerons_local.IsLocalAndGlobalEmpty());
-    CHECK(local_weak_objects()
-              ->discovered_ephemerons_local.IsLocalAndGlobalEmpty());
+    // Can only check for local emptiness here as parallel marking tasks may
+    // still be running. The caller performs the CHECKs for global emptiness.
+    CHECK(local_weak_objects()->current_ephemerons_local.IsLocalEmpty());
+    CHECK(local_weak_objects()->discovered_ephemerons_local.IsLocalEmpty());
 
     ++iterations;
   } while (another_ephemeron_iteration_main_thread ||
@@ -2150,10 +2143,6 @@ bool MarkCompactCollector::ProcessEphemeronsUntilFixpoint() {
            !local_marking_worklists()->IsWrapperEmpty() ||
            !heap()->local_embedder_heap_tracer()->IsRemoteTracingDone());
 
-  CHECK(local_marking_worklists()->IsEmpty());
-  CHECK(local_weak_objects()->current_ephemerons_local.IsLocalAndGlobalEmpty());
-  CHECK(local_weak_objects()
-            ->discovered_ephemerons_local.IsLocalAndGlobalEmpty());
   return true;
 }
 
@@ -2195,10 +2184,9 @@ bool MarkCompactCollector::ProcessEphemerons() {
   return another_ephemeron_iteration;
 }
 
-void MarkCompactCollector::ProcessEphemeronsLinear() {
+void MarkCompactCollector::MarkTransitiveClosureLinear() {
   TRACE_GC(heap()->tracer(),
            GCTracer::Scope::MC_MARK_WEAK_CLOSURE_EPHEMERON_LINEAR);
-  CHECK(heap()->concurrent_marking()->IsStopped());
   std::unordered_multimap<HeapObject, HeapObject, Object::Hasher> key_to_values;
   Ephemeron ephemeron;
 
@@ -2319,6 +2307,9 @@ std::pair<size_t, size_t> MarkCompactCollector::ProcessMarkingWorklist(
   PtrComprCageBase cage_base(isolate);
   CodePageHeaderModificationScope rwx_write_scope(
       "Marking of Code objects require write access to Code page headers");
+  if (parallel_marking_)
+    heap_->concurrent_marking()->RescheduleJobIfNeeded(
+        TaskPriority::kUserBlocking);
   while (local_marking_worklists()->Pop(&object) ||
          local_marking_worklists()->PopOnHold(&object)) {
     // Left trimming may result in grey or black filler objects on the marking
@@ -2385,19 +2376,7 @@ bool MarkCompactCollector::ProcessEphemeron(HeapObject key, HeapObject value) {
   return false;
 }
 
-void MarkCompactCollector::ProcessEphemeronMarking() {
-  DCHECK(local_marking_worklists()->IsEmpty());
-
-  // Incremental marking might leave ephemerons in main task's local
-  // buffer, flush it into global pool.
-  local_weak_objects()->next_ephemerons_local.Publish();
-
-  if (!ProcessEphemeronsUntilFixpoint()) {
-    // Fixpoint iteration needed too many iterations and was cancelled. Use the
-    // guaranteed linear algorithm.
-    ProcessEphemeronsLinear();
-  }
-
+void MarkCompactCollector::VerifyEphemeronMarking() {
 #ifdef VERIFY_HEAP
   if (FLAG_verify_heap) {
     Ephemeron ephemeron;
@@ -2409,10 +2388,19 @@ void MarkCompactCollector::ProcessEphemeronMarking() {
       CHECK(!ProcessEphemeron(ephemeron.key, ephemeron.value));
     }
   }
-#endif
+#endif  // VERIFY_HEAP
+}
 
-  CHECK(local_marking_worklists()->IsEmpty());
-  CHECK(heap()->local_embedder_heap_tracer()->IsRemoteTracingDone());
+void MarkCompactCollector::MarkTransitiveClosure() {
+  // Incremental marking might leave ephemerons in main task's local
+  // buffer, flush it into global pool.
+  local_weak_objects()->next_ephemerons_local.Publish();
+
+  if (!MarkTransitiveClosureUntilFixpoint()) {
+    // Fixpoint iteration needed too many iterations and was cancelled. Use the
+    // guaranteed linear algorithm.
+    MarkTransitiveClosureLinear();
+  }
 }
 
 void MarkCompactCollector::ProcessTopOptimizedFrame(ObjectVisitor* visitor,
@@ -2493,50 +2481,33 @@ void MarkCompactCollector::MarkLiveObjects() {
     MarkObjectsFromClientHeaps();
   }
 
-  {
-    TRACE_GC(heap()->tracer(), GCTracer::Scope::MC_MARK_MAIN);
-    if (FLAG_parallel_marking) {
-      heap_->concurrent_marking()->RescheduleJobIfNeeded(
-          TaskPriority::kUserBlocking);
-    }
-    DrainMarkingWorklist();
-
-    FinishConcurrentMarking();
-    DrainMarkingWorklist();
-  }
-
-  {
-    TRACE_GC(heap()->tracer(), GCTracer::Scope::MC_MARK_WEAK_CLOSURE);
-
-    DCHECK(local_marking_worklists()->IsEmpty());
-
-    // Mark objects reachable through the embedder heap. This phase is
-    // opportunistic as it may not discover graphs that are only reachable
-    // through ephemerons.
+  if (FLAG_parallel_marking) {
+    TRACE_GC(heap()->tracer(), GCTracer::Scope::MC_MARK_FULL_CLOSURE_PARALLEL);
+    parallel_marking_ = true;
+    heap_->concurrent_marking()->RescheduleJobIfNeeded(
+        TaskPriority::kUserBlocking);
+    MarkTransitiveClosure();
     {
       TRACE_GC(heap()->tracer(),
-               GCTracer::Scope::MC_MARK_EMBEDDER_TRACING_CLOSURE);
-      do {
-        // PerformWrapperTracing() also empties the work items collected by
-        // concurrent markers. As a result this call needs to happen at least
-        // once.
-        PerformWrapperTracing();
-        DrainMarkingWorklist();
-      } while (!heap_->local_embedder_heap_tracer()->IsRemoteTracingDone() ||
-               !local_marking_worklists()->IsWrapperEmpty());
-      DCHECK(local_marking_worklists()->IsWrapperEmpty());
-      DCHECK(local_marking_worklists()->IsEmpty());
+               GCTracer::Scope::MC_MARK_FULL_CLOSURE_PARALLEL_JOIN);
+      FinishConcurrentMarking();
     }
+    parallel_marking_ = false;
+  }
 
-    // The objects reachable from the roots are marked, yet unreachable objects
-    // are unmarked. Mark objects reachable due to embedder heap tracing or
-    // harmony weak maps.
-    {
-      TRACE_GC(heap()->tracer(),
-               GCTracer::Scope::MC_MARK_WEAK_CLOSURE_EPHEMERON);
-      ProcessEphemeronMarking();
-      DCHECK(local_marking_worklists()->IsEmpty());
-    }
+  {
+    TRACE_GC(heap()->tracer(), GCTracer::Scope::MC_MARK_FULL_CLOSURE);
+    // Complete the transitive closure single-threaded to avoid races with
+    // multiple threads when processing weak maps and embedder heaps.
+    CHECK(heap()->concurrent_marking()->IsStopped());
+    MarkTransitiveClosure();
+    CHECK(local_marking_worklists()->IsEmpty());
+    CHECK(
+        local_weak_objects()->current_ephemerons_local.IsLocalAndGlobalEmpty());
+    CHECK(local_weak_objects()
+              ->discovered_ephemerons_local.IsLocalAndGlobalEmpty());
+    CHECK(heap()->local_embedder_heap_tracer()->IsRemoteTracingDone());
+    VerifyEphemeronMarking();
   }
 
   if (was_marked_incrementally) {

src/heap/mark-compact.h

@@ -614,7 +614,8 @@ class MarkCompactCollector final {
   void UpdatePointersInClientHeap(Isolate* client);
 
   // Marks object reachable from harmony weak maps and wrapper tracing.
-  void ProcessEphemeronMarking();
+  void MarkTransitiveClosure();
+  void VerifyEphemeronMarking();
 
   // If the call-site of the top optimized code was not prepared for
   // deoptimization, then treat embedded pointers in the code as strong as
@@ -629,19 +630,21 @@ class MarkCompactCollector final {
   // Returns true if value was actually marked.
   bool ProcessEphemeron(HeapObject key, HeapObject value);
 
-  // Marks ephemerons and drains marking worklist iteratively
-  // until a fixpoint is reached. Returns false if too many iterations have been
-  // tried and the linear approach should be used.
-  bool ProcessEphemeronsUntilFixpoint();
+  // Marks the transitive closure by draining the marking worklist iteratively,
+  // applying ephemerons semantics and invoking embedder tracing until a
+  // fixpoint is reached. Returns false if too many iterations have been tried
+  // and the linear approach should be used.
+  bool MarkTransitiveClosureUntilFixpoint();
+
+  // Marks the transitive closure applying ephemeron semantics and invoking
+  // embedder tracing with a linear algorithm for ephemerons. Only used if
+  // fixpoint iteration doesn't finish within a few iterations.
+  void MarkTransitiveClosureLinear();
 
   // Drains ephemeron and marking worklists. Single iteration of the
   // fixpoint iteration.
   bool ProcessEphemerons();
 
-  // Mark ephemerons and drain marking worklist with a linear algorithm.
-  // Only used if fixpoint iteration doesn't finish within a few iterations.
-  void ProcessEphemeronsLinear();
-
   // Perform Wrapper Tracing if in use.
   void PerformWrapperTracing();
 
@@ -747,6 +750,7 @@ class MarkCompactCollector final {
   bool compacting_ = false;
   bool black_allocation_ = false;
   bool have_code_to_deoptimize_ = false;
+  bool parallel_marking_ = false;
 
   MarkingWorklists marking_worklists_;
 

src/heap/weak-object-worklists.h

@@ -42,7 +42,7 @@ class TransitionArray;
   /* 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       \
+     MarkCompactCollector::MarkTransitiveClosureUntilFixpoint drains/fills   \
      these worklists. current_ephemerons is used as draining worklist in     \
      the current fixpoint iteration. */                                      \
   F(Ephemeron, current_ephemerons, CurrentEphemerons)                        \

src/init/heap-symbols.h

@@ -568,12 +568,11 @@
   F(MC_MARK_CLIENT_HEAPS)                            \
   F(MC_MARK_EMBEDDER_PROLOGUE)                       \
   F(MC_MARK_EMBEDDER_TRACING)                        \
-  F(MC_MARK_EMBEDDER_TRACING_CLOSURE)                \
   F(MC_MARK_FINISH_INCREMENTAL)                      \
-  F(MC_MARK_MAIN)                                    \
+  F(MC_MARK_FULL_CLOSURE_PARALLEL)                   \
+  F(MC_MARK_FULL_CLOSURE_PARALLEL_JOIN)              \
   F(MC_MARK_ROOTS)                                   \
-  F(MC_MARK_WEAK_CLOSURE)                            \
-  F(MC_MARK_WEAK_CLOSURE_EPHEMERON)                  \
+  F(MC_MARK_FULL_CLOSURE)                            \
   F(MC_MARK_WEAK_CLOSURE_EPHEMERON_MARKING)          \
   F(MC_MARK_WEAK_CLOSURE_EPHEMERON_LINEAR)           \
   F(MC_SWEEP_CODE)                                   \