Revert "[heap] Use normal marking write barrier for fixed array elements"

This reverts commit 7c426286.

Reason for revert: Compile error on cfi: https://ci.chromium.org/p/v8/builders/ci/V8%20Linux64%20-%20cfi/20196

Original change's description:
> [heap] Use normal marking write barrier for fixed array elements
> 
> This simplifies the marking write barrier for elements to mark the
> values instead of revisiting the array.
> 
> Bug: chromium:918485
> 
> Change-Id: Id5da0d5b9ff8385a256fe14f4bf7171f9f6343e1
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1588459
> Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
> Reviewed-by: Hannes Payer <hpayer@chromium.org>
> Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#61125}

TBR=ulan@chromium.org,hpayer@chromium.org,mlippautz@chromium.org

Change-Id: I8576fe00b19db906aa80ca9cb54c3b8cc95e3d97
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:918485
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1590076Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61128}

src/heap/concurrent-marking.cc

@@ -330,14 +330,24 @@ class ConcurrentMarkingVisitor final
     marking_state_.GreyToBlack(object);
     int size = FixedArray::BodyDescriptor::SizeOf(map, object);
     size_t current_progress_bar = chunk->ProgressBar();
-    int start = std::max(static_cast<int>(current_progress_bar),
-                         FixedArray::BodyDescriptor::kStartOffset);
+    if (current_progress_bar == 0) {
+      // Try to move the progress bar forward to start offset. This solves the
+      // problem of not being able to observe a progress bar reset when
+      // processing the first kProgressBarScanningChunk.
+      if (!chunk->TrySetProgressBar(0,
+                                    FixedArray::BodyDescriptor::kStartOffset))
+        return 0;
+      current_progress_bar = FixedArray::BodyDescriptor::kStartOffset;
+    }
+    int start = static_cast<int>(current_progress_bar);
     int end = Min(size, start + kProgressBarScanningChunk);
     if (start < end) {
       VisitPointers(object, object.RawField(start), object.RawField(end));
-      bool success = chunk->TrySetProgressBar(current_progress_bar, end);
-      CHECK(success);
-      if (end < size) {
+      // Setting the progress bar can fail if the object that is currently
+      // scanned is also revisited. In this case, there may be two tasks racing
+      // on the progress counter. The looser can bail out because the progress
+      // bar is reset before the tasks race on the object.
+      if (chunk->TrySetProgressBar(current_progress_bar, end) && (end < size)) {
         // The object can be pushed back onto the marking worklist only after
         // progress bar was updated.
         shared_.Push(object);

src/heap/heap-write-barrier-inl.h

@@ -43,7 +43,7 @@ V8_EXPORT_PRIVATE void Heap_GenerationalBarrierForElementsSlow(Heap* heap,
                                                                int offset,
                                                                int length);
 V8_EXPORT_PRIVATE void Heap_MarkingBarrierForElementsSlow(Heap* heap,
-                                                          FixedArray object);
+                                                          HeapObject object);
 V8_EXPORT_PRIVATE void Heap_MarkingBarrierForDescriptorArraySlow(
     Heap* heap, HeapObject host, HeapObject descriptor_array,
     int number_of_own_descriptors);
@@ -216,12 +216,12 @@ inline void MarkingBarrier(HeapObject object, MaybeObjectSlot slot,
                                          value_heap_object);
 }
 
-inline void MarkingBarrierForElements(Heap* heap, FixedArray array) {
+inline void MarkingBarrierForElements(Heap* heap, HeapObject object) {
   heap_internals::MemoryChunk* object_chunk =
-      heap_internals::MemoryChunk::FromHeapObject(array);
+      heap_internals::MemoryChunk::FromHeapObject(object);
   if (!object_chunk->IsMarking()) return;
 
-  Heap_MarkingBarrierForElementsSlow(heap, array);
+  Heap_MarkingBarrierForElementsSlow(heap, object);
 }
 
 inline void MarkingBarrierForCode(Code host, RelocInfo* rinfo,

src/heap/heap-write-barrier.h

@@ -47,7 +47,7 @@ void GenerationalBarrierForCode(Code host, RelocInfo* rinfo, HeapObject object);
 // Marking write barrier.
 void MarkingBarrier(HeapObject object, ObjectSlot slot, Object value);
 void MarkingBarrier(HeapObject object, MaybeObjectSlot slot, MaybeObject value);
-void MarkingBarrierForElements(Heap* heap, FixedArray array);
+void MarkingBarrierForElements(Heap* heap, HeapObject object);
 void MarkingBarrierForCode(Code host, RelocInfo* rinfo, HeapObject object);
 
 void MarkingBarrierForDescriptorArray(Heap* heap, HeapObject host,

src/heap/heap.cc

@@ -30,7 +30,6 @@
 #include "src/heap/gc-tracer.h"
 #include "src/heap/heap-controller.h"
 #include "src/heap/heap-write-barrier-inl.h"
-#include "src/heap/incremental-marking-inl.h"
 #include "src/heap/incremental-marking.h"
 #include "src/heap/mark-compact-inl.h"
 #include "src/heap/mark-compact.h"
@@ -112,8 +111,8 @@ void Heap_GenerationalBarrierForElementsSlow(Heap* heap, FixedArray array,
   Heap::GenerationalBarrierForElementsSlow(heap, array, offset, length);
 }
 
-void Heap_MarkingBarrierForElementsSlow(Heap* heap, FixedArray array) {
-  Heap::MarkingBarrierForElementsSlow(heap, array);
+void Heap_MarkingBarrierForElementsSlow(Heap* heap, HeapObject object) {
+  Heap::MarkingBarrierForElementsSlow(heap, object);
 }
 
 void Heap_MarkingBarrierForDescriptorArraySlow(Heap* heap, HeapObject host,
@@ -5896,8 +5895,14 @@ void Heap::MarkingBarrierSlow(HeapObject object, Address slot,
                                                value);
 }
 
-void Heap::MarkingBarrierForElementsSlow(Heap* heap, FixedArray array) {
-  heap->incremental_marking()->RecordWrites(array);
+void Heap::MarkingBarrierForElementsSlow(Heap* heap, HeapObject object) {
+  IncrementalMarking::MarkingState* marking_state =
+      heap->incremental_marking()->marking_state();
+  if (!marking_state->IsBlack(object)) {
+    marking_state->WhiteToGrey(object);
+    marking_state->GreyToBlack(object);
+  }
+  heap->incremental_marking()->RevisitObject(object);
 }
 
 void Heap::MarkingBarrierForCodeSlow(Code host, RelocInfo* rinfo,

src/heap/heap.h

@@ -366,8 +366,8 @@ class Heap {
   V8_EXPORT_PRIVATE static void MarkingBarrierSlow(HeapObject object,
                                                    Address slot,
                                                    HeapObject value);
-  V8_EXPORT_PRIVATE static void MarkingBarrierForElementsSlow(Heap* heap,
-                                                              FixedArray array);
+  V8_EXPORT_PRIVATE static void MarkingBarrierForElementsSlow(
+      Heap* heap, HeapObject object);
   V8_EXPORT_PRIVATE static void MarkingBarrierForCodeSlow(Code host,
                                                           RelocInfo* rinfo,
                                                           HeapObject value);

src/heap/incremental-marking.cc

@@ -117,30 +117,6 @@ void IncrementalMarking::RecordWriteIntoCode(Code host, RelocInfo* rinfo,
   }
 }
 
-void IncrementalMarking::RecordWrites(FixedArray array) {
-  int length = array->length();
-  MarkCompactCollector* collector = heap_->mark_compact_collector();
-  MemoryChunk* source_page = MemoryChunk::FromHeapObject(array);
-  if (source_page->ShouldSkipEvacuationSlotRecording<AccessMode::ATOMIC>()) {
-    for (int i = 0; i < length; i++) {
-      Object value = array->get(i);
-      if (value->IsHeapObject()) {
-        BaseRecordWrite(array, HeapObject::cast(value));
-      }
-    }
-  } else {
-    for (int i = 0; i < length; i++) {
-      Object value = array->get(i);
-      if (value->IsHeapObject() &&
-          BaseRecordWrite(array, HeapObject::cast(value))) {
-        collector->RecordSlot(source_page,
-                              HeapObjectSlot(array->RawFieldOfElementAt(i)),
-                              HeapObject::cast(value));
-      }
-    }
-  }
-}
-
 bool IncrementalMarking::WhiteToGreyAndPush(HeapObject obj) {
   if (marking_state()->WhiteToGrey(obj)) {
     marking_worklist()->Push(obj);
@@ -721,6 +697,12 @@ void IncrementalMarking::UpdateMarkedBytesAfterScavenge(
   bytes_marked_ -= Min(bytes_marked_, dead_bytes_in_new_space);
 }
 
+bool IncrementalMarking::IsFixedArrayWithProgressBar(HeapObject obj) {
+  if (!obj->IsFixedArray()) return false;
+  MemoryChunk* chunk = MemoryChunk::FromHeapObject(obj);
+  return chunk->IsFlagSet(MemoryChunk::HAS_PROGRESS_BAR);
+}
+
 int IncrementalMarking::VisitObject(Map map, HeapObject obj) {
   DCHECK(marking_state()->IsGrey(obj) || marking_state()->IsBlack(obj));
   if (!marking_state()->GreyToBlack(obj)) {
@@ -753,9 +735,11 @@ void IncrementalMarking::ProcessBlackAllocatedObject(HeapObject obj) {
 void IncrementalMarking::RevisitObject(HeapObject obj) {
   DCHECK(IsMarking());
   DCHECK(marking_state()->IsBlack(obj));
-  DCHECK_IMPLIES(MemoryChunk::FromHeapObject(obj)->IsFlagSet(
-                     MemoryChunk::HAS_PROGRESS_BAR),
-                 0u == MemoryChunk::FromHeapObject(obj)->ProgressBar());
+  Page* page = Page::FromHeapObject(obj);
+  if (page->owner()->identity() == LO_SPACE ||
+      page->owner()->identity() == NEW_LO_SPACE) {
+    page->ResetProgressBar();
+  }
   Map map = obj->map();
   WhiteToGreyAndPush(map);
   IncrementalMarkingMarkingVisitor visitor(heap()->mark_compact_collector(),

src/heap/incremental-marking.h

@@ -202,7 +202,6 @@ class V8_EXPORT_PRIVATE IncrementalMarking {
   V8_INLINE void RecordWrite(HeapObject obj, ObjectSlot slot, Object value);
   V8_INLINE void RecordMaybeWeakWrite(HeapObject obj, MaybeObjectSlot slot,
                                       MaybeObject value);
-  void RecordWrites(FixedArray array);
   void RevisitObject(HeapObject obj);
   // Ensures that all descriptors int range [0, number_of_own_descripts)
   // are visited.
@@ -286,6 +285,8 @@ class V8_EXPORT_PRIVATE IncrementalMarking {
       intptr_t bytes_to_process,
       ForceCompletionAction completion = DO_NOT_FORCE_COMPLETION);
 
+  V8_INLINE bool IsFixedArrayWithProgressBar(HeapObject object);
+
   // Visits the object and returns its size.
   V8_INLINE int VisitObject(Map map, HeapObject obj);
 

src/heap/mark-compact-inl.h

@@ -398,14 +398,24 @@ int MarkingVisitor<fixed_array_mode, retaining_path_mode, MarkingState>::
     DCHECK(FLAG_use_marking_progress_bar);
     DCHECK(heap_->IsLargeObject(object));
     size_t current_progress_bar = chunk->ProgressBar();
-    int start = std::max(static_cast<int>(current_progress_bar),
-                         FixedArray::BodyDescriptor::kStartOffset);
+    if (current_progress_bar == 0) {
+      // Try to move the progress bar forward to start offset. This solves the
+      // problem of not being able to observe a progress bar reset when
+      // processing the first kProgressBarScanningChunk.
+      if (!chunk->TrySetProgressBar(0,
+                                    FixedArray::BodyDescriptor::kStartOffset))
+        return 0;
+      current_progress_bar = FixedArray::BodyDescriptor::kStartOffset;
+    }
+    int start = static_cast<int>(current_progress_bar);
     int end = Min(size, start + kProgressBarScanningChunk);
     if (start < end) {
       VisitPointers(object, object.RawField(start), object.RawField(end));
-      bool success = chunk->TrySetProgressBar(current_progress_bar, end);
-      CHECK(success);
-      if (end < size) {
+      // Setting the progress bar can fail if the object that is currently
+      // scanned is also revisited. In this case, there may be two tasks racing
+      // on the progress counter. The looser can bail out because the progress
+      // bar is reset before the tasks race on the object.
+      if (chunk->TrySetProgressBar(current_progress_bar, end) && (end < size)) {
         DCHECK(marking_state()->IsBlack(object));
         // The object can be pushed back onto the marking worklist only after
         // progress bar was updated.
@@ -483,22 +493,14 @@ void MarkCompactCollector::RecordSlot(HeapObject object, ObjectSlot slot,
 
 void MarkCompactCollector::RecordSlot(HeapObject object, HeapObjectSlot slot,
                                       HeapObject target) {
-  MemoryChunk* target_page = MemoryChunk::FromHeapObject(target);
-  MemoryChunk* source_page = MemoryChunk::FromHeapObject(object);
+  Page* target_page = Page::FromHeapObject(target);
+  Page* source_page = Page::FromHeapObject(object);
   if (target_page->IsEvacuationCandidate<AccessMode::ATOMIC>() &&
       !source_page->ShouldSkipEvacuationSlotRecording<AccessMode::ATOMIC>()) {
     RememberedSet<OLD_TO_OLD>::Insert(source_page, slot.address());
   }
 }
 
-void MarkCompactCollector::RecordSlot(MemoryChunk* source_page,
-                                      HeapObjectSlot slot, HeapObject target) {
-  MemoryChunk* target_page = MemoryChunk::FromHeapObject(target);
-  if (target_page->IsEvacuationCandidate<AccessMode::ATOMIC>()) {
-    RememberedSet<OLD_TO_OLD>::Insert(source_page, slot.address());
-  }
-}
-
 void MarkCompactCollector::AddTransitionArray(TransitionArray array) {
   weak_objects_.transition_arrays.Push(kMainThread, array);
 }

src/heap/mark-compact.h

@@ -619,8 +619,6 @@ class MarkCompactCollector final : public MarkCompactCollectorBase {
                                    HeapObject target);
   V8_INLINE static void RecordSlot(HeapObject object, HeapObjectSlot slot,
                                    HeapObject target);
-  V8_INLINE static void RecordSlot(MemoryChunk* source_page,
-                                   HeapObjectSlot slot, HeapObject target);
   void RecordLiveSlotsOnPage(Page* page);
 
   void UpdateSlots(SlotsBuffer* buffer);