[heap] Rework C++ shared marking barrier checks

Simplify the checks in the C++ marking barrier that deal with
shared objects. The checks we now use here are the same we will be
using for the JS barrier in RecordWrite (see
https://crrev.com/c/4020176).

This CL also reworks WriteWithoutHost, the barrier used for traced
handles. It doesn't use MarkValue anymore since the logic is a
bit different to the regular marking barrier on objects.

Bug: v8:13267
Change-Id: If23b65ce5f06af99a5cea864ce28a68f8d5b37de
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4031028
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#84317}

src/heap/marking-barrier-inl.h

@@ -23,19 +23,52 @@ void MarkingBarrier::MarkValue(HeapObject host, HeapObject value) {
   // In that case host has only one markbit and the second markbit belongs to
   // another object. We can detect that case by checking if value is a one word
   // filler map.
-  DCHECK_IMPLIES(
-      !host.is_null(),
-      !marking_state_.IsImpossible(host) ||
-          value == ReadOnlyRoots(heap_->isolate()).one_pointer_filler_map());
+  DCHECK(!marking_state_.IsImpossible(host) ||
+         value == ReadOnlyRoots(heap_->isolate()).one_pointer_filler_map());
 
-  if (V8_UNLIKELY(uses_shared_heap_) && value.InSharedWritableHeap()) {
-    if (ProcessSharedObject(value)) return;
+  // When shared heap isn't enabled all objects are local, we can just run the
+  // local marking barrier. Also from the point-of-view of the shared space
+  // isolate (= main isolate) also shared objects are considered local.
+  if (V8_UNLIKELY(uses_shared_heap_) && !is_shared_space_isolate_) {
+    // Check whether incremental marking is enabled for that object's space.
+    if (!MemoryChunk::FromHeapObject(host)->IsFlagSet(
+            BasicMemoryChunk::Flag::INCREMENTAL_MARKING)) {
+      return;
+    }
+
+    if (host.InSharedWritableHeap()) {
+      // Invoking shared marking barrier when storing into shared objects.
+      MarkValueShared(value);
+      return;
+    } else if (value.InSharedWritableHeap()) {
+      // No marking needed when storing shared objects in local objects.
+      return;
+    }
   }
 
-  DCHECK_IMPLIES(value.InSharedWritableHeap(), is_shared_heap_isolate_);
+  DCHECK_IMPLIES(host.InSharedWritableHeap(), is_shared_space_isolate_);
+  DCHECK_IMPLIES(value.InSharedWritableHeap(), is_shared_space_isolate_);
 
-  if (!is_activated_) return;
+  DCHECK(is_activated_);
+  MarkValueLocal(value);
+}
 
+void MarkingBarrier::MarkValueShared(HeapObject value) {
+  // Value is either in read-only space or shared heap.
+  DCHECK(value.InSharedHeap());
+
+  // We should only reach this on client isolates (= worker isolates).
+  DCHECK(v8_flags.shared_space);
+  DCHECK(!is_shared_space_isolate_);
+  DCHECK(shared_heap_worklist_.has_value());
+
+  // Mark shared object and push it onto shared heap worklist.
+  if (marking_state_.WhiteToGrey(value)) {
+    shared_heap_worklist_->Push(value);
+  }
+}
+
+void MarkingBarrier::MarkValueLocal(HeapObject value) {
   if (is_minor()) {
     // We do not need to insert into RememberedSet<OLD_TO_NEW> here because the
     // C++ marking barrier already does this for us.
@@ -51,33 +84,6 @@ void MarkingBarrier::MarkValue(HeapObject host, HeapObject value) {
   }
 }
 
-bool MarkingBarrier::ProcessSharedObject(HeapObject value) {
-  if (v8_flags.shared_space) {
-    if (is_shared_heap_isolate_) {
-      // The main isolate processes objects in the shared heap as regular local
-      // objects.
-      return false;
-    } else {
-      // Background threads need to mark shared objects when incremental marking
-      // in the shared heap is active.
-      if (shared_heap_worklist_.has_value() &&
-          marking_state_.WhiteToGrey(value)) {
-        DCHECK(v8_flags.shared_space);
-        DCHECK(!is_shared_heap_isolate_);
-        shared_heap_worklist_->Push(value);
-      }
-
-      // Background threads do not process shared objects as regular local
-      // objects.
-      return true;
-    }
-
-  } else {
-    // All threads ignore shared objects during incremental marking.
-    return true;
-  }
-}
-
 template <typename TSlot>
 inline void MarkingBarrier::MarkRange(HeapObject host, TSlot start, TSlot end) {
   auto* isolate = heap_->isolate();

src/heap/marking-barrier.cc

@@ -32,7 +32,7 @@ MarkingBarrier::MarkingBarrier(LocalHeap* local_heap)
       marking_state_(isolate()),
       is_main_thread_barrier_(local_heap->is_main_thread()),
       uses_shared_heap_(isolate()->has_shared_heap()),
-      is_shared_heap_isolate_(isolate()->is_shared_heap_isolate()) {}
+      is_shared_space_isolate_(isolate()->is_shared_space_isolate()) {}
 
 MarkingBarrier::~MarkingBarrier() { DCHECK(typed_slots_map_.empty()); }
 
@@ -53,8 +53,18 @@ void MarkingBarrier::Write(HeapObject host, HeapObjectSlot slot,
 
 void MarkingBarrier::WriteWithoutHost(HeapObject value) {
   DCHECK(is_main_thread_barrier_);
-  DCHECK(is_activated_ || shared_heap_worklist_.has_value());
-  MarkValue(HeapObject(), value);
+  DCHECK(is_activated_);
+
+  // Without a shared heap and on the shared space isolate (= main isolate) all
+  // objects are considered local.
+  if (V8_UNLIKELY(uses_shared_heap_) && !is_shared_space_isolate_) {
+    // On client isolates (= worker isolates) shared values can be ignored.
+    if (value.InSharedWritableHeap()) {
+      return;
+    }
+  }
+
+  MarkValueLocal(value);
 }
 
 void MarkingBarrier::Write(Code host, RelocInfo* reloc_info, HeapObject value) {

src/heap/marking-barrier.h

@@ -56,7 +56,9 @@ class MarkingBarrier {
   Heap* heap() const { return heap_; }
 
  private:
-  inline bool ProcessSharedObject(HeapObject value);
+  inline void MarkValueShared(HeapObject value);
+  inline void MarkValueLocal(HeapObject value);
+
   inline bool WhiteToGreyAndPush(HeapObject value);
 
   void RecordRelocSlot(Code host, RelocInfo* rinfo, HeapObject target);
@@ -86,9 +88,9 @@ class MarkingBarrier {
       typed_slots_map_;
   bool is_compacting_ = false;
   bool is_activated_ = false;
-  bool is_main_thread_barrier_;
+  const bool is_main_thread_barrier_;
   const bool uses_shared_heap_;
-  const bool is_shared_heap_isolate_;
+  const bool is_shared_space_isolate_;
   MarkingBarrierType marking_barrier_type_;
 };