cppgc: Atomic object start bitmap

This CL ports the atomic object start bitmap from blink.
Using the bitmap for mixin tracing is left as a followup.

Bug: chromium:1056170
Change-Id: I4998a0d9d76708a7bab0634e04354809dfc8c78f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2287504Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Anton Bikineev <bikineev@chromium.org>
Commit-Queue: Omer Katz <omerkatz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#68778}

src/heap/cppgc/heap-page.cc

@@ -31,7 +31,7 @@ const HeapObjectHeader* ObjectHeaderFromInnerAddressImpl(const BasePage* page,
   if (page->is_large()) {
     return LargePage::From(page)->ObjectHeader();
   }
-  const ObjectStartBitmap& bitmap =
+  const PlatformAwareObjectStartBitmap& bitmap =
       NormalPage::From(page)->object_start_bitmap();
   const HeapObjectHeader* header =
       bitmap.FindHeader(static_cast<ConstAddress>(address));

src/heap/cppgc/heap-page.h

@@ -152,8 +152,10 @@ class V8_EXPORT_PRIVATE NormalPage final : public BasePage {
     return (PayloadStart() <= address) && (address < PayloadEnd());
   }
 
-  ObjectStartBitmap& object_start_bitmap() { return object_start_bitmap_; }
-  const ObjectStartBitmap& object_start_bitmap() const {
+  PlatformAwareObjectStartBitmap& object_start_bitmap() {
+    return object_start_bitmap_;
+  }
+  const PlatformAwareObjectStartBitmap& object_start_bitmap() const {
     return object_start_bitmap_;
   }
 
@@ -161,7 +163,7 @@ class V8_EXPORT_PRIVATE NormalPage final : public BasePage {
   NormalPage(HeapBase* heap, BaseSpace* space);
   ~NormalPage();
 
-  ObjectStartBitmap object_start_bitmap_;
+  PlatformAwareObjectStartBitmap object_start_bitmap_;
 };
 
 class V8_EXPORT_PRIVATE LargePage final : public BasePage {

src/heap/cppgc/object-start-bitmap.h

@@ -10,21 +10,25 @@
 
 #include <array>
 
+#include "include/cppgc/internal/process-heap.h"
+#include "src/base/atomic-utils.h"
 #include "src/base/bits.h"
 #include "src/base/macros.h"
 #include "src/heap/cppgc/globals.h"
+#include "src/heap/cppgc/heap-object-header.h"
 
 namespace cppgc {
 namespace internal {
 
-class HeapObjectHeader;
-
 // A bitmap for recording object starts. Objects have to be allocated at
 // minimum granularity of kGranularity.
 //
 // Depends on internals such as:
 // - kBlinkPageSize
 // - kAllocationGranularity
+//
+// ObjectStartBitmap supports concurrent reads from multiple threads but
+// only a single mutator thread can write to it.
 class V8_EXPORT_PRIVATE ObjectStartBitmap {
  public:
   // Granularity of addresses added to the bitmap.
@@ -40,11 +44,19 @@ class V8_EXPORT_PRIVATE ObjectStartBitmap {
   // Finds an object header based on a
   // address_maybe_pointing_to_the_middle_of_object. Will search for an object
   // start in decreasing address order.
+  template <
+      HeapObjectHeader::AccessMode = HeapObjectHeader::AccessMode::kNonAtomic>
   inline HeapObjectHeader* FindHeader(
       ConstAddress address_maybe_pointing_to_the_middle_of_object) const;
 
+  template <
+      HeapObjectHeader::AccessMode = HeapObjectHeader::AccessMode::kNonAtomic>
   inline void SetBit(ConstAddress);
+  template <
+      HeapObjectHeader::AccessMode = HeapObjectHeader::AccessMode::kNonAtomic>
   inline void ClearBit(ConstAddress);
+  template <
+      HeapObjectHeader::AccessMode = HeapObjectHeader::AccessMode::kNonAtomic>
   inline bool CheckBit(ConstAddress) const;
 
   // Iterates all object starts recorded in the bitmap.
@@ -59,6 +71,13 @@ class V8_EXPORT_PRIVATE ObjectStartBitmap {
   inline void Clear();
 
  private:
+  template <
+      HeapObjectHeader::AccessMode = HeapObjectHeader::AccessMode::kNonAtomic>
+  inline void store(size_t cell_index, uint8_t value);
+  template <
+      HeapObjectHeader::AccessMode = HeapObjectHeader::AccessMode::kNonAtomic>
+  inline uint8_t load(size_t cell_index) const;
+
   static constexpr size_t kBitsPerCell = sizeof(uint8_t) * CHAR_BIT;
   static constexpr size_t kCellMask = kBitsPerCell - 1;
   static constexpr size_t kBitmapSize =
@@ -79,6 +98,7 @@ ObjectStartBitmap::ObjectStartBitmap(Address offset) : offset_(offset) {
   Clear();
 }
 
+template <HeapObjectHeader::AccessMode mode>
 HeapObjectHeader* ObjectStartBitmap::FindHeader(
     ConstAddress address_maybe_pointing_to_the_middle_of_object) const {
   DCHECK_LE(offset_, address_maybe_pointing_to_the_middle_of_object);
@@ -88,10 +108,10 @@ HeapObjectHeader* ObjectStartBitmap::FindHeader(
   size_t cell_index = object_start_number / kBitsPerCell;
   DCHECK_GT(object_start_bit_map_.size(), cell_index);
   const size_t bit = object_start_number & kCellMask;
-  uint8_t byte = object_start_bit_map_[cell_index] & ((1 << (bit + 1)) - 1);
+  uint8_t byte = load<mode>(cell_index) & ((1 << (bit + 1)) - 1);
   while (!byte && cell_index) {
     DCHECK_LT(0u, cell_index);
-    byte = object_start_bit_map_[--cell_index];
+    byte = load<mode>(--cell_index);
   }
   const int leading_zeroes = v8::base::bits::CountLeadingZeros(byte);
   object_start_number =
@@ -100,22 +120,47 @@ HeapObjectHeader* ObjectStartBitmap::FindHeader(
   return reinterpret_cast<HeapObjectHeader*>(object_offset + offset_);
 }
 
+template <HeapObjectHeader::AccessMode mode>
 void ObjectStartBitmap::SetBit(ConstAddress header_address) {
   size_t cell_index, object_bit;
   ObjectStartIndexAndBit(header_address, &cell_index, &object_bit);
-  object_start_bit_map_[cell_index] |= (1 << object_bit);
+  // Only a single mutator thread can write to the bitmap, so no need for CAS.
+  store<mode>(cell_index,
+              static_cast<uint8_t>(load(cell_index) | (1 << object_bit)));
 }
 
+template <HeapObjectHeader::AccessMode mode>
 void ObjectStartBitmap::ClearBit(ConstAddress header_address) {
   size_t cell_index, object_bit;
   ObjectStartIndexAndBit(header_address, &cell_index, &object_bit);
-  object_start_bit_map_[cell_index] &= ~(1 << object_bit);
+  store<mode>(cell_index,
+              static_cast<uint8_t>(load(cell_index) & ~(1 << object_bit)));
 }
 
+template <HeapObjectHeader::AccessMode mode>
 bool ObjectStartBitmap::CheckBit(ConstAddress header_address) const {
   size_t cell_index, object_bit;
   ObjectStartIndexAndBit(header_address, &cell_index, &object_bit);
-  return object_start_bit_map_[cell_index] & (1 << object_bit);
+  return load<mode>(cell_index) & (1 << object_bit);
+}
+
+template <HeapObjectHeader::AccessMode mode>
+void ObjectStartBitmap::store(size_t cell_index, uint8_t value) {
+  if (mode == HeapObjectHeader::AccessMode::kNonAtomic) {
+    object_start_bit_map_[cell_index] = value;
+    return;
+  }
+  v8::base::AsAtomicPtr(&object_start_bit_map_[cell_index])
+      ->store(value, std::memory_order_release);
+}
+
+template <HeapObjectHeader::AccessMode mode>
+uint8_t ObjectStartBitmap::load(size_t cell_index) const {
+  if (mode == HeapObjectHeader::AccessMode::kNonAtomic) {
+    return object_start_bit_map_[cell_index];
+  }
+  return v8::base::AsAtomicPtr(&object_start_bit_map_[cell_index])
+      ->load(std::memory_order_acquire);
 }
 
 void ObjectStartBitmap::ObjectStartIndexAndBit(ConstAddress header_address,
@@ -152,6 +197,61 @@ void ObjectStartBitmap::Clear() {
   std::fill(object_start_bit_map_.begin(), object_start_bit_map_.end(), 0);
 }
 
+// A platform aware version of ObjectStartBitmap to provide platform specific
+// optimizations (e.g. Use non-atomic stores on ARMv7 when not marking).
+class V8_EXPORT_PRIVATE PlatformAwareObjectStartBitmap
+    : public ObjectStartBitmap {
+ public:
+  explicit inline PlatformAwareObjectStartBitmap(Address offset);
+
+  template <
+      HeapObjectHeader::AccessMode = HeapObjectHeader::AccessMode::kNonAtomic>
+  inline void SetBit(ConstAddress);
+  template <
+      HeapObjectHeader::AccessMode = HeapObjectHeader::AccessMode::kNonAtomic>
+  inline void ClearBit(ConstAddress);
+
+ private:
+  template <HeapObjectHeader::AccessMode>
+  static bool ShouldForceNonAtomic();
+};
+
+PlatformAwareObjectStartBitmap::PlatformAwareObjectStartBitmap(Address offset)
+    : ObjectStartBitmap(offset) {}
+
+// static
+template <HeapObjectHeader::AccessMode mode>
+bool PlatformAwareObjectStartBitmap::ShouldForceNonAtomic() {
+#if defined(V8_TARGET_ARCH_ARM)
+  // Use non-atomic accesses on ARMv7 when marking is not active.
+  if (mode == HeapObjectHeader::AccessMode::kAtomic) {
+    if (V8_LIKELY(!ProcessHeap::IsAnyIncrementalOrConcurrentMarking()))
+      return true;
+  }
+#endif  // defined(V8_TARGET_ARCH_ARM)
+  return false;
+}
+
+template <HeapObjectHeader::AccessMode mode>
+void PlatformAwareObjectStartBitmap::SetBit(ConstAddress header_address) {
+  if (ShouldForceNonAtomic<mode>()) {
+    ObjectStartBitmap::SetBit<HeapObjectHeader::AccessMode::kNonAtomic>(
+        header_address);
+    return;
+  }
+  ObjectStartBitmap::SetBit<mode>(header_address);
+}
+
+template <HeapObjectHeader::AccessMode mode>
+void PlatformAwareObjectStartBitmap::ClearBit(ConstAddress header_address) {
+  if (ShouldForceNonAtomic<mode>()) {
+    ObjectStartBitmap::ClearBit<HeapObjectHeader::AccessMode::kNonAtomic>(
+        header_address);
+    return;
+  }
+  ObjectStartBitmap::ClearBit<mode>(header_address);
+}
+
 }  // namespace internal
 }  // namespace cppgc
 

src/heap/cppgc/sweeper.cc

@@ -57,7 +57,7 @@ class ObjectStartBitmapVerifier
     return true;
   }
 
-  ObjectStartBitmap* bitmap_ = nullptr;
+  PlatformAwareObjectStartBitmap* bitmap_ = nullptr;
   HeapObjectHeader* prev_ = nullptr;
 };
 
@@ -181,7 +181,7 @@ typename FinalizationBuilder::ResultType SweepNormalPage(NormalPage* page) {
   constexpr auto kAtomicAccess = HeapObjectHeader::AccessMode::kAtomic;
   FinalizationBuilder builder(page);
 
-  ObjectStartBitmap& bitmap = page->object_start_bitmap();
+  PlatformAwareObjectStartBitmap& bitmap = page->object_start_bitmap();
   bitmap.Clear();
 
   Address start_of_gap = page->PayloadStart();