heap: Minor refactorings

This CL contains minor refactorings to some parts of the garbage
collector:

- Space iterators.
- Removes a redundant call to Heap::CreateFillerObjectAt.
- Heap::CompleteSweepingFull now ensures that sweeping in the C++
  managed heap is also completed.
- Checks, comments and code cleanup.

Change-Id: I14a7fe45c270c463c94c86f45b0e65757249d548
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3377125Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Nikolaos Papaspyrou <nikolaos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78581}

src/heap/heap-inl.h

@@ -331,7 +331,6 @@ HeapObject Heap::AllocateRawWith(int size, AllocationType allocation,
       DCHECK(IsAligned(size, kTaggedSize));
       HeapObject obj = HeapObject::FromAddress(*top);
       *top += size;
-      heap->CreateFillerObjectAt(obj.address(), size, ClearRecordedSlots::kNo);
       MSAN_ALLOCATED_UNINITIALIZED_MEMORY(obj.address(), size);
       return obj;
     }

src/heap/heap.cc

@@ -1423,32 +1423,31 @@ TimedHistogram* Heap::GCTypePriorityTimer(GarbageCollector collector) {
       return isolate_->counters()->gc_scavenger_background();
     }
     return isolate_->counters()->gc_scavenger_foreground();
-  } else {
-    if (!incremental_marking()->IsStopped()) {
-      if (ShouldReduceMemory()) {
-        if (isolate_->IsIsolateInBackground()) {
-          return isolate_->counters()->gc_finalize_reduce_memory_background();
-        }
-        return isolate_->counters()->gc_finalize_reduce_memory_foreground();
-      } else {
-        if (isolate_->IsIsolateInBackground()) {
-          return isolate_->counters()->gc_finalize_background();
-        }
-        return isolate_->counters()->gc_finalize_foreground();
-      }
-    } else {
-      if (isolate_->IsIsolateInBackground()) {
-        return isolate_->counters()->gc_compactor_background();
-      }
-      return isolate_->counters()->gc_compactor_foreground();
+  }
+  DCHECK_EQ(GarbageCollector::MARK_COMPACTOR, collector);
+  if (incremental_marking()->IsStopped()) {
+    if (isolate_->IsIsolateInBackground()) {
+      return isolate_->counters()->gc_compactor_background();
+    }
+    return isolate_->counters()->gc_compactor_foreground();
+  }
+  if (ShouldReduceMemory()) {
+    if (isolate_->IsIsolateInBackground()) {
+      return isolate_->counters()->gc_finalize_reduce_memory_background();
     }
+    return isolate_->counters()->gc_finalize_reduce_memory_foreground();
   }
+  if (isolate_->IsIsolateInBackground()) {
+    return isolate_->counters()->gc_finalize_background();
+  }
+  return isolate_->counters()->gc_finalize_foreground();
 }
 
 TimedHistogram* Heap::GCTypeTimer(GarbageCollector collector) {
   if (IsYoungGenerationCollector(collector)) {
     return isolate_->counters()->gc_scavenger();
   }
+  DCHECK_EQ(GarbageCollector::MARK_COMPACTOR, collector);
   if (incremental_marking()->IsStopped()) {
     return isolate_->counters()->gc_compactor();
   }
@@ -1924,9 +1923,6 @@ void Heap::StartIncrementalMarking(int gc_flags,
   // Sweeping needs to be completed such that markbits are all cleared before
   // starting marking again.
   CompleteSweepingFull();
-  if (cpp_heap()) {
-    CppHeap::From(cpp_heap())->FinishSweepingIfRunning();
-  }
 
   base::Optional<SafepointScope> safepoint_scope;
 
@@ -1953,6 +1949,9 @@ void Heap::CompleteSweepingFull() {
   array_buffer_sweeper()->EnsureFinished();
   mark_compact_collector()->EnsureSweepingCompleted();
   DCHECK(!mark_compact_collector()->sweeping_in_progress());
+  if (cpp_heap()) {
+    CppHeap::From(cpp_heap())->FinishSweepingIfRunning();
+  }
 }
 
 void Heap::StartIncrementalMarkingIfAllocationLimitIsReached(
@@ -2156,9 +2155,6 @@ size_t Heap::PerformGarbageCollection(
   } else {
     DCHECK_EQ(GarbageCollector::MARK_COMPACTOR, collector);
     CompleteSweepingFull();
-    if (cpp_heap()) {
-      CppHeap::From(cpp_heap())->FinishSweepingIfRunning();
-    }
   }
 
   // The last GC cycle is done after completing sweeping. Start the next GC
@@ -6370,20 +6366,9 @@ void Heap::ClearRecordedSlotRange(Address start, Address end) {
 }
 
 PagedSpace* PagedSpaceIterator::Next() {
-  int space = counter_++;
-  switch (space) {
-    case RO_SPACE:
-      UNREACHABLE();
-    case OLD_SPACE:
-      return heap_->old_space();
-    case CODE_SPACE:
-      return heap_->code_space();
-    case MAP_SPACE:
-      return heap_->map_space();
-    default:
-      DCHECK_GT(space, LAST_GROWABLE_PAGED_SPACE);
-      return nullptr;
-  }
+  DCHECK_GE(counter_, FIRST_GROWABLE_PAGED_SPACE);
+  if (counter_ > LAST_GROWABLE_PAGED_SPACE) return nullptr;
+  return heap_->paged_space(counter_++);
 }
 
 SpaceIterator::SpaceIterator(Heap* heap)
@@ -6403,7 +6388,7 @@ bool SpaceIterator::HasNext() {
 }
 
 Space* SpaceIterator::Next() {
-  DCHECK(HasNext());
+  DCHECK_LE(current_space_, LAST_MUTABLE_SPACE);
   Space* space = heap_->space(current_space_++);
   DCHECK_NOT_NULL(space);
   return space;
@@ -6561,6 +6546,8 @@ HeapObjectIterator::HeapObjectIterator(
     default:
       break;
   }
+  // By not calling |space_iterator_->HasNext()|, we assume that the old
+  // space is first returned and that it has been set up.
   object_iterator_ = space_iterator_->Next()->GetObjectIterator(heap_);
   if (V8_ENABLE_THIRD_PARTY_HEAP_BOOL) heap_->tp_heap_->ResetIterator();
 }

src/heap/incremental-marking.cc

@@ -597,7 +597,6 @@ void IncrementalMarking::Hurry() {
   }
 }
 
-
 void IncrementalMarking::Stop() {
   if (IsStopped()) return;
   if (FLAG_trace_incremental_marking) {
@@ -612,8 +611,7 @@ void IncrementalMarking::Stop() {
         std::max(0, old_generation_size_mb - old_generation_limit_mb));
   }
 
-  SpaceIterator it(heap_);
-  while (it.HasNext()) {
+  for (SpaceIterator it(heap_); it.HasNext();) {
     Space* space = it.Next();
     if (space == heap_->new_space()) {
       space->RemoveAllocationObserver(&new_generation_observer_);
@@ -640,13 +638,11 @@ void IncrementalMarking::Stop() {
   background_live_bytes_.clear();
 }
 
-
 void IncrementalMarking::Finalize() {
   Hurry();
   Stop();
 }
 
-
 void IncrementalMarking::FinalizeMarking(CompletionAction action) {
   DCHECK(!finalize_marking_completed_);
   if (FLAG_trace_incremental_marking) {

src/heap/sweeper.cc

@@ -167,6 +167,8 @@ void Sweeper::StartSweeping() {
     // pages to be swept in order to move those objects).
     // Since maps don't move, there is no need to sort the pages from MAP_SPACE
     // before sweeping them.
+    // We sort in descending order of live bytes, i.e., ascending order of free
+    // bytes, because GetSweepingPageSafe returns pages in reverse order.
     if (space != MAP_SPACE) {
       int space_index = GetSweepSpaceIndex(space);
       std::sort(