cppgc: Refactor write barriers

Refactor write barriers and split calls, as e.g. DijkstraWriteBarrier
also contained logic for recording slots (cards) for the young
generation.

The new API exposes the following:
- GetWriteBarrierType(): Retrieving the type of barrier that must be
  emitted;
- DijkstraWriteBarrier(), DijkstraWriteBarrierRange(): Dijkstra-style
  write barriers;
- SteeleWriteBarrier(): Steele-style write barrier;
- GenerationalBarrier(): Barrier for recording slots when using
  multiple generations;

Compilers running with -O3 optimize the DijkstraWriteBarrierPolicy
down to the same instructions as before the split.

Change-Id: If68839cc6357b2f568986c9ce8ca753b1e96a70a
Bug: chromium:1056170
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2557514
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Omer Katz <omerkatz@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71407}

include/cppgc/heap-consistency.h

@@ -25,27 +25,62 @@ namespace subtle {
  */
 class HeapConsistency final {
  public:
+  using WriteBarrierParams = internal::WriteBarrier::Params;
+  using WriteBarrierType = internal::WriteBarrier::Type;
+
   /**
-   * Conservative Dijkstra-style write barrier that processes an object if it
-   * has not yet been processed.
+   * Gets the required write barrier type for a specific write.
    *
-   * \param slot A slot containing the pointer to the object. The slot itself
+   * \param slot Slot containing the pointer to the object. The slot itself
    *   must reside in an object that has been allocated using
    *   `MakeGarbageCollected()`.
-   * \param value The pointer to the object. May be an interior pointer to a
+   * \param value The pointer to the object. May be an interior pointer to an
+   *   interface of the actual object.
+   * \param params Parameters that may be used for actual write barrier calls.
+   *   Only filled if return value indicates that a write barrier is needed. The
+   *   contents of the `params` are an implementation detail.
+   * \returns whether a write barrier is needed and which barrier to invoke.
+   */
+  static V8_INLINE WriteBarrierType GetWriteBarrierType(
+      const void* slot, const void* value, WriteBarrierParams& params) {
+    return internal::WriteBarrier::GetWriteBarrierType(slot, value, params);
+  }
+
+  /**
+   * Gets the required write barrier type for a specific write.
+   *
+   * \param slot Slot containing the pointer to some part of an object object
+   *   that has been allocated using `MakeGarbageCollected()`. Does not consider
+   *   the value of `slot`.
+   * \param params Parameters that may be used for actual write barrier calls.
+   *   Only filled if return value indicates that a write barrier is needed. The
+   *   contents of the `params` are an implementation detail.
+   * \returns whether a write barrier is needed and which barrier to invoke.
+   */
+  static V8_INLINE WriteBarrierType
+  GetWriteBarrierType(const void* slot, WriteBarrierParams& params) {
+    return internal::WriteBarrier::GetWriteBarrierType(slot, params);
+  }
+
+  /**
+   * Conservative Dijkstra-style write barrier that processes an object if it
+   * has not yet been processed.
+   *
+   * \param params The parameters retrieved from `GetWriteBarrierType()`.
+   * \param object The pointer to the object. May be an interior pointer to a
    *   an interface of the actual object.
    */
-  static V8_INLINE void DijkstraWriteBarrier(const void* slot,
-                                             const void* value) {
-    internal::WriteBarrier::DijkstraMarkingBarrier(slot, value);
+  static V8_INLINE void DijkstraWriteBarrier(const WriteBarrierParams& params,
+                                             const void* object) {
+    internal::WriteBarrier::DijkstraMarkingBarrier(params, object);
   }
 
   /**
    * Conservative Dijkstra-style write barrier that processes a range of
    * elements if they have not yet been processed.
    *
-   * \param heap_callback A callback to retrieve the corresponding heap if
-   *   necessary.
+   * \param params The parameters retrieved from `GetWriteBarrierType()`.
+   * \param heap The corresponding heap.
    * \param first_element Pointer to the first element that should be processed.
    *   The slot itself must reside in an object that has been allocated using
    *   `MakeGarbageCollected()`.
@@ -55,13 +90,12 @@ class HeapConsistency final {
    * \param trace_callback The trace callback that should be invoked for each
    *   element if necessary.
    */
-  template <typename LazyHeapCallback>
   static V8_INLINE void DijkstraWriteBarrierRange(
-      LazyHeapCallback heap_callback, const void* first_element,
-      size_t element_size, size_t number_of_elements,
+      const WriteBarrierParams& params, HeapHandle& heap,
+      const void* first_element, size_t element_size, size_t number_of_elements,
       TraceCallback trace_callback) {
     internal::WriteBarrier::DijkstraMarkingBarrierRange(
-        heap_callback, first_element, element_size, number_of_elements,
+        params, heap, first_element, element_size, number_of_elements,
         trace_callback);
   }
 
@@ -69,12 +103,28 @@ class HeapConsistency final {
    * Steele-style write barrier that re-processes an object if it has already
    * been processed.
    *
+   * \param params The parameters retrieved from `GetWriteBarrierType()`.
    * \param object The pointer to the object which must point to an object that
    *   has been allocated using `MakeGarbageCollected()`. Interior pointers are
    *   not supported.
    */
-  static V8_INLINE void SteeleWriteBarrier(const void* object) {
-    internal::WriteBarrier::SteeleMarkingBarrier(object);
+  static V8_INLINE void SteeleWriteBarrier(const WriteBarrierParams& params,
+                                           const void* object) {
+    internal::WriteBarrier::SteeleMarkingBarrier(params, object);
+  }
+
+  /**
+   * Generational barrier for maintaining consistency when running with multiple
+   * generations.
+   *
+   * \param params The parameters retrieved from `GetWriteBarrierType()`.
+   * \param slot Slot containing the pointer to the object. The slot itself
+   *   must reside in an object that has been allocated using
+   *   `MakeGarbageCollected()`.
+   */
+  static V8_INLINE void GenerationalBarrier(const WriteBarrierParams& params,
+                                            const void* slot) {
+    internal::WriteBarrier::GenerationalBarrier(params, slot);
   }
 
  private:

include/cppgc/internal/pointer-policies.h

@@ -28,7 +28,17 @@ struct DijkstraWriteBarrierPolicy {
     // barrier doesn't break the tri-color invariant.
   }
   static void AssigningBarrier(const void* slot, const void* value) {
-    WriteBarrier::DijkstraMarkingBarrier(slot, value);
+    WriteBarrier::Params params;
+    switch (WriteBarrier::GetWriteBarrierType(slot, value, params)) {
+      case WriteBarrier::Type::kGenerational:
+        WriteBarrier::GenerationalBarrier(params, slot);
+        break;
+      case WriteBarrier::Type::kMarking:
+        WriteBarrier::DijkstraMarkingBarrier(params, value);
+        break;
+      case WriteBarrier::Type::kNone:
+        break;
+    }
   }
 };
 

src/heap/cppgc/write-barrier.cc

@@ -116,7 +116,7 @@ void WriteBarrier::SteeleMarkingBarrierSlow(const void* value) {
 
 #if defined(CPPGC_YOUNG_GENERATION)
 // static
-void WriteBarrier::GenerationalBarrierSlow(CagedHeapLocalData& local_data,
+void WriteBarrier::GenerationalBarrierSlow(const CagedHeapLocalData& local_data,
                                            const AgeTable& age_table,
                                            const void* slot,
                                            uintptr_t value_offset) {
@@ -127,5 +127,12 @@ void WriteBarrier::GenerationalBarrierSlow(CagedHeapLocalData& local_data,
 }
 #endif
 
+#if V8_ENABLE_CHECKS
+// static
+void WriteBarrier::CheckParams(Type expected_type, const Params& params) {
+  CHECK_EQ(expected_type, params.type);
+}
+#endif  // V8_ENABLE_CHECKS
+
 }  // namespace internal
 }  // namespace cppgc

test/unittests/heap/cppgc/minor-gc-unittest.cc

@@ -5,6 +5,7 @@
 #if defined(CPPGC_YOUNG_GENERATION)
 
 #include "include/cppgc/allocation.h"
+#include "include/cppgc/heap-consistency.h"
 #include "include/cppgc/persistent.h"
 #include "src/heap/cppgc/heap-object-header.h"
 #include "src/heap/cppgc/heap.h"
@@ -200,17 +201,13 @@ TYPED_TEST(MinorGCTestForType, OmitGenerationalBarrierForOnStackObject) {
     Type* ptr = nullptr;
   } stack_object;
 
-  auto* new_object = MakeGarbageCollected<Type>(this->GetAllocationHandle());
-
-  const auto& set = Heap::From(this->GetHeap())->remembered_slots();
-  const size_t set_size_before_barrier = set.size();
-
   // Try issuing generational barrier for on-stack object.
-  stack_object.ptr = new_object;
-  WriteBarrier::DijkstraMarkingBarrier(
-      reinterpret_cast<void*>(&stack_object.ptr), new_object);
-
-  EXPECT_EQ(set_size_before_barrier, set.size());
+  stack_object.ptr = MakeGarbageCollected<Type>(this->GetAllocationHandle());
+  subtle::HeapConsistency::WriteBarrierParams params;
+  EXPECT_EQ(subtle::HeapConsistency::WriteBarrierType::kNone,
+            subtle::HeapConsistency::GetWriteBarrierType(
+                reinterpret_cast<void*>(&stack_object.ptr), stack_object.ptr,
+                params));
 }
 
 TYPED_TEST(MinorGCTestForType, OmitGenerationalBarrierForSentinels) {

test/unittests/heap/cppgc/write-barrier-unittest.cc

@@ -324,26 +324,41 @@ TEST_F(WriteBarrierTest, NoWriteBarrierOnMarkedMixinApplication) {
 // Raw barriers. ===============================================================
 // =============================================================================
 
-TEST_F(WriteBarrierTest, DijkstraWriteBarrierTriggersWhenMarkingIsOn) {
+using WriteBarrierParams = subtle::HeapConsistency::WriteBarrierParams;
+using WriteBarrierType = subtle::HeapConsistency::WriteBarrierType;
+using subtle::HeapConsistency;
+
+TEST_F(NoWriteBarrierTest, WriteBarrierBailoutWhenMarkingIsOff) {
   auto* object1 = MakeGarbageCollected<GCed>(GetAllocationHandle());
   auto* object2 = MakeGarbageCollected<GCed>(GetAllocationHandle(), object1);
   {
-    ExpectWriteBarrierFires scope(marker(), {object1});
     EXPECT_FALSE(object1->IsMarked());
-    subtle::HeapConsistency::DijkstraWriteBarrier(
-        object2->next_ref().GetSlotForTesting(), object2->next_ref().Get());
-    EXPECT_TRUE(object1->IsMarked());
+    WriteBarrierParams params;
+#if defined(CPPGC_YOUNG_GENERATION)
+    WriteBarrierType expected = WriteBarrierType::kGenerational;
+#else   // !CPPGC_YOUNG_GENERATION
+    WriteBarrierType expected = WriteBarrierType::kNone;
+#endif  // !CPPGC_YOUNG_GENERATION
+    EXPECT_EQ(expected, HeapConsistency::GetWriteBarrierType(
+                            object2->next_ref().GetSlotForTesting(),
+                            object2->next_ref().Get(), params));
+    EXPECT_FALSE(object1->IsMarked());
   }
 }
 
-TEST_F(NoWriteBarrierTest, DijkstraWriteBarrierBailoutWhenMarkingIsOff) {
+TEST_F(WriteBarrierTest, DijkstraWriteBarrierTriggersWhenMarkingIsOn) {
   auto* object1 = MakeGarbageCollected<GCed>(GetAllocationHandle());
   auto* object2 = MakeGarbageCollected<GCed>(GetAllocationHandle(), object1);
   {
+    ExpectWriteBarrierFires scope(marker(), {object1});
     EXPECT_FALSE(object1->IsMarked());
-    subtle::HeapConsistency::DijkstraWriteBarrier(
-        object2->next_ref().GetSlotForTesting(), object2->next_ref().Get());
-    EXPECT_FALSE(object1->IsMarked());
+    WriteBarrierParams params;
+    EXPECT_EQ(WriteBarrierType::kMarking,
+              HeapConsistency::GetWriteBarrierType(
+                  object2->next_ref().GetSlotForTesting(),
+                  object2->next_ref().Get(), params));
+    HeapConsistency::DijkstraWriteBarrier(params, object2->next_ref().Get());
+    EXPECT_TRUE(object1->IsMarked());
   }
 }
 
@@ -353,8 +368,12 @@ TEST_F(WriteBarrierTest, DijkstraWriteBarrierBailoutIfMarked) {
   EXPECT_TRUE(HeapObjectHeader::FromPayload(object1).TryMarkAtomic());
   {
     ExpectNoWriteBarrierFires scope(marker(), {object1});
-    subtle::HeapConsistency::DijkstraWriteBarrier(
-        object2->next_ref().GetSlotForTesting(), object2->next_ref().Get());
+    WriteBarrierParams params;
+    EXPECT_EQ(WriteBarrierType::kMarking,
+              HeapConsistency::GetWriteBarrierType(
+                  object2->next_ref().GetSlotForTesting(),
+                  object2->next_ref().Get(), params));
+    HeapConsistency::DijkstraWriteBarrier(params, object2->next_ref().Get());
   }
 }
 
@@ -392,28 +411,16 @@ TEST_F(WriteBarrierTest, DijkstraWriteBarrierRangeTriggersWhenMarkingIsOn) {
   {
     ExpectWriteBarrierFires scope(marker(), {object1});
     EXPECT_FALSE(object1->IsMarked());
-    subtle::HeapConsistency::DijkstraWriteBarrierRange(
-        [this]() -> cppgc::HeapHandle& { return GetHeap()->GetHeapHandle(); },
-        object2->objects, sizeof(InlinedObject), 4,
-        TraceTrait<InlinedObject>::Trace);
+    WriteBarrierParams params;
+    EXPECT_EQ(WriteBarrierType::kMarking,
+              HeapConsistency::GetWriteBarrierType(object2->objects, params));
+    HeapConsistency::DijkstraWriteBarrierRange(
+        params, GetHeap()->GetHeapHandle(), object2->objects,
+        sizeof(InlinedObject), 4, TraceTrait<InlinedObject>::Trace);
     EXPECT_TRUE(object1->IsMarked());
   }
 }
 
-TEST_F(NoWriteBarrierTest, DijkstraWriteBarrierRangeBailoutWhenMarkingIsOff) {
-  auto* object1 = MakeGarbageCollected<GCed>(GetAllocationHandle());
-  auto* object2 = MakeGarbageCollected<GCedWithInlinedArray>(
-      GetAllocationHandle(), object1);
-  {
-    EXPECT_FALSE(object1->IsMarked());
-    subtle::HeapConsistency::DijkstraWriteBarrierRange(
-        [this]() -> cppgc::HeapHandle& { return GetHeap()->GetHeapHandle(); },
-        object2->objects, sizeof(InlinedObject), 4,
-        TraceTrait<InlinedObject>::Trace);
-    EXPECT_FALSE(object1->IsMarked());
-  }
-}
-
 TEST_F(WriteBarrierTest, DijkstraWriteBarrierRangeBailoutIfMarked) {
   auto* object1 = MakeGarbageCollected<GCed>(GetAllocationHandle());
   auto* object2 = MakeGarbageCollected<GCedWithInlinedArray>(
@@ -421,10 +428,12 @@ TEST_F(WriteBarrierTest, DijkstraWriteBarrierRangeBailoutIfMarked) {
   EXPECT_TRUE(HeapObjectHeader::FromPayload(object1).TryMarkAtomic());
   {
     ExpectNoWriteBarrierFires scope(marker(), {object1});
-    subtle::HeapConsistency::DijkstraWriteBarrierRange(
-        [this]() -> cppgc::HeapHandle& { return GetHeap()->GetHeapHandle(); },
-        object2->objects, sizeof(InlinedObject), 4,
-        TraceTrait<InlinedObject>::Trace);
+    WriteBarrierParams params;
+    EXPECT_EQ(WriteBarrierType::kMarking,
+              HeapConsistency::GetWriteBarrierType(object2->objects, params));
+    HeapConsistency::DijkstraWriteBarrierRange(
+        params, GetHeap()->GetHeapHandle(), object2->objects,
+        sizeof(InlinedObject), 4, TraceTrait<InlinedObject>::Trace);
   }
 }
 
@@ -434,8 +443,11 @@ TEST_F(WriteBarrierTest, SteeleWriteBarrierTriggersWhenMarkingIsOn) {
   {
     ExpectWriteBarrierFires scope(marker(), {object1});
     EXPECT_TRUE(HeapObjectHeader::FromPayload(object1).TryMarkAtomic());
-    // Steele barrier puts the object on the worklist for rescanning.
-    subtle::HeapConsistency::SteeleWriteBarrier(object2->next_ref().Get());
+    WriteBarrierParams params;
+    EXPECT_EQ(WriteBarrierType::kMarking,
+              HeapConsistency::GetWriteBarrierType(object2->next_ref().Get(),
+                                                   params));
+    HeapConsistency::SteeleWriteBarrier(params, object2->next_ref().Get());
   }
 }
 
@@ -444,7 +456,11 @@ TEST_F(WriteBarrierTest, SteeleWriteBarrierBailoutIfNotMarked) {
   auto* object2 = MakeGarbageCollected<GCed>(GetAllocationHandle(), object1);
   {
     ExpectNoWriteBarrierFires scope(marker(), {object1});
-    subtle::HeapConsistency::SteeleWriteBarrier(object2->next_ref().Get());
+    WriteBarrierParams params;
+    EXPECT_EQ(WriteBarrierType::kMarking,
+              HeapConsistency::GetWriteBarrierType(object2->next_ref().Get(),
+                                                   params));
+    HeapConsistency::SteeleWriteBarrier(params, object2->next_ref().Get());
   }
 }