[heap] Iterate handles with special left-trim visitor

mlippautz · Commit bot · commit a7159577b7d0 · 2016-06-29T08:18:20.000Z
BUG=chromium:620553 LOG=N R=hpayer@chromium.org Review-Url: https://codereview.chromium.org/2102243002 Cr-Commit-Position: refs/heads/master@{#37366}
diff --git a/src/heap/heap-inl.h b/src/heap/heap-inl.h
@@ -482,31 +482,6 @@ void Heap::CopyBlock(Address dst, Address src, int byte_size) {
             static_cast<size_t>(byte_size / kPointerSize));
 }
 
-bool Heap::PurgeLeftTrimmedObject(Object** object) {
-  HeapObject* current = reinterpret_cast<HeapObject*>(*object);
-  const MapWord map_word = current->map_word();
-  if (current->IsFiller() && !map_word.IsForwardingAddress()) {
-#ifdef DEBUG
-    // We need to find a FixedArrayBase map after walking the fillers.
-    while (current->IsFiller()) {
-      Address next = reinterpret_cast<Address>(current);
-      if (current->map() == one_pointer_filler_map()) {
-        next += kPointerSize;
-      } else if (current->map() == two_pointer_filler_map()) {
-        next += 2 * kPointerSize;
-      } else {
-        next += current->Size();
-      }
-      current = reinterpret_cast<HeapObject*>(next);
-    }
-    DCHECK(current->IsFixedArrayBase());
-#endif  // DEBUG
-    *object = nullptr;
-    return true;
-  }
-  return false;
-}
-
 template <Heap::FindMementoMode mode>
 AllocationMemento* Heap::FindAllocationMemento(HeapObject* object) {
   // Check if there is potentially a memento behind the object. If
diff --git a/src/heap/heap.cc b/src/heap/heap.cc
@@ -4755,6 +4755,49 @@ void Heap::IterateSmiRoots(ObjectVisitor* v) {
   v->Synchronize(VisitorSynchronization::kSmiRootList);
 }
 
+// We cannot avoid stale handles to left-trimmed objects, but can only make
+// sure all handles still needed are updated. Filter out a stale pointer
+// and clear the slot to allow post processing of handles (needed because
+// the sweeper might actually free the underlying page).
+class FixStaleLeftTrimmedHandlesVisitor : public ObjectVisitor {
+ public:
+  explicit FixStaleLeftTrimmedHandlesVisitor(Heap* heap) : heap_(heap) {
+    USE(heap_);
+  }
+
+  void VisitPointer(Object** p) override { FixHandle(p); }
+
+  void VisitPointers(Object** start, Object** end) override {
+    for (Object** p = start; p < end; p++) FixHandle(p);
+  }
+
+ private:
+  inline void FixHandle(Object** p) {
+    HeapObject* current = reinterpret_cast<HeapObject*>(*p);
+    if (!current->IsHeapObject()) return;
+    const MapWord map_word = current->map_word();
+    if (!map_word.IsForwardingAddress() && current->IsFiller()) {
+#ifdef DEBUG
+      // We need to find a FixedArrayBase map after walking the fillers.
+      while (current->IsFiller()) {
+        Address next = reinterpret_cast<Address>(current);
+        if (current->map() == heap_->one_pointer_filler_map()) {
+          next += kPointerSize;
+        } else if (current->map() == heap_->two_pointer_filler_map()) {
+          next += 2 * kPointerSize;
+        } else {
+          next += current->Size();
+        }
+        current = reinterpret_cast<HeapObject*>(next);
+      }
+      DCHECK(current->IsFixedArrayBase());
+#endif  // DEBUG
+      *p = nullptr;
+    }
+  }
+
+  Heap* heap_;
+};
 
 void Heap::IterateStrongRoots(ObjectVisitor* v, VisitMode mode) {
   v->VisitPointers(&roots_[0], &roots_[kStrongRootListLength]);
@@ -4777,6 +4820,8 @@ void Heap::IterateStrongRoots(ObjectVisitor* v, VisitMode mode) {
   v->Synchronize(VisitorSynchronization::kCompilationCache);
 
   // Iterate over local handles in handle scopes.
+  FixStaleLeftTrimmedHandlesVisitor left_trim_visitor(this);
+  isolate_->handle_scope_implementer()->Iterate(&left_trim_visitor);
   isolate_->handle_scope_implementer()->Iterate(v);
   isolate_->IterateDeferredHandles(v);
   v->Synchronize(VisitorSynchronization::kHandleScope);
diff --git a/src/heap/heap.h b/src/heap/heap.h
@@ -632,12 +632,6 @@ class Heap {
   // stored on the map to facilitate fast dispatch for {StaticVisitorBase}.
   static int GetStaticVisitorIdForMap(Map* map);
 
-  // We cannot avoid stale handles to left-trimmed objects, but can only make
-  // sure all handles still needed are updated. Filter out a stale pointer
-  // and clear the slot to allow post processing of handles (needed because
-  // the sweeper might actually free the underlying page).
-  inline bool PurgeLeftTrimmedObject(Object** object);
-
   // Notifies the heap that is ok to start marking or other activities that
   // should not happen during deserialization.
   void NotifyDeserializationComplete();
diff --git a/src/heap/mark-compact.cc b/src/heap/mark-compact.cc
@@ -1413,8 +1413,6 @@ class RootMarkingVisitor : public ObjectVisitor {
 
     HeapObject* object = HeapObject::cast(*p);
 
-    if (collector_->heap()->PurgeLeftTrimmedObject(p)) return;
-
     MarkBit mark_bit = Marking::MarkBitFrom(object);
     if (Marking::IsBlackOrGrey(mark_bit)) return;
 
diff --git a/src/heap/scavenger.cc b/src/heap/scavenger.cc
@@ -445,8 +445,6 @@ void ScavengeVisitor::ScavengePointer(Object** p) {
   Object* object = *p;
   if (!heap_->InNewSpace(object)) return;
 
-  if (heap_->PurgeLeftTrimmedObject(p)) return;
-
   Scavenger::ScavengeObject(reinterpret_cast<HeapObject**>(p),
                             reinterpret_cast<HeapObject*>(object));
 }

Original file line number	Diff line number	Diff line change
`@@ -445,8 +445,6 @@ void ScavengeVisitor::ScavengePointer(Object** p) {`
`445`	`445`	`Object* object = *p;`
`446`	`446`	`if (!heap_->InNewSpace(object)) return;`
`447`	`447`
`448`		`- if (heap_->PurgeLeftTrimmedObject(p)) return;`
`449`		`-`
`450`	`448`	`Scavenger::ScavengeObject(reinterpret_cast<HeapObject**>(p),`
`451`	`449`	`reinterpret_cast<HeapObject*>(object));`
`452`	`450`	`}`