[heap] Combine write barrier flag checks

Adding the shared heap write barrier caused regressions on some
benchmarks. Presumably this is because the compiler can't merge the
fast paths of the generational and shared heap write barrier.

This CL therefore introduces a CombinedHeapBarrier that manually
unifies the fast path for the marking, generational and shared heap
write barrier. This should make the barrier easier to optimize for
the compiler. In particular it should help to ensure that page flags
don't need to be loaded multiple times in a single full write barrier.

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

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

@@ -41,7 +41,7 @@ V8_EXPORT_PRIVATE void Heap_SharedHeapBarrierForCodeSlow(Code host,
                                                          HeapObject object);
 
 V8_EXPORT_PRIVATE void Heap_GenerationalEphemeronKeyBarrierSlow(
-    Heap* heap, EphemeronHashTable table, Address slot);
+    Heap* heap, HeapObject table, Address slot);
 
 // Do not use these internal details anywhere outside of this file. These
 // internals are only intended to shortcut write barrier checks.
@@ -107,37 +107,39 @@ inline void GenerationalBarrierInternal(HeapObject object, Address slot,
   Heap_GenerationalBarrierSlow(object, slot, value);
 }
 
-inline void SharedHeapBarrierInternal(HeapObject object, Address slot,
-                                      HeapObject value) {
-  DCHECK(Heap_PageFlagsAreConsistent(object));
-  heap_internals::MemoryChunk* value_chunk =
-      heap_internals::MemoryChunk::FromHeapObject(value);
-  heap_internals::MemoryChunk* object_chunk =
-      heap_internals::MemoryChunk::FromHeapObject(object);
-
-  // We only care about pointers into the shared heap.
-  if (!value_chunk->InSharedHeap()) return;
+inline void CombinedWriteBarrierInternal(HeapObject host, HeapObjectSlot slot,
+                                         HeapObject value,
+                                         WriteBarrierMode mode) {
+  DCHECK(mode == UPDATE_WRITE_BARRIER || mode == UPDATE_WEAK_WRITE_BARRIER);
 
-  // Do not record slots in new space or the shared heap itself.
-  if (object_chunk->InYoungGeneration() || object_chunk->InSharedHeap()) return;
+  heap_internals::MemoryChunk* host_chunk =
+      heap_internals::MemoryChunk::FromHeapObject(host);
 
-  Heap_SharedHeapBarrierSlow(object, slot, value);
-}
-
-inline void GenerationalEphemeronKeyBarrierInternal(EphemeronHashTable table,
-                                                    Address slot,
-                                                    HeapObject value) {
-  DCHECK(Heap::PageFlagsAreConsistent(table));
   heap_internals::MemoryChunk* value_chunk =
       heap_internals::MemoryChunk::FromHeapObject(value);
-  heap_internals::MemoryChunk* table_chunk =
-      heap_internals::MemoryChunk::FromHeapObject(table);
 
-  if (!value_chunk->InYoungGeneration() || table_chunk->InYoungGeneration()) {
-    return;
+  const bool host_in_young_gen = host_chunk->InYoungGeneration();
+  const bool host_in_shared = host_chunk->InSharedHeap();
+  const bool is_marking = host_chunk->IsMarking();
+
+  if (!host_in_young_gen) {
+    if (value_chunk->InYoungGeneration()) {
+      // Generational write barrier (old-to-new).
+      Heap_GenerationalBarrierSlow(host, slot.address(), value);
+
+    } else if (value_chunk->InSharedHeap()) {
+      // Shared heap write barrier (old-to-shared).
+      if (!host_in_shared) {
+        Heap_SharedHeapBarrierSlow(host, slot.address(), value);
+      }
+    }
   }
 
-  Heap_GenerationalEphemeronKeyBarrierSlow(table_chunk->GetHeap(), table, slot);
+  // Marking barrier: mark value & record slots when marking is on.
+  if (mode == UPDATE_WRITE_BARRIER && is_marking) {
+    WriteBarrier::MarkingSlow(host_chunk->GetHeap(), host, HeapObjectSlot(slot),
+                              value);
+  }
 }
 
 }  // namespace heap_internals
@@ -158,35 +160,72 @@ inline void WriteBarrierForCode(Code host) {
   Heap_WriteBarrierForCodeSlow(host);
 }
 
-inline void GenerationalBarrier(HeapObject object, ObjectSlot slot,
-                                Object value) {
-  if (V8_ENABLE_THIRD_PARTY_HEAP_BOOL) return;
-  DCHECK(!HasWeakHeapObjectTag(value));
+inline void CombinedWriteBarrier(HeapObject host, ObjectSlot slot, Object value,
+                                 WriteBarrierMode mode) {
+  if (mode == SKIP_WRITE_BARRIER) {
+    SLOW_DCHECK(!WriteBarrier::IsRequired(host, value));
+    return;
+  }
+
   if (!value.IsHeapObject()) return;
-  GenerationalBarrier(object, slot, HeapObject::cast(value));
+  heap_internals::CombinedWriteBarrierInternal(host, HeapObjectSlot(slot),
+                                               HeapObject::cast(value), mode);
 }
 
-inline void GenerationalBarrier(HeapObject object, ObjectSlot slot,
-                                Code value) {
-  if (V8_ENABLE_THIRD_PARTY_HEAP_BOOL) return;
-  DCHECK(!Heap_ValueMightRequireGenerationalWriteBarrier(value));
-}
+inline void CombinedWriteBarrier(HeapObject host, MaybeObjectSlot slot,
+                                 MaybeObject value, WriteBarrierMode mode) {
+  if (mode == SKIP_WRITE_BARRIER) {
+    SLOW_DCHECK(!WriteBarrier::IsRequired(host, value));
+    return;
+  }
 
-inline void GenerationalBarrier(HeapObject object, ObjectSlot slot,
-                                HeapObject value) {
-  if (V8_ENABLE_THIRD_PARTY_HEAP_BOOL) return;
-  DCHECK(!HasWeakHeapObjectTag(*slot));
-  heap_internals::GenerationalBarrierInternal(object, slot.address(), value);
+  HeapObject value_object;
+  if (!value->GetHeapObject(&value_object)) return;
+  heap_internals::CombinedWriteBarrierInternal(host, HeapObjectSlot(slot),
+                                               value_object, mode);
 }
 
-inline void GenerationalEphemeronKeyBarrier(EphemeronHashTable table,
-                                            ObjectSlot slot, Object value) {
-  if (V8_ENABLE_THIRD_PARTY_HEAP_BOOL) return;
-  DCHECK(!HasWeakHeapObjectTag(*slot));
-  DCHECK(!HasWeakHeapObjectTag(value));
-  DCHECK(value.IsHeapObject());
-  heap_internals::GenerationalEphemeronKeyBarrierInternal(
-      table, slot.address(), HeapObject::cast(value));
+inline void CombinedEphemeronWriteBarrier(EphemeronHashTable host,
+                                          ObjectSlot slot, Object value,
+                                          WriteBarrierMode mode) {
+  if (mode == SKIP_WRITE_BARRIER) {
+    SLOW_DCHECK(!WriteBarrier::IsRequired(host, value));
+    return;
+  }
+
+  DCHECK_EQ(mode, UPDATE_WRITE_BARRIER);
+  if (!value.IsHeapObject()) return;
+
+  heap_internals::MemoryChunk* host_chunk =
+      heap_internals::MemoryChunk::FromHeapObject(host);
+
+  HeapObject heap_object_value = HeapObject::cast(value);
+  heap_internals::MemoryChunk* value_chunk =
+      heap_internals::MemoryChunk::FromHeapObject(heap_object_value);
+
+  const bool host_in_young_gen = host_chunk->InYoungGeneration();
+  const bool host_in_shared = host_chunk->InSharedHeap();
+  const bool is_marking = host_chunk->IsMarking();
+
+  if (!host_in_young_gen) {
+    if (value_chunk->InYoungGeneration()) {
+      // Generational write barrier (old-to-new).
+      Heap_GenerationalEphemeronKeyBarrierSlow(host_chunk->GetHeap(), host,
+                                               slot.address());
+
+    } else if (value_chunk->InSharedHeap()) {
+      // Shared heap write barrier (old-to-shared).
+      if (!host_in_shared) {
+        Heap_SharedHeapBarrierSlow(host, slot.address(), heap_object_value);
+      }
+    }
+  }
+
+  // Marking barrier: mark value & record slots when marking is on.
+  if (is_marking) {
+    WriteBarrier::MarkingSlow(host_chunk->GetHeap(), host, HeapObjectSlot(slot),
+                              heap_object_value);
+  }
 }
 
 inline void GenerationalBarrier(HeapObject object, MaybeObjectSlot slot,
@@ -207,23 +246,6 @@ inline void GenerationalBarrierForCode(Code host, RelocInfo* rinfo,
   Heap_GenerationalBarrierForCodeSlow(host, rinfo, object);
 }
 
-inline void SharedHeapBarrier(HeapObject object, ObjectSlot slot,
-                              Object value) {
-  if (V8_ENABLE_THIRD_PARTY_HEAP_BOOL) return;
-  HeapObject value_heap_object;
-  if (!value.GetHeapObject(&value_heap_object)) return;
-  heap_internals::SharedHeapBarrierInternal(object, slot.address(),
-                                            value_heap_object);
-}
-inline void SharedHeapBarrier(HeapObject object, MaybeObjectSlot slot,
-                              MaybeObject value) {
-  if (V8_ENABLE_THIRD_PARTY_HEAP_BOOL) return;
-  HeapObject value_heap_object;
-  if (!value->GetHeapObject(&value_heap_object)) return;
-  heap_internals::SharedHeapBarrierInternal(object, slot.address(),
-                                            value_heap_object);
-}
-
 inline void SharedHeapBarrierForCode(Code host, RelocInfo* rinfo,
                                      HeapObject object) {
   if (V8_ENABLE_THIRD_PARTY_HEAP_BOOL) return;

src/heap/heap-write-barrier.h

@@ -33,20 +33,20 @@ void WriteBarrierForCode(Code host, RelocInfo* rinfo, Object value);
 void WriteBarrierForCode(Code host, RelocInfo* rinfo, HeapObject value);
 void WriteBarrierForCode(Code host);
 
+void CombinedWriteBarrier(HeapObject object, ObjectSlot slot, Object value,
+                          WriteBarrierMode mode);
+void CombinedWriteBarrier(HeapObject object, MaybeObjectSlot slot,
+                          MaybeObject value, WriteBarrierMode mode);
+
+void CombinedEphemeronWriteBarrier(EphemeronHashTable object, ObjectSlot slot,
+                                   Object value, WriteBarrierMode mode);
+
 // Generational write barrier.
-void GenerationalBarrier(HeapObject object, ObjectSlot slot, Object value);
-void GenerationalBarrier(HeapObject object, ObjectSlot slot, Code value);
-void GenerationalBarrier(HeapObject object, ObjectSlot slot, HeapObject value);
 void GenerationalBarrier(HeapObject object, MaybeObjectSlot slot,
                          MaybeObject value);
-void GenerationalEphemeronKeyBarrier(EphemeronHashTable table, ObjectSlot slot,
-                                     Object value);
 void GenerationalBarrierForCode(Code host, RelocInfo* rinfo, HeapObject object);
 
 // Shared heap write barrier.
-void SharedHeapBarrier(HeapObject object, ObjectSlot slot, Object value);
-void SharedHeapBarrier(HeapObject object, MaybeObjectSlot slot,
-                       MaybeObject value);
 void SharedHeapBarrierForCode(Code host, RelocInfo* rinfo, HeapObject object);
 
 inline bool IsReadOnlyHeapObject(HeapObject object);
@@ -61,6 +61,7 @@ class V8_EXPORT_PRIVATE WriteBarrier {
   static inline void Marking(Code host, RelocInfo*, HeapObject value);
   static inline void Marking(JSArrayBuffer host, ArrayBufferExtension*);
   static inline void Marking(DescriptorArray, int number_of_own_descriptors);
+
   // It is invoked from generated code and has to take raw addresses.
   static int MarkingFromCode(Address raw_host, Address raw_slot);
   // Invoked from global handles where no host object is available.
@@ -78,11 +79,12 @@ class V8_EXPORT_PRIVATE WriteBarrier {
   static bool IsImmortalImmovableHeapObject(HeapObject object);
 #endif
 
+  static void MarkingSlow(Heap* heap, HeapObject host, HeapObjectSlot,
+                          HeapObject value);
+
  private:
   static inline base::Optional<Heap*> GetHeapIfMarking(HeapObject object);
 
-  static void MarkingSlow(Heap* heap, HeapObject host, HeapObjectSlot,
-                          HeapObject value);
   static void MarkingSlow(Heap* heap, Code host, RelocInfo*, HeapObject value);
   static void MarkingSlow(Heap* heap, JSArrayBuffer host,
                           ArrayBufferExtension*);

src/heap/heap.cc

@@ -90,6 +90,7 @@
 #include "src/objects/feedback-vector.h"
 #include "src/objects/free-space-inl.h"
 #include "src/objects/hash-table-inl.h"
+#include "src/objects/hash-table.h"
 #include "src/objects/instance-type.h"
 #include "src/objects/maybe-object.h"
 #include "src/objects/shared-function-info.h"
@@ -163,9 +164,9 @@ void Heap_SharedHeapBarrierForCodeSlow(Code host, RelocInfo* rinfo,
   Heap::SharedHeapBarrierForCodeSlow(host, rinfo, object);
 }
 
-void Heap_GenerationalEphemeronKeyBarrierSlow(Heap* heap,
-                                              EphemeronHashTable table,
+void Heap_GenerationalEphemeronKeyBarrierSlow(Heap* heap, HeapObject host,
                                               Address slot) {
+  EphemeronHashTable table = EphemeronHashTable::cast(host);
   heap->RecordEphemeronKeyWrite(table, slot);
 }
 
@@ -7321,14 +7322,9 @@ void Heap::EphemeronKeyWriteBarrierFromCode(Address raw_object,
                                             Address key_slot_address,
                                             Isolate* isolate) {
   EphemeronHashTable table = EphemeronHashTable::cast(Object(raw_object));
-  MaybeObjectSlot key_slot(key_slot_address);
-  MaybeObject maybe_key = *key_slot;
-  HeapObject key;
-  if (!maybe_key.GetHeapObject(&key)) return;
-  if (!ObjectInYoungGeneration(table) && ObjectInYoungGeneration(key)) {
-    isolate->heap()->RecordEphemeronKeyWrite(table, key_slot_address);
-  }
-  WriteBarrier::Marking(table, key_slot, maybe_key);
+  ObjectSlot key_slot(key_slot_address);
+  CombinedEphemeronWriteBarrier(table, key_slot, *key_slot,
+                                UPDATE_WRITE_BARRIER);
 }
 
 enum RangeWriteBarrierMode {

src/objects/object-macros.h

@@ -449,24 +449,22 @@
 #ifdef V8_DISABLE_WRITE_BARRIERS
 #define WRITE_BARRIER(object, offset, value)
 #else
-#define WRITE_BARRIER(object, offset, value)                         \
-  do {                                                               \
-    DCHECK_NOT_NULL(GetHeapFromWritableObject(object));              \
-    WriteBarrier::Marking(object, (object).RawField(offset), value); \
-    GenerationalBarrier(object, (object).RawField(offset), value);   \
-    SharedHeapBarrier(object, (object).RawField(offset), value);     \
+#define WRITE_BARRIER(object, offset, value)                       \
+  do {                                                             \
+    DCHECK_NOT_NULL(GetHeapFromWritableObject(object));            \
+    CombinedWriteBarrier(object, (object).RawField(offset), value, \
+                         UPDATE_WRITE_BARRIER);                    \
   } while (false)
 #endif
 
 #ifdef V8_DISABLE_WRITE_BARRIERS
 #define WEAK_WRITE_BARRIER(object, offset, value)
 #else
-#define WEAK_WRITE_BARRIER(object, offset, value)                             \
-  do {                                                                        \
-    DCHECK_NOT_NULL(GetHeapFromWritableObject(object));                       \
-    WriteBarrier::Marking(object, (object).RawMaybeWeakField(offset), value); \
-    GenerationalBarrier(object, (object).RawMaybeWeakField(offset), value);   \
-    SharedHeapBarrier(object, (object).RawMaybeWeakField(offset), value);     \
+#define WEAK_WRITE_BARRIER(object, offset, value)                           \
+  do {                                                                      \
+    DCHECK_NOT_NULL(GetHeapFromWritableObject(object));                     \
+    CombinedWriteBarrier(object, (object).RawMaybeWeakField(offset), value, \
+                         UPDATE_WRITE_BARRIER);                             \
   } while (false)
 #endif
 
@@ -476,13 +474,12 @@
 #define EPHEMERON_KEY_WRITE_BARRIER(object, offset, value) \
   WRITE_BARRIER(object, offset, value)
 #else
-#define EPHEMERON_KEY_WRITE_BARRIER(object, offset, value)                    \
-  do {                                                                        \
-    DCHECK_NOT_NULL(GetHeapFromWritableObject(object));                       \
-    EphemeronHashTable table = EphemeronHashTable::cast(object);              \
-    WriteBarrier::Marking(object, (object).RawField(offset), value);          \
-    GenerationalEphemeronKeyBarrier(table, (object).RawField(offset), value); \
-    SharedHeapBarrier(object, (object).RawField(offset), value);              \
+#define EPHEMERON_KEY_WRITE_BARRIER(object, offset, value)          \
+  do {                                                              \
+    DCHECK_NOT_NULL(GetHeapFromWritableObject(object));             \
+    CombinedEphemeronWriteBarrier(EphemeronHashTable::cast(object), \
+                                  (object).RawField(offset), value, \
+                                  UPDATE_WRITE_BARRIER);            \
   } while (false)
 #endif
 
@@ -492,19 +489,10 @@
 #define CONDITIONAL_WRITE_BARRIER(object, offset, value, mode) \
   WRITE_BARRIER(object, offset, value)
 #else
-#define CONDITIONAL_WRITE_BARRIER(object, offset, value, mode)           \
-  do {                                                                   \
-    DCHECK_NOT_NULL(GetHeapFromWritableObject(object));                  \
-    DCHECK_NE(mode, UPDATE_EPHEMERON_KEY_WRITE_BARRIER);                 \
-    if (mode != SKIP_WRITE_BARRIER) {                                    \
-      if (mode == UPDATE_WRITE_BARRIER) {                                \
-        WriteBarrier::Marking(object, (object).RawField(offset), value); \
-      }                                                                  \
-      GenerationalBarrier(object, (object).RawField(offset), value);     \
-      SharedHeapBarrier(object, (object).RawField(offset), value);       \
-    } else {                                                             \
-      SLOW_DCHECK(!WriteBarrier::IsRequired(object, value));             \
-    }                                                                    \
+#define CONDITIONAL_WRITE_BARRIER(object, offset, value, mode)            \
+  do {                                                                    \
+    DCHECK_NOT_NULL(GetHeapFromWritableObject(object));                   \
+    CombinedWriteBarrier(object, (object).RawField(offset), value, mode); \
   } while (false)
 #endif
 
@@ -514,20 +502,11 @@
 #define CONDITIONAL_WEAK_WRITE_BARRIER(object, offset, value, mode) \
   WRITE_BARRIER(object, offset, value)
 #else
-#define CONDITIONAL_WEAK_WRITE_BARRIER(object, offset, value, mode)           \
-  do {                                                                        \
-    DCHECK_NOT_NULL(GetHeapFromWritableObject(object));                       \
-    DCHECK_NE(mode, UPDATE_EPHEMERON_KEY_WRITE_BARRIER);                      \
-    if (mode != SKIP_WRITE_BARRIER) {                                         \
-      if (mode == UPDATE_WRITE_BARRIER) {                                     \
-        WriteBarrier::Marking(object, (object).RawMaybeWeakField(offset),     \
-                              value);                                         \
-      }                                                                       \
-      GenerationalBarrier(object, (object).RawMaybeWeakField(offset), value); \
-      SharedHeapBarrier(object, (object).RawMaybeWeakField(offset), value);   \
-    } else {                                                                  \
-      SLOW_DCHECK(!WriteBarrier::IsRequired(object, value));                  \
-    }                                                                         \
+#define CONDITIONAL_WEAK_WRITE_BARRIER(object, offset, value, mode)         \
+  do {                                                                      \
+    DCHECK_NOT_NULL(GetHeapFromWritableObject(object));                     \
+    CombinedWriteBarrier(object, (object).RawMaybeWeakField(offset), value, \
+                         mode);                                             \
   } while (false)
 #endif
 
@@ -537,18 +516,8 @@
 #define CONDITIONAL_EPHEMERON_KEY_WRITE_BARRIER(object, offset, value, mode) \
   do {                                                                       \
     DCHECK_NOT_NULL(GetHeapFromWritableObject(object));                      \
-    DCHECK_NE(mode, UPDATE_EPHEMERON_KEY_WRITE_BARRIER);                     \
-    EphemeronHashTable table = EphemeronHashTable::cast(object);             \
-    if (mode != SKIP_WRITE_BARRIER) {                                        \
-      if (mode == UPDATE_WRITE_BARRIER) {                                    \
-        WriteBarrier::Marking(object, (object).RawField(offset), value);     \
-      }                                                                      \
-      GenerationalEphemeronKeyBarrier(table, (object).RawField(offset),      \
-                                      value);                                \
-      SharedHeapBarrier(table, (object).RawField(offset), value);            \
-    } else {                                                                 \
-      SLOW_DCHECK(!WriteBarrier::IsRequired(object, value));                 \
-    }                                                                        \
+    CombinedEphemeronWriteBarrier(EphemeronHashTable::cast(object),          \
+                                  (object).RawField(offset), value, mode);   \
   } while (false)
 #endif