[heap] Convert overapproximate weak closure phase into finalize incremental...

[heap] Convert overapproximate weak closure phase into finalize incremental marking phase and revisit the root set there.

BUG=chromium:548562
LOG=n

Review URL: https://codereview.chromium.org/1428683002

Cr-Commit-Position: refs/heads/master@{#31627}

src/flag-definitions.h

@@ -658,13 +658,13 @@ DEFINE_BOOL(age_code, true,
             "track un-executed functions to age code and flush only "
             "old code (required for code flushing)")
 DEFINE_BOOL(incremental_marking, true, "use incremental marking")
-DEFINE_BOOL(overapproximate_weak_closure, true,
-            "overapproximate weak closure to reduce atomic pause time")
-DEFINE_INT(min_progress_during_object_groups_marking, 128,
-           "keep overapproximating the weak closure as long as we discover at "
+DEFINE_BOOL(finalize_marking_incrementally, true,
+            "finalize marking in incremental steps")
+DEFINE_INT(min_progress_during_incremental_marking_finalization, 128,
+           "keep finalizing incremental marking as long as we discover at "
            "least this many unmarked objects")
-DEFINE_INT(max_object_groups_marking_rounds, 3,
-           "at most try this many times to over approximate the weak closure")
+DEFINE_INT(max_incremental_marking_finalization_rounds, 3,
+           "at most try this many times to finalize incremental marking")
 DEFINE_BOOL(concurrent_sweeping, true, "use concurrent sweeping")
 DEFINE_BOOL(parallel_compaction, false, "use parallel compaction")
 DEFINE_BOOL(trace_incremental_marking, false,

src/heap/gc-tracer.cc

@@ -501,7 +501,6 @@ void GCTracer::PrintNVP() const {
                    "compaction_ptrs=%.1f "
                    "intracompaction_ptrs=%.1f "
                    "misc_compaction=%.1f "
-                   "weak_closure=%.1f "
                    "inc_weak_closure=%.1f "
                    "weakcollection_process=%.1f "
                    "weakcollection_clear=%.1f "
@@ -567,8 +566,7 @@ void GCTracer::PrintNVP() const {
                    current_.scopes[Scope::MC_UPDATE_POINTERS_TO_EVACUATED],
                    current_.scopes[Scope::MC_UPDATE_POINTERS_BETWEEN_EVACUATED],
                    current_.scopes[Scope::MC_UPDATE_MISC_POINTERS],
-                   current_.scopes[Scope::MC_WEAKCLOSURE],
-                   current_.scopes[Scope::MC_INCREMENTAL_WEAKCLOSURE],
+                   current_.scopes[Scope::MC_INCREMENTAL_FINALIZE],
                    current_.scopes[Scope::MC_WEAKCOLLECTION_PROCESS],
                    current_.scopes[Scope::MC_WEAKCOLLECTION_CLEAR],
                    current_.scopes[Scope::MC_WEAKCOLLECTION_ABORT],

src/heap/gc-tracer.h

@@ -125,8 +125,7 @@ class GCTracer {
       MC_UPDATE_POINTERS_TO_EVACUATED,
       MC_UPDATE_POINTERS_BETWEEN_EVACUATED,
       MC_UPDATE_MISC_POINTERS,
-      MC_INCREMENTAL_WEAKCLOSURE,
-      MC_WEAKCLOSURE,
+      MC_INCREMENTAL_FINALIZE,
       MC_WEAKCOLLECTION_PROCESS,
       MC_WEAKCOLLECTION_CLEAR,
       MC_WEAKCOLLECTION_ABORT,

src/heap/heap.cc

@@ -774,9 +774,9 @@ void Heap::HandleGCRequest() {
                       current_gc_callback_flags_);
     return;
   }
-  DCHECK(FLAG_overapproximate_weak_closure);
-  if (!incremental_marking()->weak_closure_was_overapproximated()) {
-    OverApproximateWeakClosure("GC interrupt");
+  DCHECK(FLAG_finalize_marking_incrementally);
+  if (!incremental_marking()->finalize_marking_completed()) {
+    FinalizeIncrementalMarking("GC interrupt");
   }
 }
 
@@ -786,14 +786,13 @@ void Heap::ScheduleIdleScavengeIfNeeded(int bytes_allocated) {
 }
 
 
-void Heap::OverApproximateWeakClosure(const char* gc_reason) {
+void Heap::FinalizeIncrementalMarking(const char* gc_reason) {
   if (FLAG_trace_incremental_marking) {
     PrintF("[IncrementalMarking] Overapproximate weak closure (%s).\n",
            gc_reason);
   }
 
-  GCTracer::Scope gc_scope(tracer(),
-                           GCTracer::Scope::MC_INCREMENTAL_WEAKCLOSURE);
+  GCTracer::Scope gc_scope(tracer(), GCTracer::Scope::MC_INCREMENTAL_FINALIZE);
 
   {
     GCCallbacksScope scope(this);
@@ -805,7 +804,7 @@ void Heap::OverApproximateWeakClosure(const char* gc_reason) {
       CallGCPrologueCallbacks(kGCTypeIncrementalMarking, kNoGCCallbackFlags);
     }
   }
-  incremental_marking()->MarkObjectGroups();
+  incremental_marking()->FinalizeIncrementally();
   {
     GCCallbacksScope scope(this);
     if (scope.CheckReenter()) {
@@ -4071,11 +4070,12 @@ void Heap::ReduceNewSpaceSize() {
 
 
 void Heap::FinalizeIncrementalMarkingIfComplete(const char* comment) {
-  if (FLAG_overapproximate_weak_closure && incremental_marking()->IsMarking() &&
+  if (FLAG_finalize_marking_incrementally &&
+      incremental_marking()->IsMarking() &&
       (incremental_marking()->IsReadyToOverApproximateWeakClosure() ||
-       (!incremental_marking()->weak_closure_was_overapproximated() &&
+       (!incremental_marking()->finalize_marking_completed() &&
         mark_compact_collector()->marking_deque()->IsEmpty()))) {
-    OverApproximateWeakClosure(comment);
+    FinalizeIncrementalMarking(comment);
   } else if (incremental_marking()->IsComplete() ||
              (mark_compact_collector()->marking_deque()->IsEmpty())) {
     CollectAllGarbage(current_gc_flags_, comment);
@@ -4088,13 +4088,13 @@ bool Heap::TryFinalizeIdleIncrementalMarking(double idle_time_in_ms) {
   size_t final_incremental_mark_compact_speed_in_bytes_per_ms =
       static_cast<size_t>(
           tracer()->FinalIncrementalMarkCompactSpeedInBytesPerMillisecond());
-  if (FLAG_overapproximate_weak_closure &&
+  if (FLAG_finalize_marking_incrementally &&
       (incremental_marking()->IsReadyToOverApproximateWeakClosure() ||
-       (!incremental_marking()->weak_closure_was_overapproximated() &&
+       (!incremental_marking()->finalize_marking_completed() &&
         mark_compact_collector()->marking_deque()->IsEmpty() &&
         gc_idle_time_handler_->ShouldDoOverApproximateWeakClosure(
             static_cast<size_t>(idle_time_in_ms))))) {
-    OverApproximateWeakClosure(
+    FinalizeIncrementalMarking(
         "Idle notification: overapproximate weak closure");
     return true;
   } else if (incremental_marking()->IsComplete() ||

src/heap/heap.h

@@ -1809,7 +1809,7 @@ class Heap {
   // implicit references from global handles, but don't atomically complete
   // marking. If we continue to mark incrementally, we might have marked
   // objects that die later.
-  void OverApproximateWeakClosure(const char* gc_reason);
+  void FinalizeIncrementalMarking(const char* gc_reason);
 
   // Returns the timer used for a given GC type.
   // - GCScavenger: young generation GC

src/heap/incremental-marking.cc

@@ -42,8 +42,8 @@ IncrementalMarking::IncrementalMarking(Heap* heap)
       no_marking_scope_depth_(0),
       unscanned_bytes_of_large_object_(0),
       was_activated_(false),
-      weak_closure_was_overapproximated_(false),
-      weak_closure_approximation_rounds_(0),
+      finalize_marking_completed_(false),
+      incremental_marking_finalization_rounds_(0),
       request_type_(COMPLETE_MARKING) {}
 
 
@@ -623,33 +623,57 @@ void IncrementalMarking::StartMarking() {
 }
 
 
-void IncrementalMarking::MarkObjectGroups() {
-  DCHECK(FLAG_overapproximate_weak_closure);
-  DCHECK(!weak_closure_was_overapproximated_);
+void IncrementalMarking::MarkRoots() {
+  DCHECK(FLAG_finalize_marking_incrementally);
+  DCHECK(!finalize_marking_completed_);
   DCHECK(IsMarking());
 
-  int old_marking_deque_top =
-      heap_->mark_compact_collector()->marking_deque()->top();
+  IncrementalMarkingRootMarkingVisitor visitor(this);
+  heap_->IterateStrongRoots(&visitor, VISIT_ONLY_STRONG);
+}
 
-  heap_->mark_compact_collector()->MarkImplicitRefGroups(&MarkObject);
+
+void IncrementalMarking::MarkObjectGroups() {
+  DCHECK(FLAG_finalize_marking_incrementally);
+  DCHECK(!finalize_marking_completed_);
+  DCHECK(IsMarking());
 
   IncrementalMarkingRootMarkingVisitor visitor(this);
+  heap_->mark_compact_collector()->MarkImplicitRefGroups(&MarkObject);
   heap_->isolate()->global_handles()->IterateObjectGroups(
       &visitor, &MarkCompactCollector::IsUnmarkedHeapObjectWithHeap);
+  heap_->isolate()->global_handles()->RemoveImplicitRefGroups();
+  heap_->isolate()->global_handles()->RemoveObjectGroups();
+}
+
+
+void IncrementalMarking::FinalizeIncrementally() {
+  DCHECK(FLAG_finalize_marking_incrementally);
+  DCHECK(!finalize_marking_completed_);
+  DCHECK(IsMarking());
+
+  int old_marking_deque_top =
+      heap_->mark_compact_collector()->marking_deque()->top();
+
+  // After finishing incremental marking, we try to discover all unmarked
+  // objects to reduce the marking load in the final pause.
+  // 1) We scan and mark the roots again to find all changes to the root set.
+  // 2) We mark the object groups.
+  MarkRoots();
+  MarkObjectGroups();
 
   int marking_progress =
       abs(old_marking_deque_top -
-          heap_->mark_compact_collector()->marking_deque()->top());
+          heap_->mark_compact_collector()->marking_deque()->top()) /
+      kPointerSize;
 
-  ++weak_closure_approximation_rounds_;
-  if ((weak_closure_approximation_rounds_ >=
-       FLAG_max_object_groups_marking_rounds) ||
-      (marking_progress < FLAG_min_progress_during_object_groups_marking)) {
-    weak_closure_was_overapproximated_ = true;
+  ++incremental_marking_finalization_rounds_;
+  if ((incremental_marking_finalization_rounds_ >=
+       FLAG_max_incremental_marking_finalization_rounds) ||
+      (marking_progress <
+       FLAG_min_progress_during_incremental_marking_finalization)) {
+    finalize_marking_completed_ = true;
   }
-
-  heap_->isolate()->global_handles()->RemoveImplicitRefGroups();
-  heap_->isolate()->global_handles()->RemoveObjectGroups();
 }
 
 
@@ -860,13 +884,15 @@ void IncrementalMarking::Finalize() {
 }
 
 
-void IncrementalMarking::OverApproximateWeakClosure(CompletionAction action) {
-  DCHECK(FLAG_overapproximate_weak_closure);
-  DCHECK(!weak_closure_was_overapproximated_);
+void IncrementalMarking::FinalizeMarking(CompletionAction action) {
+  DCHECK(FLAG_finalize_marking_incrementally);
+  DCHECK(!finalize_marking_completed_);
   if (FLAG_trace_incremental_marking) {
-    PrintF("[IncrementalMarking] requesting weak closure overapproximation.\n");
+    PrintF(
+        "[IncrementalMarking] requesting finalization of incremental "
+        "marking.\n");
   }
-  request_type_ = OVERAPPROXIMATION;
+  request_type_ = FINALIZATION;
   if (action == GC_VIA_STACK_GUARD) {
     heap_->isolate()->stack_guard()->RequestGC();
   }
@@ -893,8 +919,8 @@ void IncrementalMarking::MarkingComplete(CompletionAction action) {
 
 void IncrementalMarking::Epilogue() {
   was_activated_ = false;
-  weak_closure_was_overapproximated_ = false;
-  weak_closure_approximation_rounds_ = 0;
+  finalize_marking_completed_ = false;
+  incremental_marking_finalization_rounds_ = 0;
 }
 
 
@@ -1072,9 +1098,9 @@ intptr_t IncrementalMarking::Step(intptr_t allocated_bytes,
       if (heap_->mark_compact_collector()->marking_deque()->IsEmpty()) {
         if (completion == FORCE_COMPLETION ||
             IsIdleMarkingDelayCounterLimitReached()) {
-          if (FLAG_overapproximate_weak_closure &&
-              !weak_closure_was_overapproximated_) {
-            OverApproximateWeakClosure(action);
+          if (FLAG_finalize_marking_incrementally &&
+              !finalize_marking_completed_) {
+            FinalizeMarking(action);
           } else {
             MarkingComplete(action);
           }

src/heap/incremental-marking.h

@@ -27,7 +27,7 @@ class IncrementalMarking {
 
   enum ForceCompletionAction { FORCE_COMPLETION, DO_NOT_FORCE_COMPLETION };
 
-  enum GCRequestType { COMPLETE_MARKING, OVERAPPROXIMATION };
+  enum GCRequestType { COMPLETE_MARKING, FINALIZATION };
 
   struct StepActions {
     StepActions(CompletionAction complete_action_,
@@ -56,12 +56,12 @@ class IncrementalMarking {
   bool should_hurry() { return should_hurry_; }
   void set_should_hurry(bool val) { should_hurry_ = val; }
 
-  bool weak_closure_was_overapproximated() const {
-    return weak_closure_was_overapproximated_;
+  bool finalize_marking_completed() const {
+    return finalize_marking_completed_;
   }
 
   void SetWeakClosureWasOverApproximatedForTesting(bool val) {
-    weak_closure_was_overapproximated_ = val;
+    finalize_marking_completed_ = val;
   }
 
   inline bool IsStopped() { return state() == STOPPED; }
@@ -73,8 +73,7 @@ class IncrementalMarking {
   inline bool IsComplete() { return state() == COMPLETE; }
 
   inline bool IsReadyToOverApproximateWeakClosure() const {
-    return request_type_ == OVERAPPROXIMATION &&
-           !weak_closure_was_overapproximated_;
+    return request_type_ == FINALIZATION && !finalize_marking_completed_;
   }
 
   GCRequestType request_type() const { return request_type_; }
@@ -87,7 +86,7 @@ class IncrementalMarking {
 
   void Start(const char* reason = nullptr);
 
-  void MarkObjectGroups();
+  void FinalizeIncrementally();
 
   void UpdateMarkingDequeAfterScavenge();
 
@@ -97,7 +96,7 @@ class IncrementalMarking {
 
   void Stop();
 
-  void OverApproximateWeakClosure(CompletionAction action);
+  void FinalizeMarking(CompletionAction action);
 
   void MarkingComplete(CompletionAction action);
 
@@ -223,6 +222,9 @@ class IncrementalMarking {
 
   void StartMarking();
 
+  void MarkRoots();
+  void MarkObjectGroups();
+
   void ActivateIncrementalWriteBarrier(PagedSpace* space);
   static void ActivateIncrementalWriteBarrier(NewSpace* space);
   void ActivateIncrementalWriteBarrier();
@@ -266,9 +268,9 @@ class IncrementalMarking {
 
   bool was_activated_;
 
-  bool weak_closure_was_overapproximated_;
+  bool finalize_marking_completed_;
 
-  int weak_closure_approximation_rounds_;
+  int incremental_marking_finalization_rounds_;
 
   GCRequestType request_type_;
 

test/cctest/cctest.h

@@ -641,7 +641,7 @@ static inline void SimulateIncrementalMarking(i::Heap* heap,
   while (!marking->IsComplete()) {
     marking->Step(i::MB, i::IncrementalMarking::NO_GC_VIA_STACK_GUARD);
     if (marking->IsReadyToOverApproximateWeakClosure()) {
-      marking->MarkObjectGroups();
+      marking->FinalizeIncrementally();
     }
   }
   CHECK(marking->IsComplete());