[heap] Refactor heap object iteration

BUG=

Review-Url: https://codereview.chromium.org/2516303006
Cr-Commit-Position: refs/heads/master@{#41226}

src/heap/array-buffer-tracker.cc

@@ -78,8 +78,8 @@ void LocalArrayBufferTracker::Process(Callback callback) {
 
 void ArrayBufferTracker::FreeDeadInNewSpace(Heap* heap) {
   DCHECK_EQ(heap->gc_state(), Heap::HeapState::SCAVENGE);
-  for (Page* page : NewSpacePageRange(heap->new_space()->FromSpaceStart(),
-                                      heap->new_space()->FromSpaceEnd())) {
+  for (Page* page : PageRange(heap->new_space()->FromSpaceStart(),
+                              heap->new_space()->FromSpaceEnd())) {
     bool empty = ProcessBuffers(page, kUpdateForwardedRemoveOthers);
     CHECK(empty);
   }

src/heap/heap.cc

@@ -4710,8 +4710,8 @@ void Heap::Verify() {
 
 void Heap::ZapFromSpace() {
   if (!new_space_->IsFromSpaceCommitted()) return;
-  for (Page* page : NewSpacePageRange(new_space_->FromSpaceStart(),
-                                      new_space_->FromSpaceEnd())) {
+  for (Page* page :
+       PageRange(new_space_->FromSpaceStart(), new_space_->FromSpaceEnd())) {
     for (Address cursor = page->area_start(), limit = page->area_end();
          cursor < limit; cursor += kPointerSize) {
       Memory::Address_at(cursor) = kFromSpaceZapValue;

src/heap/mark-compact.cc

@@ -133,7 +133,7 @@ static void VerifyMarking(NewSpace* space) {
   // page->area_start() as start of range on all pages.
   CHECK_EQ(space->bottom(), Page::FromAddress(space->bottom())->area_start());
 
-  NewSpacePageRange range(space->bottom(), end);
+  PageRange range(space->bottom(), end);
   for (auto it = range.begin(); it != range.end();) {
     Page* page = *(it++);
     Address limit = it != range.end() ? page->area_end() : end;
@@ -185,29 +185,21 @@ class VerifyEvacuationVisitor : public ObjectVisitor {
 static void VerifyEvacuation(Page* page) {
   VerifyEvacuationVisitor visitor;
   HeapObjectIterator iterator(page);
-  for (HeapObject* heap_object = iterator.Next(); heap_object != NULL;
+  for (HeapObject* heap_object = iterator.Next(); heap_object != nullptr;
        heap_object = iterator.Next()) {
-    // We skip free space objects.
-    if (!heap_object->IsFiller()) {
-      heap_object->Iterate(&visitor);
-    }
+    CHECK(!heap_object->IsFiller());
+    heap_object->Iterate(&visitor);
   }
 }
 
 
 static void VerifyEvacuation(NewSpace* space) {
   VerifyEvacuationVisitor visitor;
-  NewSpacePageRange range(space->bottom(), space->top());
-  for (auto it = range.begin(); it != range.end();) {
-    Page* page = *(it++);
-    Address current = page->area_start();
-    Address limit = it != range.end() ? page->area_end() : space->top();
-    CHECK(limit == space->top() || !page->Contains(space->top()));
-    while (current < limit) {
-      HeapObject* object = HeapObject::FromAddress(current);
-      object->Iterate(&visitor);
-      current += object->Size();
-    }
+  HeapObjectIterator iterator(space);
+  for (HeapObject* heap_object = iterator.Next(); heap_object != nullptr;
+       heap_object = iterator.Next()) {
+    CHECK(!heap_object->IsFiller());
+    heap_object->Iterate(&visitor);
   }
 }
 
@@ -332,7 +324,7 @@ void MarkCompactCollector::VerifyMarkbitsAreClean(PagedSpace* space) {
 
 
 void MarkCompactCollector::VerifyMarkbitsAreClean(NewSpace* space) {
-  for (Page* p : NewSpacePageRange(space->bottom(), space->top())) {
+  for (Page* p : PageRange(space->bottom(), space->top())) {
     CHECK(p->markbits()->IsClean());
     CHECK_EQ(0, p->LiveBytes());
   }
@@ -1964,7 +1956,7 @@ void MarkCompactCollector::DiscoverGreyObjectsInSpace(PagedSpace* space) {
 
 void MarkCompactCollector::DiscoverGreyObjectsInNewSpace() {
   NewSpace* space = heap()->new_space();
-  for (Page* page : NewSpacePageRange(space->bottom(), space->top())) {
+  for (Page* page : PageRange(space->bottom(), space->top())) {
     DiscoverGreyObjectsOnPage(page);
     if (marking_deque()->IsFull()) return;
   }
@@ -3031,7 +3023,7 @@ static String* UpdateReferenceInExternalStringTableEntry(Heap* heap,
 void MarkCompactCollector::EvacuateNewSpacePrologue() {
   NewSpace* new_space = heap()->new_space();
   // Append the list of new space pages to be processed.
-  for (Page* p : NewSpacePageRange(new_space->bottom(), new_space->top())) {
+  for (Page* p : PageRange(new_space->bottom(), new_space->top())) {
     newspace_evacuation_candidates_.Add(p);
   }
   new_space->Flip();
@@ -3817,7 +3809,7 @@ void UpdateToSpacePointersInParallel(Heap* heap, base::Semaphore* semaphore) {
       heap, heap->isolate()->cancelable_task_manager(), semaphore);
   Address space_start = heap->new_space()->bottom();
   Address space_end = heap->new_space()->top();
-  for (Page* page : NewSpacePageRange(space_start, space_end)) {
+  for (Page* page : PageRange(space_start, space_end)) {
     Address start =
         page->Contains(space_start) ? space_start : page->area_start();
     Address end = page->Contains(space_end) ? space_end : page->area_end();

src/heap/spaces-inl.h

@@ -28,10 +28,14 @@ PageIteratorImpl<PAGE_TYPE> PageIteratorImpl<PAGE_TYPE>::operator++(int) {
   return tmp;
 }
 
-NewSpacePageRange::NewSpacePageRange(Address start, Address limit)
-    : range_(Page::FromAddress(start),
-             Page::FromAllocationAreaAddress(limit)->next_page()) {
-  SemiSpace::AssertValidRange(start, limit);
+PageRange::PageRange(Address start, Address limit)
+    : begin_(Page::FromAllocationAreaAddress(start)),
+      end_(Page::FromAllocationAreaAddress(limit)->next_page()) {
+#ifdef DEBUG
+  if (begin_->InNewSpace()) {
+    SemiSpace::AssertValidRange(start, limit);
+  }
+#endif  // DEBUG
 }
 
 // -----------------------------------------------------------------------------
@@ -68,8 +72,14 @@ HeapObject* HeapObjectIterator::Next() {
 
 HeapObject* HeapObjectIterator::FromCurrentPage() {
   while (cur_addr_ != cur_end_) {
-    if (cur_addr_ == space_->top() && cur_addr_ != space_->limit()) {
-      cur_addr_ = space_->limit();
+    // When the current address equals top we have to handle two scenarios:
+    // - Old space page: Move forward to limit if top != limit. We will find
+    //   a filler following limit.
+    // - New space page: We have to stop iteration before the linear allocation
+    //   area as there are no fillers behind it.
+    if (cur_addr_ == space_->top() &&
+        (cur_addr_ != space_->limit() || current_page()->InNewSpace())) {
+      cur_addr_ = current_page()->InNewSpace() ? cur_end_ : space_->limit();
       continue;
     }
     HeapObject* obj = HeapObject::FromAddress(cur_addr_);
@@ -79,7 +89,7 @@ HeapObject* HeapObjectIterator::FromCurrentPage() {
     if (!obj->IsFiller()) {
       if (obj->IsCode()) {
         DCHECK_EQ(space_, space_->heap()->code_space());
-        DCHECK_CODEOBJECT_SIZE(obj_size, space_);
+        DCHECK_CODEOBJECT_SIZE(obj_size, reinterpret_cast<PagedSpace*>(space_));
       } else {
         DCHECK_OBJECT_SIZE(obj_size);
       }

src/heap/spaces.cc

@@ -29,19 +29,26 @@ HeapObjectIterator::HeapObjectIterator(PagedSpace* space)
       cur_end_(nullptr),
       space_(space),
       page_range_(space->anchor()->next_page(), space->anchor()),
-      current_page_(page_range_.begin()) {}
+      next_page_(page_range_.begin()) {}
+
+HeapObjectIterator::HeapObjectIterator(NewSpace* space)
+    : cur_addr_(nullptr),
+      cur_end_(nullptr),
+      space_(space),
+      page_range_(space->bottom(), space->top()),
+      next_page_(page_range_.begin()) {}
 
 HeapObjectIterator::HeapObjectIterator(Page* page)
     : cur_addr_(nullptr),
       cur_end_(nullptr),
-      space_(reinterpret_cast<PagedSpace*>(page->owner())),
+      space_(reinterpret_cast<SpaceWithInlineAllocationArea*>(page->owner())),
       page_range_(page),
-      current_page_(page_range_.begin()) {
+      next_page_(page_range_.begin()) {
 #ifdef DEBUG
   Space* owner = page->owner();
-  DCHECK(owner == page->heap()->old_space() ||
-         owner == page->heap()->map_space() ||
-         owner == page->heap()->code_space());
+  Heap* heap = page->heap();
+  DCHECK(owner == heap->old_space() || owner == heap->map_space() ||
+         owner == heap->code_space() || owner == heap->new_space());
 #endif  // DEBUG
 }
 
@@ -49,8 +56,8 @@ HeapObjectIterator::HeapObjectIterator(Page* page)
 // objects.  This happens at the end of the page.
 bool HeapObjectIterator::AdvanceToNextPage() {
   DCHECK_EQ(cur_addr_, cur_end_);
-  if (current_page_ == page_range_.end()) return false;
-  Page* cur_page = *(current_page_++);
+  if (next_page_ == page_range_.end()) return false;
+  Page* cur_page = *(next_page_++);
   space_->heap()
       ->mark_compact_collector()
       ->sweeper()
@@ -1180,7 +1187,9 @@ void Space::AllocationStep(Address soon_object, int size) {
 
 PagedSpace::PagedSpace(Heap* heap, AllocationSpace space,
                        Executability executable)
-    : Space(heap, space, executable), anchor_(this), free_list_(this) {
+    : SpaceWithInlineAllocationArea(heap, space, executable),
+      anchor_(this),
+      free_list_(this) {
   area_size_ = MemoryAllocator::PageAreaSize(space);
   accounting_stats_.Clear();
 
@@ -2095,7 +2104,7 @@ void SemiSpace::set_age_mark(Address mark) {
   DCHECK_EQ(Page::FromAllocationAreaAddress(mark)->owner(), this);
   age_mark_ = mark;
   // Mark all pages up to the one containing mark.
-  for (Page* p : NewSpacePageRange(space_start(), mark)) {
+  for (Page* p : PageRange(space_start(), mark)) {
     p->SetFlag(MemoryChunk::NEW_SPACE_BELOW_AGE_MARK);
   }
 }

src/heap/spaces.h

@@ -817,7 +817,6 @@ class LargePage : public MemoryChunk {
   friend class MemoryAllocator;
 };
 
-
 // ----------------------------------------------------------------------------
 // Space is the abstract superclass for all allocation spaces.
 class Space : public Malloced {
@@ -923,6 +922,81 @@ class Space : public Malloced {
   DISALLOW_COPY_AND_ASSIGN(Space);
 };
 
+// An abstraction of allocation and relocation pointers in a page-structured
+// space.
+class AllocationInfo {
+ public:
+  AllocationInfo() : original_top_(nullptr), top_(nullptr), limit_(nullptr) {}
+  AllocationInfo(Address top, Address limit)
+      : original_top_(top), top_(top), limit_(limit) {}
+
+  void Reset(Address top, Address limit) {
+    original_top_ = top;
+    set_top(top);
+    set_limit(limit);
+  }
+
+  Address original_top() {
+    SLOW_DCHECK(top_ == NULL ||
+                (reinterpret_cast<intptr_t>(top_) & kHeapObjectTagMask) == 0);
+    return original_top_;
+  }
+
+  INLINE(void set_top(Address top)) {
+    SLOW_DCHECK(top == NULL ||
+                (reinterpret_cast<intptr_t>(top) & kHeapObjectTagMask) == 0);
+    top_ = top;
+  }
+
+  INLINE(Address top()) const {
+    SLOW_DCHECK(top_ == NULL ||
+                (reinterpret_cast<intptr_t>(top_) & kHeapObjectTagMask) == 0);
+    return top_;
+  }
+
+  Address* top_address() { return &top_; }
+
+  INLINE(void set_limit(Address limit)) { limit_ = limit; }
+
+  INLINE(Address limit()) const { return limit_; }
+
+  Address* limit_address() { return &limit_; }
+
+#ifdef DEBUG
+  bool VerifyPagedAllocation() {
+    return (Page::FromAllocationAreaAddress(top_) ==
+            Page::FromAllocationAreaAddress(limit_)) &&
+           (top_ <= limit_);
+  }
+#endif
+
+ private:
+  // The original top address when the allocation info was initialized.
+  Address original_top_;
+  // Current allocation top.
+  Address top_;
+  // Current allocation limit.
+  Address limit_;
+};
+
+class SpaceWithInlineAllocationArea : public Space {
+ public:
+  SpaceWithInlineAllocationArea(Heap* heap, AllocationSpace id,
+                                Executability executable)
+      : Space(heap, id, executable), allocation_info_(nullptr, nullptr) {}
+
+  Address top() { return allocation_info_.top(); }
+  Address limit() { return allocation_info_.limit(); }
+
+  Address* allocation_top_address() { return allocation_info_.top_address(); }
+
+  Address* allocation_limit_address() {
+    return allocation_info_.limit_address();
+  }
+
+ protected:
+  AllocationInfo allocation_info_;
+};
 
 class MemoryChunkValidator {
   // Computed offsets should match the compiler generated ones.
@@ -1416,8 +1490,11 @@ typedef PageIteratorImpl<LargePage> LargePageIterator;
 class PageRange {
  public:
   typedef PageIterator iterator;
-  PageRange(Page* begin, Page* end) : begin_(begin), end_(end) {}
+
   explicit PageRange(Page* page) : PageRange(page, page->next_page()) {}
+  PageRange(Page* begin, Page* end) : begin_(begin), end_(end) {}
+  inline PageRange(Address start, Address limit);
+
   iterator begin() { return iterator(begin_); }
   iterator end() { return iterator(end_); }
 
@@ -1437,8 +1514,8 @@ class PageRange {
 // iterator in order to be sure to visit these new objects.
 class V8_EXPORT_PRIVATE HeapObjectIterator : public ObjectIterator {
  public:
-  // Creates a new object iterator in a given space.
   explicit HeapObjectIterator(PagedSpace* space);
+  explicit HeapObjectIterator(NewSpace* space);
   explicit HeapObjectIterator(Page* page);
 
   // Advance to the next object, skipping free spaces and other fillers and
@@ -1447,6 +1524,7 @@ class V8_EXPORT_PRIVATE HeapObjectIterator : public ObjectIterator {
   inline HeapObject* Next() override;
 
  private:
+  inline Page* current_page() { return (*next_page_)->prev_page(); }
   // Fast (inlined) path of next().
   inline HeapObject* FromCurrentPage();
 
@@ -1456,78 +1534,11 @@ class V8_EXPORT_PRIVATE HeapObjectIterator : public ObjectIterator {
 
   Address cur_addr_;  // Current iteration point.
   Address cur_end_;   // End iteration point.
-  PagedSpace* space_;
+  SpaceWithInlineAllocationArea* space_;
   PageRange page_range_;
-  PageRange::iterator current_page_;
-};
-
-
-// -----------------------------------------------------------------------------
-// A space has a circular list of pages. The next page can be accessed via
-// Page::next_page() call.
-
-// An abstraction of allocation and relocation pointers in a page-structured
-// space.
-class AllocationInfo {
- public:
-  AllocationInfo() : original_top_(nullptr), top_(nullptr), limit_(nullptr) {}
-  AllocationInfo(Address top, Address limit)
-      : original_top_(top), top_(top), limit_(limit) {}
-
-  void Reset(Address top, Address limit) {
-    original_top_ = top;
-    set_top(top);
-    set_limit(limit);
-  }
-
-  Address original_top() {
-    SLOW_DCHECK(top_ == NULL ||
-                (reinterpret_cast<intptr_t>(top_) & kHeapObjectTagMask) == 0);
-    return original_top_;
-  }
-
-  INLINE(void set_top(Address top)) {
-    SLOW_DCHECK(top == NULL ||
-                (reinterpret_cast<intptr_t>(top) & kHeapObjectTagMask) == 0);
-    top_ = top;
-  }
-
-  INLINE(Address top()) const {
-    SLOW_DCHECK(top_ == NULL ||
-                (reinterpret_cast<intptr_t>(top_) & kHeapObjectTagMask) == 0);
-    return top_;
-  }
-
-  Address* top_address() { return &top_; }
-
-  INLINE(void set_limit(Address limit)) {
-    limit_ = limit;
-  }
-
-  INLINE(Address limit()) const {
-    return limit_;
-  }
-
-  Address* limit_address() { return &limit_; }
-
-#ifdef DEBUG
-  bool VerifyPagedAllocation() {
-    return (Page::FromAllocationAreaAddress(top_) ==
-            Page::FromAllocationAreaAddress(limit_)) &&
-           (top_ <= limit_);
-  }
-#endif
-
- private:
-  // The original top address when the allocation info was initialized.
-  Address original_top_;
-  // Current allocation top.
-  Address top_;
-  // Current allocation limit.
-  Address limit_;
+  PageRange::iterator next_page_;
 };
 
-
 // An abstraction of the accounting statistics of a page-structured space.
 //
 // The stats are only set by functions that ensure they stay balanced. These
@@ -1877,18 +1888,7 @@ class LocalAllocationBuffer {
   AllocationInfo allocation_info_;
 };
 
-class NewSpacePageRange {
- public:
-  typedef PageRange::iterator iterator;
-  inline NewSpacePageRange(Address start, Address limit);
-  iterator begin() { return range_.begin(); }
-  iterator end() { return range_.end(); }
-
- private:
-  PageRange range_;
-};
-
-class PagedSpace : public Space {
+class PagedSpace : public SpaceWithInlineAllocationArea {
  public:
   typedef PageIterator iterator;
 
@@ -1966,18 +1966,6 @@ class PagedSpace : public Space {
   // due to being too small to use for allocation.
   virtual size_t Waste() { return free_list_.wasted_bytes(); }
 
-  // Returns the allocation pointer in this space.
-  Address top() { return allocation_info_.top(); }
-  Address limit() { return allocation_info_.limit(); }
-
-  // The allocation top address.
-  Address* allocation_top_address() { return allocation_info_.top_address(); }
-
-  // The allocation limit address.
-  Address* allocation_limit_address() {
-    return allocation_info_.limit_address();
-  }
-
   enum UpdateSkipList { UPDATE_SKIP_LIST, IGNORE_SKIP_LIST };
 
   // Allocate the requested number of bytes in the space if possible, return a
@@ -2150,9 +2138,6 @@ class PagedSpace : public Space {
   // The space's free list.
   FreeList free_list_;
 
-  // Normal allocation information.
-  AllocationInfo allocation_info_;
-
   // Mutex guarding any concurrent access to the space.
   base::Mutex space_mutex_;
 
@@ -2367,12 +2352,12 @@ class SemiSpaceIterator : public ObjectIterator {
 // The new space consists of a contiguous pair of semispaces.  It simply
 // forwards most functions to the appropriate semispace.
 
-class NewSpace : public Space {
+class NewSpace : public SpaceWithInlineAllocationArea {
  public:
   typedef PageIterator iterator;
 
   explicit NewSpace(Heap* heap)
-      : Space(heap, NEW_SPACE, NOT_EXECUTABLE),
+      : SpaceWithInlineAllocationArea(heap, NEW_SPACE, NOT_EXECUTABLE),
         to_space_(heap, kToSpace),
         from_space_(heap, kFromSpace),
         reservation_(),
@@ -2508,18 +2493,6 @@ class NewSpace : public Space {
     return to_space_.minimum_capacity();
   }
 
-  // Return the address of the allocation pointer in the active semispace.
-  Address top() {
-    DCHECK(to_space_.current_page()->ContainsLimit(allocation_info_.top()));
-    return allocation_info_.top();
-  }
-
-  // Return the address of the allocation pointer limit in the active semispace.
-  Address limit() {
-    DCHECK(to_space_.current_page()->ContainsLimit(allocation_info_.limit()));
-    return allocation_info_.limit();
-  }
-
   // Return the address of the first object in the active semispace.
   Address bottom() { return to_space_.space_start(); }
 
@@ -2528,14 +2501,6 @@ class NewSpace : public Space {
   // Set the age mark in the active semispace.
   void set_age_mark(Address mark) { to_space_.set_age_mark(mark); }
 
-  // The allocation top and limit address.
-  Address* allocation_top_address() { return allocation_info_.top_address(); }
-
-  // The allocation limit address.
-  Address* allocation_limit_address() {
-    return allocation_info_.limit_address();
-  }
-
   MUST_USE_RESULT INLINE(AllocationResult AllocateRawAligned(
       int size_in_bytes, AllocationAlignment alignment));
 
@@ -2652,10 +2617,6 @@ class NewSpace : public Space {
   SemiSpace from_space_;
   base::VirtualMemory reservation_;
 
-  // Allocation pointer and limit for normal allocation and allocation during
-  // mark-compact collection.
-  AllocationInfo allocation_info_;
-
   Address top_on_previous_step_;
 
   HistogramInfo* allocated_histogram_;