[heap] Build proper iterator for iterating live objects

- Iterator advancing is kept mainly unchanged.
- The iterator stores the size of the object which is to be used by the
  caller in follow ups. This way we might be able to avoid further out
  of line loads.
- The iteartor follows the regular std conventions allowing range based
  loops.

Bug: chromium:651354
Change-Id: I8928224a62d3a48a48145a2d00279a28608bc634
Reviewed-on: https://chromium-review.googlesource.com/543335
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46085}

src/heap/mark-compact-inl.h

@@ -48,10 +48,45 @@ void MarkCompactCollector::RecordSlot(HeapObject* object, Object** slot,
   }
 }
 
-template <LiveObjectIterationMode T>
-HeapObject* LiveObjectIterator<T>::Next() {
+template <LiveObjectIterationMode mode>
+LiveObjectRange<mode>::iterator::iterator(MemoryChunk* chunk,
+                                          MarkingState state, Address start)
+    : chunk_(chunk),
+      one_word_filler_map_(chunk->heap()->one_pointer_filler_map()),
+      two_word_filler_map_(chunk->heap()->two_pointer_filler_map()),
+      free_space_map_(chunk->heap()->free_space_map()),
+      it_(chunk, state) {
+  it_.Advance(Bitmap::IndexToCell(
+      Bitmap::CellAlignIndex(chunk_->AddressToMarkbitIndex(start))));
+  if (!it_.Done()) {
+    cell_base_ = it_.CurrentCellBase();
+    current_cell_ = *it_.CurrentCell();
+    AdvanceToNextValidObject();
+  } else {
+    current_object_ = nullptr;
+  }
+}
+
+template <LiveObjectIterationMode mode>
+typename LiveObjectRange<mode>::iterator& LiveObjectRange<mode>::iterator::
+operator++() {
+  AdvanceToNextValidObject();
+  return *this;
+}
+
+template <LiveObjectIterationMode mode>
+typename LiveObjectRange<mode>::iterator LiveObjectRange<mode>::iterator::
+operator++(int) {
+  iterator retval = *this;
+  ++(*this);
+  return retval;
+}
+
+template <LiveObjectIterationMode mode>
+void LiveObjectRange<mode>::iterator::AdvanceToNextValidObject() {
   while (!it_.Done()) {
     HeapObject* object = nullptr;
+    int size = 0;
     while (current_cell_ != 0) {
       uint32_t trailing_zeros = base::bits::CountTrailingZeros32(current_cell_);
       Address addr = cell_base_ + trailing_zeros * kPointerSize;
@@ -59,10 +94,8 @@ HeapObject* LiveObjectIterator<T>::Next() {
       // Clear the first bit of the found object..
       current_cell_ &= ~(1u << trailing_zeros);
 
-      uint32_t second_bit_index = 0;
-      if (trailing_zeros < Bitmap::kBitIndexMask) {
-        second_bit_index = 1u << (trailing_zeros + 1);
-      } else {
+      uint32_t second_bit_index = 1u << (trailing_zeros + 1);
+      if (trailing_zeros >= Bitmap::kBitIndexMask) {
         second_bit_index = 0x1;
         // The overlapping case; there has to exist a cell after the current
         // cell.
@@ -71,7 +104,8 @@ HeapObject* LiveObjectIterator<T>::Next() {
         // that case we can return immediately.
         if (!it_.Advance()) {
           DCHECK(HeapObject::FromAddress(addr)->map() == one_word_filler_map_);
-          return nullptr;
+          current_object_ = nullptr;
+          return;
         }
         cell_base_ = it_.CurrentCellBase();
         current_cell_ = *it_.CurrentCell();
@@ -84,7 +118,8 @@ HeapObject* LiveObjectIterator<T>::Next() {
         // object ends.
         HeapObject* black_object = HeapObject::FromAddress(addr);
         map = base::NoBarrierAtomicValue<Map*>::FromAddress(addr)->Value();
-        Address end = addr + black_object->SizeFromMap(map) - kPointerSize;
+        size = black_object->SizeFromMap(map);
+        Address end = addr + size - kPointerSize;
         // One word filler objects do not borrow the second mark bit. We have
         // to jump over the advancing and clearing part.
         // Note that we know that we are at a one word filler when
@@ -105,12 +140,13 @@ HeapObject* LiveObjectIterator<T>::Next() {
           current_cell_ &= ~(end_index_mask + end_index_mask - 1);
         }
 
-        if (T == kBlackObjects || T == kAllLiveObjects) {
+        if (mode == kBlackObjects || mode == kAllLiveObjects) {
           object = black_object;
         }
-      } else if ((T == kGreyObjects || T == kAllLiveObjects)) {
+      } else if ((mode == kGreyObjects || mode == kAllLiveObjects)) {
         map = base::NoBarrierAtomicValue<Map*>::FromAddress(addr)->Value();
         object = HeapObject::FromAddress(addr);
+        size = object->SizeFromMap(map);
       }
 
       // We found a live object.
@@ -139,9 +175,23 @@ HeapObject* LiveObjectIterator<T>::Next() {
         current_cell_ = *it_.CurrentCell();
       }
     }
-    if (object != nullptr) return object;
+    if (object != nullptr) {
+      current_object_ = object;
+      current_size_ = size;
+      return;
+    }
   }
-  return nullptr;
+  current_object_ = nullptr;
+}
+
+template <LiveObjectIterationMode mode>
+typename LiveObjectRange<mode>::iterator LiveObjectRange<mode>::begin() {
+  return iterator(chunk_, state_, start_);
+}
+
+template <LiveObjectIterationMode mode>
+typename LiveObjectRange<mode>::iterator LiveObjectRange<mode>::end() {
+  return iterator(chunk_, state_, end_);
 }
 
 }  // namespace internal

src/heap/mark-compact.cc

@@ -1435,11 +1435,10 @@ void MarkCompactCollector::DiscoverGreyObjectsWithIterator(T* it) {
 
 void MarkCompactCollector::DiscoverGreyObjectsOnPage(MemoryChunk* p) {
   DCHECK(!marking_deque()->IsFull());
-  LiveObjectIterator<kGreyObjects> it(p, MarkingState::Internal(p));
-  HeapObject* object = NULL;
-  while ((object = it.Next()) != NULL) {
-    bool success =
-        ObjectMarking::GreyToBlack(object, MarkingState::Internal(object));
+  for (auto object_and_size :
+       LiveObjectRange<kGreyObjects>(p, marking_state(p))) {
+    HeapObject* const object = object_and_size.first;
+    bool success = ObjectMarking::GreyToBlack(object, marking_state(object));
     DCHECK(success);
     USE(success);
     PushBlack(object);
@@ -2773,10 +2772,10 @@ void MinorMarkCompactCollector::MakeIterable(
   MarkCompactCollector* full_collector = heap()->mark_compact_collector();
   Address free_start = p->area_start();
   DCHECK(reinterpret_cast<intptr_t>(free_start) % (32 * kPointerSize) == 0);
-  LiveObjectIterator<kGreyObjects> it(p, marking_state(p));
-  HeapObject* object = nullptr;
 
-  while ((object = it.Next()) != nullptr) {
+  for (auto object_and_size :
+       LiveObjectRange<kGreyObjects>(p, marking_state(p))) {
+    HeapObject* const object = object_and_size.first;
     DCHECK(ObjectMarking::IsGrey(object, marking_state(object)));
     Address free_end = object->address();
     if (free_end != free_start) {
@@ -4021,10 +4020,8 @@ int MarkCompactCollector::Sweeper::RawSweep(
   intptr_t max_freed_bytes = 0;
   int curr_region = -1;
 
-  LiveObjectIterator<kBlackObjects> it(p, state);
-  HeapObject* object = NULL;
-
-  while ((object = it.Next()) != NULL) {
+  for (auto object_and_size : LiveObjectRange<kBlackObjects>(p, state)) {
+    HeapObject* const object = object_and_size.first;
     DCHECK(ObjectMarking::IsBlack(object, state));
     Address free_end = object->address();
     if (free_end != free_start) {
@@ -4145,10 +4142,8 @@ bool LiveObjectVisitor::VisitBlackObjects(MemoryChunk* chunk,
                                           const MarkingState& state,
                                           Visitor* visitor,
                                           IterationMode iteration_mode) {
-  LiveObjectIterator<kBlackObjects> it(chunk, state);
-  HeapObject* object = nullptr;
-  while ((object = it.Next()) != nullptr) {
-    DCHECK(ObjectMarking::IsBlack(object, state));
+  for (auto object_and_size : LiveObjectRange<kBlackObjects>(chunk, state)) {
+    HeapObject* const object = object_and_size.first;
     if (!visitor->Visit(object)) {
       if (iteration_mode == kClearMarkbits) {
         state.bitmap()->ClearRange(
@@ -4175,9 +4170,8 @@ bool LiveObjectVisitor::VisitGreyObjectsNoFail(MemoryChunk* chunk,
                                                const MarkingState& state,
                                                Visitor* visitor,
                                                IterationMode iteration_mode) {
-  LiveObjectIterator<kGreyObjects> it(chunk, state);
-  HeapObject* object = nullptr;
-  while ((object = it.Next()) != nullptr) {
+  for (auto object_and_size : LiveObjectRange<kGreyObjects>(chunk, state)) {
+    HeapObject* const object = object_and_size.first;
     DCHECK(ObjectMarking::IsGrey(object, state));
     if (!visitor->Visit(object)) {
       UNREACHABLE();
@@ -4191,11 +4185,9 @@ bool LiveObjectVisitor::VisitGreyObjectsNoFail(MemoryChunk* chunk,
 
 void LiveObjectVisitor::RecomputeLiveBytes(MemoryChunk* chunk,
                                            const MarkingState& state) {
-  LiveObjectIterator<kAllLiveObjects> it(chunk, state);
   int new_live_size = 0;
-  HeapObject* object = nullptr;
-  while ((object = it.Next()) != nullptr) {
-    new_live_size += object->Size();
+  for (auto object_and_size : LiveObjectRange<kAllLiveObjects>(chunk, state)) {
+    new_live_size += object_and_size.first->Size();
   }
   state.SetLiveBytes(new_live_size);
 }
@@ -4334,10 +4326,9 @@ class ToSpaceUpdatingItem : public UpdatingItem {
     // For young generation evacuations we want to visit grey objects, for
     // full MC, we need to visit black objects.
     PointersUpdatingVisitor visitor;
-    LiveObjectIterator<kAllLiveObjects> it(chunk_, marking_state_);
-    HeapObject* object = nullptr;
-    while ((object = it.Next()) != nullptr) {
-      object->IterateBodyFast(&visitor);
+    for (auto object_and_size :
+         LiveObjectRange<kAllLiveObjects>(chunk_, marking_state_)) {
+      object_and_size.first->IterateBodyFast(&visitor);
     }
   }
 

src/heap/mark-compact.h

@@ -128,7 +128,7 @@ class MarkBitCellIterator BASE_EMBEDDED {
     cells_ = state.bitmap()->cells();
   }
 
-  inline bool Done() { return cell_index_ == last_cell_index_; }
+  inline bool Done() { return cell_index_ >= last_cell_index_; }
 
   inline bool HasNext() { return cell_index_ < last_cell_index_ - 1; }
 
@@ -177,36 +177,65 @@ class MarkBitCellIterator BASE_EMBEDDED {
   Address cell_base_;
 };
 
-// Grey objects can happen on black pages when black objects transition to
-// grey e.g. when calling RecordWrites on them.
 enum LiveObjectIterationMode {
   kBlackObjects,
   kGreyObjects,
   kAllLiveObjects
 };
 
-template <LiveObjectIterationMode T>
-class LiveObjectIterator BASE_EMBEDDED {
+template <LiveObjectIterationMode mode>
+class LiveObjectRange {
  public:
-  LiveObjectIterator(MemoryChunk* chunk, MarkingState state)
-      : chunk_(chunk),
-        it_(chunk_, state),
-        cell_base_(it_.CurrentCellBase()),
-        current_cell_(*it_.CurrentCell()),
-        one_word_filler_map_(chunk->heap()->one_pointer_filler_map()),
-        two_word_filler_map_(chunk->heap()->two_pointer_filler_map()),
-        free_space_map_(chunk->heap()->free_space_map()) {}
+  class iterator {
+   public:
+    using value_type = std::pair<HeapObject*, int /* size */>;
+    using pointer = const value_type*;
+    using reference = const value_type&;
+    using iterator_category = std::forward_iterator_tag;
+
+    inline iterator(MemoryChunk* chunk, MarkingState state, Address start);
+
+    inline iterator& operator++();
+    inline iterator operator++(int);
+
+    bool operator==(iterator other) const {
+      return current_object_ == other.current_object_;
+    }
 
-  V8_INLINE HeapObject* Next();
+    bool operator!=(iterator other) const { return !(*this == other); }
+
+    value_type operator*() {
+      return std::make_pair(current_object_, current_size_);
+    }
 
    private:
+    inline void AdvanceToNextValidObject();
+
     MemoryChunk* const chunk_;
-  MarkBitCellIterator it_;
-  Address cell_base_;
-  MarkBit::CellType current_cell_;
     Map* const one_word_filler_map_;
     Map* const two_word_filler_map_;
     Map* const free_space_map_;
+    MarkBitCellIterator it_;
+    Address cell_base_;
+    MarkBit::CellType current_cell_;
+    HeapObject* current_object_;
+    int current_size_;
+  };
+
+  LiveObjectRange(MemoryChunk* chunk, MarkingState state)
+      : chunk_(chunk),
+        state_(state),
+        start_(chunk_->area_start()),
+        end_(chunk->area_end()) {}
+
+  inline iterator begin();
+  inline iterator end();
+
+ private:
+  MemoryChunk* const chunk_;
+  MarkingState state_;
+  Address start_;
+  Address end_;
 };
 
 class LiveObjectVisitor BASE_EMBEDDED {

test/cctest/heap/test-mark-compact.cc

@@ -355,10 +355,9 @@ TEST(Regress5829) {
                              ClearRecordedSlots::kNo);
   heap->old_space()->EmptyAllocationInfo();
   Page* page = Page::FromAddress(array->address());
-  LiveObjectIterator<kGreyObjects> it(page, MarkingState::Internal(page));
-  HeapObject* object = nullptr;
-  while ((object = it.Next()) != nullptr) {
-    CHECK(!object->IsFiller());
+  for (auto object_and_size :
+       LiveObjectRange<kGreyObjects>(page, MarkingState::Internal(page))) {
+    CHECK(!object_and_size.first->IsFiller());
   }
 }