Refactor PointerUpdatingVisitor.

src/heap/mark-compact.cc

 // Copyright 2012 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/mark-compact.h"
 
 #include "src/base/atomicops.h"
 #include "src/base/bits.h"
 #include "src/base/sys-info.h"
 #include "src/code-stubs.h"
@@ skipping 2781 matching lines @@
         slot_type = CODE_ENTRY_SLOT;
       } else {
         DCHECK(RelocInfo::IsEmbeddedObject(rmode));
         slot_type = OBJECT_SLOT;
       }
     }
     RememberedSet<OLD_TO_OLD>::InsertTyped(source_page, slot_type, addr);
   }
 }
 
-static inline void UpdateTypedSlot(Isolate* isolate, ObjectVisitor* v,
-                                   SlotType slot_type, Address addr) {
+static inline SlotCallbackResult UpdateSlot(Heap* heap, Object** slot) {

[ulan, 2016/05/19 08:31:08]

I just moved this function and changed its type from void to SlotCallbackResult.

+  Object* obj = reinterpret_cast<Object*>(
+      base::NoBarrier_Load(reinterpret_cast<base::AtomicWord*>(slot)));
+
+  if (obj->IsHeapObject()) {
+    HeapObject* heap_obj = HeapObject::cast(obj);
+    MapWord map_word = heap_obj->map_word();
+    if (map_word.IsForwardingAddress()) {
+      DCHECK(heap->InFromSpace(heap_obj) ||

[ahaas, 2016/05/19 08:48:44]

you could get the heap from heap_obj->GetHeap()

[ulan, 2016/05/19 09:01:35]

Done :)

+             MarkCompactCollector::IsOnEvacuationCandidate(heap_obj) ||
+             Page::FromAddress(heap_obj->address())
+                 ->IsFlagSet(Page::COMPACTION_WAS_ABORTED));
+      HeapObject* target = map_word.ToForwardingAddress();
+      base::NoBarrier_CompareAndSwap(
+          reinterpret_cast<base::AtomicWord*>(slot),
+          reinterpret_cast<base::AtomicWord>(obj),
+          reinterpret_cast<base::AtomicWord>(target));
+      DCHECK(!heap->InFromSpace(target) &&
+             !MarkCompactCollector::IsOnEvacuationCandidate(target));
+    }
+  }
+  return REMOVE_SLOT;
+}
+
+// Updates a cell slot using an untyped slot callback.
+// The callback accepts (Heap*, Object**) and returns SlotCallbackResult.
+template <typename Callback>
+SlotCallbackResult UpdateCell(Heap* heap, RelocInfo* rinfo, Callback callback) {
+  DCHECK(rinfo->rmode() == RelocInfo::CELL);
+  Object* cell = rinfo->target_cell();
+  Object* old_cell = cell;
+  SlotCallbackResult result = callback(heap, &cell);
+  if (cell != old_cell) {
+    rinfo->set_target_cell(reinterpret_cast<Cell*>(cell));
+  }
+  return result;
+}
+
+// Updates a code entry slot using an untyped slot callback.
+// The callback accepts (Heap*, Object**) and returns SlotCallbackResult.
+template <typename Callback>
+SlotCallbackResult UpdateCodeEntry(Heap* heap, Address entry_address,
+                                   Callback callback) {
+  Object* code = Code::GetObjectFromEntryAddress(entry_address);

[ahaas, 2016/05/19 08:48:44]

Why do you not do the DCHECK here?

[ulan, 2016/05/19 09:01:35]

There is not reloc info.

+  Object* old_code = code;
+  SlotCallbackResult result = callback(heap, &code);
+  if (code != old_code) {
+    Memory::Address_at(entry_address) = reinterpret_cast<Code*>(code)->entry();
+  }
+  return result;
+}
+
+// Updates a code target slot using an untyped slot callback.
+// The callback accepts (Heap*, Object**) and returns SlotCallbackResult.
+template <typename Callback>
+SlotCallbackResult UpdateCodeTarget(Heap* heap, RelocInfo* rinfo,
+                                    Callback callback) {
+  DCHECK(RelocInfo::IsCodeTarget(rinfo->rmode()));
+  Object* target = Code::GetCodeFromTargetAddress(rinfo->target_address());
+  Object* old_target = target;
+  SlotCallbackResult result = callback(heap, &target);
+  if (target != old_target) {
+    rinfo->set_target_address(Code::cast(target)->instruction_start());
+  }
+  return result;
+}
+
+// Updates an embedded pointer slot using an untyped slot callback.
+// The callback accepts (Heap*, Object**) and returns SlotCallbackResult.
+template <typename Callback>
+SlotCallbackResult UpdateEmbeddedPointer(Heap* heap, RelocInfo* rinfo,
+                                         Callback callback) {
+  DCHECK(rinfo->rmode() == RelocInfo::EMBEDDED_OBJECT);
+  Object* target = rinfo->target_object();
+  Object* old_target = target;
+  SlotCallbackResult result = callback(heap, &target);
+  if (target != old_target) {
+    rinfo->set_target_object(target);
+  }
+  return result;
+}
+
+// Updates a debug target slot using an untyped slot callback.
+// The callback accepts (Heap*, Object**) and returns SlotCallbackResult.
+template <typename Callback>
+SlotCallbackResult UpdateDebugTarget(Heap* heap, RelocInfo* rinfo,
+                                     Callback callback) {
+  DCHECK(RelocInfo::IsDebugBreakSlot(rinfo->rmode()) &&
+         rinfo->IsPatchedDebugBreakSlotSequence());
+  Object* target = Code::GetCodeFromTargetAddress(rinfo->debug_call_address());
+  SlotCallbackResult result = callback(heap, &target);
+  rinfo->set_debug_call_address(Code::cast(target)->instruction_start());
+  return result;
+}
+
+// Updates a typed slot using an untyped slot callback.
+// The callback accepts (Heap*, Object**) and returns SlotCallbackResult.
+template <typename Callback>
+static inline SlotCallbackResult UpdateTypedSlot(Isolate* isolate,
+                                                 SlotType slot_type,
+                                                 Address addr,
+                                                 Callback callback) {
+  Heap* heap = isolate->heap();
   switch (slot_type) {
     case CODE_TARGET_SLOT: {
       RelocInfo rinfo(isolate, addr, RelocInfo::CODE_TARGET, 0, NULL);
-      rinfo.Visit(isolate, v);
-      break;
+      return UpdateCodeTarget(heap, &rinfo, callback);
     }
     case CELL_TARGET_SLOT: {
       RelocInfo rinfo(isolate, addr, RelocInfo::CELL, 0, NULL);
-      rinfo.Visit(isolate, v);
-      break;
+      return UpdateCell(heap, &rinfo, callback);
     }
     case CODE_ENTRY_SLOT: {
-      v->VisitCodeEntry(addr);
-      break;
+      return UpdateCodeEntry(heap, addr, callback);
     }
     case DEBUG_TARGET_SLOT: {
       RelocInfo rinfo(isolate, addr, RelocInfo::DEBUG_BREAK_SLOT_AT_POSITION, 0,
                       NULL);
-      if (rinfo.IsPatchedDebugBreakSlotSequence()) rinfo.Visit(isolate, v);
-      break;
+      if (rinfo.IsPatchedDebugBreakSlotSequence()) {
+        return UpdateDebugTarget(heap, &rinfo, callback);
+      }
+      return REMOVE_SLOT;
     }
     case EMBEDDED_OBJECT_SLOT: {
       RelocInfo rinfo(isolate, addr, RelocInfo::EMBEDDED_OBJECT, 0, NULL);
-      rinfo.Visit(isolate, v);
-      break;
+      return UpdateEmbeddedPointer(heap, &rinfo, callback);
     }
     case OBJECT_SLOT: {
-      v->VisitPointer(reinterpret_cast<Object**>(addr));
-      break;
+      return callback(heap, reinterpret_cast<Object**>(addr));
     }
-    default:
+    case NUMBER_OF_SLOT_TYPES:
       UNREACHABLE();
       break;
   }
 }
 
 
 // Visitor for updating pointers from live objects in old spaces to new space.
 // It does not expect to encounter pointers to dead objects.
 class PointersUpdatingVisitor : public ObjectVisitor {
  public:
   explicit PointersUpdatingVisitor(Heap* heap) : heap_(heap) {}
 
-  void VisitPointer(Object** p) override { UpdatePointer(p); }
+  void VisitPointer(Object** p) override { UpdateSlot(heap_, p); }
 
   void VisitPointers(Object** start, Object** end) override {
-    for (Object** p = start; p < end; p++) UpdatePointer(p);
+    for (Object** p = start; p < end; p++) UpdateSlot(heap_, p);
   }
 
   void VisitCell(RelocInfo* rinfo) override {
-    DCHECK(rinfo->rmode() == RelocInfo::CELL);
-    Object* cell = rinfo->target_cell();
-    Object* old_cell = cell;
-    VisitPointer(&cell);
-    if (cell != old_cell) {
-      rinfo->set_target_cell(reinterpret_cast<Cell*>(cell));
-    }
+    UpdateCell(heap_, rinfo, UpdateSlot);
   }
 
   void VisitEmbeddedPointer(RelocInfo* rinfo) override {
-    DCHECK(rinfo->rmode() == RelocInfo::EMBEDDED_OBJECT);
-    Object* target = rinfo->target_object();
-    Object* old_target = target;
-    VisitPointer(&target);
-    // Avoid unnecessary changes that might unnecessary flush the instruction
-    // cache.
-    if (target != old_target) {
-      rinfo->set_target_object(target);
-    }
+    UpdateEmbeddedPointer(heap_, rinfo, UpdateSlot);
   }
 
   void VisitCodeTarget(RelocInfo* rinfo) override {
-    DCHECK(RelocInfo::IsCodeTarget(rinfo->rmode()));
-    Object* target = Code::GetCodeFromTargetAddress(rinfo->target_address());
-    Object* old_target = target;
-    VisitPointer(&target);
-    if (target != old_target) {
-      rinfo->set_target_address(Code::cast(target)->instruction_start());
-    }
-  }
-
-  void VisitCodeAgeSequence(RelocInfo* rinfo) override {
-    DCHECK(RelocInfo::IsCodeAgeSequence(rinfo->rmode()));
-    Object* stub = rinfo->code_age_stub();
-    DCHECK(stub != NULL);
-    VisitPointer(&stub);
-    if (stub != rinfo->code_age_stub()) {
-      rinfo->set_code_age_stub(Code::cast(stub));
-    }
+    UpdateCodeTarget(heap_, rinfo, UpdateSlot);
   }
 
   void VisitCodeEntry(Address entry_address) override {
-    Object* code = Code::GetObjectFromEntryAddress(entry_address);
-    Object* old_code = code;
-    VisitPointer(&code);
-    if (code != old_code) {
-      Memory::Address_at(entry_address) =
-          reinterpret_cast<Code*>(code)->entry();
-    }
+    UpdateCodeEntry(heap_, entry_address, UpdateSlot);
   }
 
   void VisitDebugTarget(RelocInfo* rinfo) override {
-    DCHECK(RelocInfo::IsDebugBreakSlot(rinfo->rmode()) &&
-           rinfo->IsPatchedDebugBreakSlotSequence());
-    Object* target =
-        Code::GetCodeFromTargetAddress(rinfo->debug_call_address());
-    VisitPointer(&target);
-    rinfo->set_debug_call_address(Code::cast(target)->instruction_start());
-  }
-
-  static inline void UpdateSlot(Heap* heap, Object** slot) {
-    Object* obj = reinterpret_cast<Object*>(
-        base::NoBarrier_Load(reinterpret_cast<base::AtomicWord*>(slot)));
-
-    if (!obj->IsHeapObject()) return;
-
-    HeapObject* heap_obj = HeapObject::cast(obj);
-
-    MapWord map_word = heap_obj->map_word();
-    if (map_word.IsForwardingAddress()) {
-      DCHECK(heap->InFromSpace(heap_obj) ||
-             MarkCompactCollector::IsOnEvacuationCandidate(heap_obj) ||
-             Page::FromAddress(heap_obj->address())
-                 ->IsFlagSet(Page::COMPACTION_WAS_ABORTED));
-      HeapObject* target = map_word.ToForwardingAddress();
-      base::NoBarrier_CompareAndSwap(
-          reinterpret_cast<base::AtomicWord*>(slot),
-          reinterpret_cast<base::AtomicWord>(obj),
-          reinterpret_cast<base::AtomicWord>(target));
-      DCHECK(!heap->InFromSpace(target) &&
-             !MarkCompactCollector::IsOnEvacuationCandidate(target));
-    }
+    UpdateDebugTarget(heap_, rinfo, UpdateSlot);
   }
 
  private:
-  inline void UpdatePointer(Object** p) { UpdateSlot(heap_, p); }
-
   Heap* heap_;
 };
 
 static String* UpdateReferenceInExternalStringTableEntry(Heap* heap,
                                                          Object** p) {
   MapWord map_word = HeapObject::cast(*p)->map_word();
 
   if (map_word.IsForwardingAddress()) {
     return String::cast(map_word.ToForwardingAddress());
   }
@@ skipping 671 matching lines @@
   if (FLAG_verify_heap && !sweeper().sweeping_in_progress()) {
     VerifyEvacuation(heap());
   }
 #endif
 }
 
 template <PointerDirection direction>
 class PointerUpdateJobTraits {
  public:
   typedef int PerPageData;  // Per page data is not used in this job.
-  typedef PointersUpdatingVisitor* PerTaskData;
+  typedef int PerTaskData;  // Per task data is not used in this job.
 
-  static bool ProcessPageInParallel(Heap* heap, PerTaskData visitor,
-                                    MemoryChunk* chunk, PerPageData) {
+  static bool ProcessPageInParallel(Heap* heap, PerTaskData, MemoryChunk* chunk,
+                                    PerPageData) {
     UpdateUntypedPointers(heap, chunk);
-    UpdateTypedPointers(heap, chunk, visitor);
+    UpdateTypedPointers(heap, chunk);
     return true;
   }
   static const bool NeedSequentialFinalization = false;
   static void FinalizePageSequentially(Heap*, MemoryChunk*, bool, PerPageData) {
   }
 
  private:
   static void UpdateUntypedPointers(Heap* heap, MemoryChunk* chunk) {
     if (direction == OLD_TO_NEW) {
       RememberedSet<OLD_TO_NEW>::IterateWithWrapper(heap, chunk,
                                                     UpdateOldToNewSlot);
     } else {
       RememberedSet<OLD_TO_OLD>::Iterate(chunk, [heap](Address slot) {
-        PointersUpdatingVisitor::UpdateSlot(heap,
-                                            reinterpret_cast<Object**>(slot));
-        return REMOVE_SLOT;
+        return UpdateSlot(heap, reinterpret_cast<Object**>(slot));
       });
     }
   }
 
-  static void UpdateTypedPointers(Heap* heap, MemoryChunk* chunk,
-                                  PointersUpdatingVisitor* visitor) {
+  static void UpdateTypedPointers(Heap* heap, MemoryChunk* chunk) {
     if (direction == OLD_TO_OLD) {
       Isolate* isolate = heap->isolate();
       RememberedSet<OLD_TO_OLD>::IterateTyped(
-          chunk, [isolate, visitor](SlotType type, Address slot) {
-            UpdateTypedSlot(isolate, visitor, type, slot);
-            return REMOVE_SLOT;
+          chunk, [isolate](SlotType type, Address slot) {
+            return UpdateTypedSlot(isolate, type, slot, UpdateSlot);
           });
     }
   }
 
   static void UpdateOldToNewSlot(HeapObject** address, HeapObject* object) {
     MapWord map_word = object->map_word();
     // There could still be stale pointers in large object space, map space,
     // and old space for pages that have been promoted.
     if (map_word.IsForwardingAddress()) {
       // Update the corresponding slot.
       *address = map_word.ToForwardingAddress();
     }
   }
 };
 
 int NumberOfPointerUpdateTasks(int pages) {
   if (!FLAG_parallel_pointer_update) return 1;
   const int kMaxTasks = 4;
   const int kPagesPerTask = 4;
   return Min(kMaxTasks, (pages + kPagesPerTask - 1) / kPagesPerTask);
 }
 
 template <PointerDirection direction>
 void UpdatePointersInParallel(Heap* heap) {
   PageParallelJob<PointerUpdateJobTraits<direction> > job(
       heap, heap->isolate()->cancelable_task_manager());
   RememberedSet<direction>::IterateMemoryChunks(
       heap, [&job](MemoryChunk* chunk) { job.AddPage(chunk, 0); });
-  PointersUpdatingVisitor visitor(heap);
   int num_pages = job.NumberOfPages();
   int num_tasks = NumberOfPointerUpdateTasks(num_pages);
-  job.Run(num_tasks, [&visitor](int i) { return &visitor; });
+  job.Run(num_tasks, [](int i) { return 0; });
 }
 
 class ToSpacePointerUpdateJobTraits {
  public:
   typedef std::pair<Address, Address> PerPageData;
   typedef PointersUpdatingVisitor* PerTaskData;
 
   static bool ProcessPageInParallel(Heap* heap, PerTaskData visitor,
                                     MemoryChunk* chunk, PerPageData limits) {
     for (Address cur = limits.first; cur < limits.second;) {
@@ skipping 291 matching lines @@
     MarkBit mark_bit = Marking::MarkBitFrom(host);
     if (Marking::IsBlack(mark_bit)) {
       RelocInfo rinfo(isolate(), pc, RelocInfo::CODE_TARGET, 0, host);
       RecordRelocSlot(host, &rinfo, target);
     }
   }
 }
 
 }  // namespace internal
 }  // namespace v8