[heap] Visit not fully constructed objects before epilogue

We can avoid draining the full marking worklist again by visiting them
before initially draining the worklist. This also preserves the
invariant the the epilogue should not find any new objects.

Bug: chromium:843903
Change-Id: I178d7b564ad9aaf5a086183dad83adfcb5c7d5a1
Reviewed-on: https://chromium-review.googlesource.com/1158412Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579786}

third_party/blink/renderer/platform/heap/heap.cc

@@ -211,12 +211,7 @@ void ThreadHeap::RegisterMovingObjectCallback(MovableReference reference,
                                              callback_data);
 }
 
-void ThreadHeap::ProcessMarkingStack(Visitor* visitor) {
-  bool complete = AdvanceMarkingStackProcessing(visitor, TimeTicks::Max());
-  CHECK(complete);
-}
-
-void ThreadHeap::MarkNotFullyConstructedObjects(Visitor* visitor) {
+void ThreadHeap::MarkNotFullyConstructedObjects(MarkingVisitor* visitor) {
   DCHECK(!thread_state_->IsIncrementalMarking());
   ThreadHeapStatsCollector::Scope stats_scope(
       stats_collector(),
@@ -226,8 +221,7 @@ void ThreadHeap::MarkNotFullyConstructedObjects(Visitor* visitor) {
   while (
       not_fully_constructed_worklist_->Pop(WorklistTaskId::MainThread, &item)) {
     BasePage* const page = PageFromObject(item);
-    reinterpret_cast<MarkingVisitor*>(visitor)->ConservativelyMarkAddress(
-        page, reinterpret_cast<Address>(item));
+    visitor->ConservativelyMarkAddress(page, reinterpret_cast<Address>(item));
   }
 }
 
@@ -255,8 +249,7 @@ void ThreadHeap::InvokeEphemeronCallbacks(Visitor* visitor) {
   ephemeron_callbacks_ = std::move(final_set);
 }
 
-bool ThreadHeap::AdvanceMarkingStackProcessing(Visitor* visitor,
-                                               TimeTicks deadline) {
+bool ThreadHeap::AdvanceMarking(MarkingVisitor* visitor, TimeTicks deadline) {
   const size_t kDeadlineCheckInterval = 2500;
   size_t processed_callback_count = 0;
   // Ephemeron fixed point loop.

third_party/blink/renderer/platform/heap/heap.h

@@ -287,10 +287,12 @@ class PLATFORM_EXPORT ThreadHeap {
   template <typename T>
   static Address Reallocate(void* previous, size_t);
 
-  void ProcessMarkingStack(Visitor*);
   void WeakProcessing(Visitor*);
-  void MarkNotFullyConstructedObjects(Visitor*);
-  bool AdvanceMarkingStackProcessing(Visitor*, TimeTicks deadline);
+
+  // Marks not fully constructed objects.
+  void MarkNotFullyConstructedObjects(MarkingVisitor*);
+  // Marks the transitive closure including ephemerons.
+  bool AdvanceMarking(MarkingVisitor*, TimeTicks deadline);
   void VerifyMarking();
 
   // Conservatively checks whether an address is a pointer in any of the

third_party/blink/renderer/platform/heap/thread_state.cc

@@ -1588,6 +1588,7 @@ void ThreadState::RunAtomicPause(BlinkGC::StackState stack_state,
           GcReasonString(reason));
       AtomicPausePrologue(stack_state, marking_type, reason);
       MarkPhaseVisitRoots();
+      MarkPhaseVisitNotFullyConstructedObjects();
       CHECK(MarkPhaseAdvanceMarking(TimeTicks::Max()));
       MarkPhaseEpilogue(marking_type);
     }
@@ -1685,9 +1686,9 @@ void ThreadState::MarkPhaseVisitRoots() {
 
 bool ThreadState::MarkPhaseAdvanceMarking(TimeTicks deadline) {
   StackFrameDepthScope stack_depth_scope(&Heap().GetStackFrameDepth());
-  // 3. Transitive closure to trace objects including ephemerons.
-  return Heap().AdvanceMarkingStackProcessing(current_gc_data_.visitor.get(),
-                                              deadline);
+  return Heap().AdvanceMarking(
+      reinterpret_cast<MarkingVisitor*>(current_gc_data_.visitor.get()),
+      deadline);
 }
 
 bool ThreadState::ShouldVerifyMarking() const {
@@ -1699,12 +1700,13 @@ bool ThreadState::ShouldVerifyMarking() const {
   return should_verify_marking;
 }
 
+void ThreadState::MarkPhaseVisitNotFullyConstructedObjects() {
+  Heap().MarkNotFullyConstructedObjects(
+      reinterpret_cast<MarkingVisitor*>(current_gc_data_.visitor.get()));
+}
+
 void ThreadState::MarkPhaseEpilogue(BlinkGC::MarkingType marking_type) {
   Visitor* visitor = current_gc_data_.visitor.get();
-  // Finish marking of not-fully-constructed objects.
-  Heap().MarkNotFullyConstructedObjects(visitor);
-  CHECK(Heap().AdvanceMarkingStackProcessing(visitor, TimeTicks::Max()));
-
   {
     // See ProcessHeap::CrossThreadPersistentMutex().
     MutexLocker persistent_lock(ProcessHeap::CrossThreadPersistentMutex());

third_party/blink/renderer/platform/heap/thread_state.h

@@ -607,6 +607,7 @@ class PLATFORM_EXPORT ThreadState final
   void AtomicPauseEpilogue(BlinkGC::MarkingType, BlinkGC::SweepingType);
   void MarkPhaseEpilogue(BlinkGC::MarkingType);
   void MarkPhaseVisitRoots();
+  void MarkPhaseVisitNotFullyConstructedObjects();
   bool MarkPhaseAdvanceMarking(TimeTicks deadline);
   void VerifyMarking(BlinkGC::MarkingType);