Use the incremental root marking visitor for overapproximating the weak closure

Since incremental marking hasn't completed yet at this point, we can't
use the MC's root visitor.

Also, we shouldn't hurry with incremental marking while
overapproximating

BUG=v8:3862
R=hpayer@chromium.org
LOG=n

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

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

src/heap/heap.cc

@@ -776,9 +776,7 @@ void Heap::OverApproximateWeakClosure(const char* gc_reason) {
       CallGCPrologueCallbacks(kGCTypeMarkSweepCompact, kNoGCCallbackFlags);
     }
   }
-  mark_compact_collector()->OverApproximateWeakClosure();
-  incremental_marking()->set_should_hurry(false);
-  incremental_marking()->set_weak_closure_was_overapproximated(true);
+  incremental_marking()->MarkObjectGroups();
   {
     GCCallbacksScope scope(this);
     if (scope.CheckReenter()) {

src/heap/incremental-marking.cc

@@ -251,7 +251,7 @@ class IncrementalMarkingMarkingVisitor
 
   // Marks the object grey and pushes it on the marking stack.
   INLINE(static void MarkObject(Heap* heap, Object* obj)) {
-    IncrementalMarking::MarkObject(heap, obj);
+    IncrementalMarking::MarkObject(heap, HeapObject::cast(obj));
   }
 
   // Marks the object black without pushing it on the marking stack.
@@ -287,7 +287,7 @@ class IncrementalMarkingRootMarkingVisitor : public ObjectVisitor {
     Object* obj = *p;
     if (!obj->IsHeapObject()) return;
 
-    IncrementalMarking::MarkObject(heap_, obj);
+    IncrementalMarking::MarkObject(heap_, HeapObject::cast(obj));
   }
 
   Heap* heap_;
@@ -545,6 +545,26 @@ void IncrementalMarking::StartMarking(CompactionFlag flag) {
 }
 
 
+void IncrementalMarking::MarkObjectGroups() {
+  DCHECK(FLAG_overapproximate_weak_closure);
+  DCHECK(!weak_closure_was_overapproximated_);
+
+  GCTracer::Scope gc_scope(heap_->tracer(),
+                           GCTracer::Scope::MC_INCREMENTAL_WEAKCLOSURE);
+
+  heap_->mark_compact_collector()->MarkImplicitRefGroups(&MarkObject);
+
+  IncrementalMarkingRootMarkingVisitor visitor(this);
+  heap_->isolate()->global_handles()->IterateObjectGroups(
+      &visitor, &MarkCompactCollector::IsUnmarkedHeapObjectWithHeap);
+
+  heap_->isolate()->global_handles()->RemoveImplicitRefGroups();
+  heap_->isolate()->global_handles()->RemoveObjectGroups();
+
+  weak_closure_was_overapproximated_ = true;
+}
+
+
 void IncrementalMarking::PrepareForScavenge() {
   if (!IsMarking()) return;
   NewSpacePageIterator it(heap_->new_space()->FromSpaceStart(),
@@ -625,13 +645,12 @@ void IncrementalMarking::VisitObject(Map* map, HeapObject* obj, int size) {
 }
 
 
-void IncrementalMarking::MarkObject(Heap* heap, Object* obj) {
-  HeapObject* heap_object = HeapObject::cast(obj);
-  MarkBit mark_bit = Marking::MarkBitFrom(heap_object);
+void IncrementalMarking::MarkObject(Heap* heap, HeapObject* obj) {
+  MarkBit mark_bit = Marking::MarkBitFrom(obj);
   if (mark_bit.data_only()) {
-    MarkBlackOrKeepGrey(heap_object, mark_bit, heap_object->Size());
+    MarkBlackOrKeepGrey(obj, mark_bit, obj->Size());
   } else if (Marking::IsWhite(mark_bit)) {
-    heap->incremental_marking()->WhiteToGreyAndPush(heap_object, mark_bit);
+    heap->incremental_marking()->WhiteToGreyAndPush(obj, mark_bit);
   }
 }
 
@@ -774,7 +793,6 @@ void IncrementalMarking::OverApproximateWeakClosure() {
   if (FLAG_trace_incremental_marking) {
     PrintF("[IncrementalMarking] requesting weak closure overapproximation.\n");
   }
-  set_should_hurry(true);
   request_type_ = OVERAPPROXIMATION;
   heap_->isolate()->stack_guard()->RequestGC();
 }

src/heap/incremental-marking.h

@@ -67,6 +67,8 @@ class IncrementalMarking {
 
   void Stop();
 
+  void MarkObjectGroups();
+
   void PrepareForScavenge();
 
   void UpdateMarkingDequeAfterScavenge();
@@ -189,7 +191,7 @@ class IncrementalMarking {
 
   bool IsIdleMarkingDelayCounterLimitReached();
 
-  INLINE(static void MarkObject(Heap* heap, Object* object));
+  INLINE(static void MarkObject(Heap* heap, HeapObject* object));
 
   Heap* heap() const { return heap_; }
 

src/heap/mark-compact.cc

@@ -1985,7 +1985,8 @@ void MarkCompactCollector::MarkRoots(RootMarkingVisitor* visitor) {
 }
 
 
-void MarkCompactCollector::MarkImplicitRefGroups() {
+void MarkCompactCollector::MarkImplicitRefGroups(
+    MarkObjectFunction mark_object) {
   List<ImplicitRefGroup*>* ref_groups =
       isolate()->global_handles()->implicit_ref_groups();
 
@@ -2003,9 +2004,7 @@ void MarkCompactCollector::MarkImplicitRefGroups() {
     // A parent object is marked, so mark all child heap objects.
     for (size_t j = 0; j < entry->length; ++j) {
       if ((*children[j])->IsHeapObject()) {
-        HeapObject* child = HeapObject::cast(*children[j]);
-        MarkBit mark = Marking::MarkBitFrom(child);
-        MarkObject(child, mark);
+        mark_object(heap(), HeapObject::cast(*children[j]));
       }
     }
 
@@ -2104,7 +2103,7 @@ void MarkCompactCollector::ProcessEphemeralMarking(
     if (!only_process_harmony_weak_collections) {
       isolate()->global_handles()->IterateObjectGroups(
           visitor, &IsUnmarkedHeapObjectWithHeap);
-      MarkImplicitRefGroups();
+      MarkImplicitRefGroups(&MarkCompactMarkingVisitor::MarkObject);
     }
     ProcessWeakCollections();
     work_to_do = !marking_deque_.IsEmpty();
@@ -2224,21 +2223,6 @@ void MarkCompactCollector::UncommitMarkingDeque() {
 }
 
 
-void MarkCompactCollector::OverApproximateWeakClosure() {
-  GCTracer::Scope gc_scope(heap()->tracer(),
-                           GCTracer::Scope::MC_INCREMENTAL_WEAKCLOSURE);
-
-  RootMarkingVisitor root_visitor(heap());
-  isolate()->global_handles()->IterateObjectGroups(
-      &root_visitor, &IsUnmarkedHeapObjectWithHeap);
-  MarkImplicitRefGroups();
-
-  // Remove object groups after marking phase.
-  heap()->isolate()->global_handles()->RemoveObjectGroups();
-  heap()->isolate()->global_handles()->RemoveImplicitRefGroups();
-}
-
-
 void MarkCompactCollector::MarkLiveObjects() {
   GCTracer::Scope gc_scope(heap()->tracer(), GCTracer::Scope::MC_MARK);
   double start_time = 0.0;

src/heap/mark-compact.h

@@ -16,6 +16,9 @@ namespace internal {
 // to the first live object in the page (only used for old and map objects).
 typedef bool (*IsAliveFunction)(HeapObject* obj, int* size, int* offset);
 
+// Callback function to mark an object in a given heap.
+typedef void (*MarkObjectFunction)(Heap* heap, HeapObject* object);
+
 // Forward declarations.
 class CodeFlusher;
 class MarkCompactCollector;
@@ -550,6 +553,7 @@ class MarkCompactCollector {
   static const uint32_t kMultiFreeEncoding = 1;
 
   static inline bool IsMarked(Object* obj);
+  static bool IsUnmarkedHeapObjectWithHeap(Heap* heap, Object** p);
 
   inline Heap* heap() const { return heap_; }
   inline Isolate* isolate() const;
@@ -663,6 +667,10 @@ class MarkCompactCollector {
   // to artificially keep AllocationSites alive for a time.
   void MarkAllocationSite(AllocationSite* site);
 
+  // Mark objects in implicit references groups if their parent object
+  // is marked.
+  void MarkImplicitRefGroups(MarkObjectFunction mark_object);
+
   MarkingDeque* marking_deque() { return &marking_deque_; }
 
   void EnsureMarkingDequeIsCommittedAndInitialize();
@@ -671,8 +679,6 @@ class MarkCompactCollector {
 
   void UncommitMarkingDeque();
 
-  void OverApproximateWeakClosure();
-
   // The following four methods can just be called after marking, when the
   // whole transitive closure is known. They must be called before sweeping
   // when mark bits are still intact.
@@ -776,10 +782,6 @@ class MarkCompactCollector {
   // the string table are weak.
   void MarkStringTable(RootMarkingVisitor* visitor);
 
-  // Mark objects in implicit references groups if their parent object
-  // is marked.
-  void MarkImplicitRefGroups();
-
   // Mark objects reachable (transitively) from objects in the marking stack
   // or overflowed in the heap.
   void ProcessMarkingDeque();
@@ -816,7 +818,6 @@ class MarkCompactCollector {
   // Callback function for telling whether the object *p is an unmarked
   // heap object.
   static bool IsUnmarkedHeapObject(Object** p);
-  static bool IsUnmarkedHeapObjectWithHeap(Heap* heap, Object** p);
 
   // Map transitions from a live map to a dead map must be killed.
   // We replace them with a null descriptor, with the same key.