cppgc: Add support for young generation only marking verification

Marking verification should only process young generation objects when
verifying the marking state of a young generation garbage collection.

Bug: v8:12324
Change-Id: I01db261437ec5c42ddb6c79c44e31b5fe0e536d7
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3231343Reviewed-by: Omer Katz <omerkatz@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#77518}

src/heap/cppgc-js/cpp-heap.cc

@@ -504,13 +504,18 @@ void CppHeap::TraceEpilogue(TraceSummary* trace_summary) {
   buffered_allocated_bytes_ = 0;
   const size_t bytes_allocated_in_prefinalizers = ExecutePreFinalizers();
 #if CPPGC_VERIFY_HEAP
-  UnifiedHeapMarkingVerifier verifier(*this);
+  UnifiedHeapMarkingVerifier verifier(
+      *this, cppgc::internal::Heap::Config::CollectionType::kMajor);
   verifier.Run(
       stack_state_of_prev_gc(), stack_end_of_current_gc(),
       stats_collector()->marked_bytes() + bytes_allocated_in_prefinalizers);
 #endif  // CPPGC_VERIFY_HEAP
   USE(bytes_allocated_in_prefinalizers);
 
+#if defined(CPPGC_YOUNG_GENERATION)
+  ResetRememberedSet();
+#endif  // defined(CPPGC_YOUNG_GENERATION)
+
   {
     cppgc::subtle::NoGarbageCollectionScope no_gc(*this);
     cppgc::internal::Sweeper::SweepingConfig::CompactableSpaceHandling

src/heap/cppgc-js/unified-heap-marking-verifier.cc

@@ -56,9 +56,10 @@ class UnifiedHeapVerificationVisitor final : public JSVisitor {
 }  // namespace
 
 UnifiedHeapMarkingVerifier::UnifiedHeapMarkingVerifier(
-    cppgc::internal::HeapBase& heap_base)
+    cppgc::internal::HeapBase& heap_base,
+    cppgc::internal::Heap::Config::CollectionType collection_type)
     : MarkingVerifierBase(
-          heap_base, state_,
+          heap_base, collection_type, state_,
           std::make_unique<UnifiedHeapVerificationVisitor>(state_)) {}
 
 }  // namespace internal

src/heap/cppgc-js/unified-heap-marking-verifier.h

@@ -13,7 +13,8 @@ namespace internal {
 class V8_EXPORT_PRIVATE UnifiedHeapMarkingVerifier final
     : public cppgc::internal::MarkingVerifierBase {
  public:
-  explicit UnifiedHeapMarkingVerifier(cppgc::internal::HeapBase&);
+  UnifiedHeapMarkingVerifier(cppgc::internal::HeapBase&,
+                             cppgc::internal::Heap::Config::CollectionType);
   ~UnifiedHeapMarkingVerifier() final = default;
 
  private:

src/heap/cppgc/heap-base.cc

@@ -108,6 +108,31 @@ size_t HeapBase::ExecutePreFinalizers() {
   return prefinalizer_handler_->ExtractBytesAllocatedInPrefinalizers();
 }
 
+#if defined(CPPGC_YOUNG_GENERATION)
+void HeapBase::ResetRememberedSet() {
+  class AllLABsAreEmpty final : protected HeapVisitor<AllLABsAreEmpty> {
+    friend class HeapVisitor<AllLABsAreEmpty>;
+
+   public:
+    explicit AllLABsAreEmpty(RawHeap& raw_heap) { Traverse(raw_heap); }
+
+    bool value() const { return !some_lab_is_set_; }
+
+   protected:
+    bool VisitNormalPageSpace(NormalPageSpace& space) {
+      some_lab_is_set_ |= space.linear_allocation_buffer().size();
+      return true;
+    }
+
+   private:
+    bool some_lab_is_set_ = false;
+  };
+  DCHECK(AllLABsAreEmpty(raw_heap()).value());
+  caged_heap().local_data().age_table.Reset(&caged_heap().allocator());
+  remembered_slots().clear();
+}
+#endif  // defined(CPPGC_YOUNG_GENERATION)
+
 void HeapBase::Terminate() {
   DCHECK(!IsMarking());
   CHECK(!in_disallow_gc_scope());

src/heap/cppgc/heap-base.h

@@ -163,7 +163,7 @@ class V8_EXPORT_PRIVATE HeapBase : public cppgc::HeapHandle {
 
 #if defined(CPPGC_YOUNG_GENERATION)
   std::set<void*>& remembered_slots() { return remembered_slots_; }
-#endif
+#endif  // defined(CPPGC_YOUNG_GENERATION)
 
   size_t ObjectPayloadSize() const;
 
@@ -218,6 +218,10 @@ class V8_EXPORT_PRIVATE HeapBase : public cppgc::HeapHandle {
   // Returns amount of bytes allocated while executing prefinalizers.
   size_t ExecutePreFinalizers();
 
+#if defined(CPPGC_YOUNG_GENERATION)
+  void ResetRememberedSet();
+#endif  // defined(CPPGC_YOUNG_GENERATION)
+
   PageAllocator* page_allocator() const;
 
   RawHeap raw_heap_;

src/heap/cppgc/heap.cc

@@ -149,7 +149,7 @@ void Heap::StartGarbageCollection(Config config) {
 #if defined(CPPGC_YOUNG_GENERATION)
   if (config.collection_type == Config::CollectionType::kMajor)
     SequentialUnmarker unmarker(raw_heap());
-#endif
+#endif  // defined(CPPGC_YOUNG_GENERATION)
 
   const Marker::MarkingConfig marking_config{
       config.collection_type, config.stack_state, config.marking_type,
@@ -177,7 +177,7 @@ void Heap::FinalizeGarbageCollection(Config::StackState stack_state) {
   marker_.reset();
   const size_t bytes_allocated_in_prefinalizers = ExecutePreFinalizers();
 #if CPPGC_VERIFY_HEAP
-  MarkingVerifier verifier(*this);
+  MarkingVerifier verifier(*this, config_.collection_type);
   verifier.Run(
       config_.stack_state, stack_end_of_current_gc(),
       stats_collector()->marked_bytes() + bytes_allocated_in_prefinalizers);
@@ -187,6 +187,10 @@ void Heap::FinalizeGarbageCollection(Config::StackState stack_state) {
 #endif
   USE(bytes_allocated_in_prefinalizers);
 
+#if defined(CPPGC_YOUNG_GENERATION)
+  ResetRememberedSet();
+#endif  // defined(CPPGC_YOUNG_GENERATION)
+
   subtle::NoGarbageCollectionScope no_gc(*this);
   const Sweeper::SweepingConfig sweeping_config{
       config_.sweeping_type,

src/heap/cppgc/marker.cc

@@ -82,15 +82,6 @@ void VisitRememberedSlots(HeapBase& heap,
 #endif
 }
 
-// Assumes that all spaces have their LABs reset.
-void ResetRememberedSet(HeapBase& heap) {
-#if defined(CPPGC_YOUNG_GENERATION)
-  auto& local_data = heap.caged_heap().local_data();
-  local_data.age_table.Reset(&heap.caged_heap().allocator());
-  heap.remembered_slots().clear();
-#endif
-}
-
 static constexpr size_t kDefaultDeadlineCheckInterval = 150u;
 
 template <size_t kDeadlineCheckInterval = kDefaultDeadlineCheckInterval,
@@ -294,7 +285,6 @@ void MarkerBase::LeaveAtomicPause() {
     StatsCollector::EnabledScope stats_scope(
         heap().stats_collector(), StatsCollector::kMarkAtomicEpilogue);
     DCHECK(!incremental_marking_handle_);
-    ResetRememberedSet(heap());
     heap().stats_collector()->NotifyMarkingCompleted(
         // GetOverallMarkedBytes also includes concurrently marked bytes.
         schedule_.GetOverallMarkedBytes());

src/heap/cppgc/marking-verifier.cc

@@ -4,6 +4,7 @@
 
 #include "src/heap/cppgc/marking-verifier.h"
 
+#include "include/cppgc/internal/caged-heap-local-data.h"
 #include "src/base/logging.h"
 #include "src/heap/cppgc/gc-info-table.h"
 #include "src/heap/cppgc/heap-object-header.h"
@@ -14,12 +15,31 @@
 namespace cppgc {
 namespace internal {
 
+void VerificationState::VerifyMarked(const void* base_object_payload) const {
+  const HeapObjectHeader& child_header =
+      HeapObjectHeader::FromObject(base_object_payload);
+
+  if (!child_header.IsMarked()) {
+    FATAL(
+        "MarkingVerifier: Encountered unmarked object.\n"
+        "#\n"
+        "# Hint:\n"
+        "#   %s (%p)\n"
+        "#     \\-> %s (%p)",
+        parent_ ? parent_->GetName().value : "Stack",
+        parent_ ? parent_->ObjectStart() : nullptr,
+        child_header.GetName().value, child_header.ObjectStart());
+  }
+}
+
 MarkingVerifierBase::MarkingVerifierBase(
-    HeapBase& heap, VerificationState& verification_state,
+    HeapBase& heap, Heap::Config::CollectionType collection_type,
+    VerificationState& verification_state,
     std::unique_ptr<cppgc::Visitor> visitor)
     : ConservativeTracingVisitor(heap, *heap.page_backend(), *visitor.get()),
       verification_state_(verification_state),
-      visitor_(std::move(visitor)) {}
+      visitor_(std::move(visitor)),
+      collection_type_(collection_type) {}
 
 void MarkingVerifierBase::Run(
     Heap::Config::StackState stack_state, uintptr_t stack_end,
@@ -38,25 +58,8 @@ void MarkingVerifierBase::Run(
                in_construction_objects_heap_.find(header));
     }
   }
-  if (expected_marked_bytes) {
-    CHECK_EQ(expected_marked_bytes.value(), found_marked_bytes_);
-  }
-}
-
-void VerificationState::VerifyMarked(const void* base_object_payload) const {
-  const HeapObjectHeader& child_header =
-      HeapObjectHeader::FromObject(base_object_payload);
-
-  if (!child_header.IsMarked()) {
-    FATAL(
-        "MarkingVerifier: Encountered unmarked object.\n"
-        "#\n"
-        "# Hint:\n"
-        "#   %s (%p)\n"
-        "#     \\-> %s (%p)",
-        parent_ ? parent_->GetName().value : "Stack",
-        parent_ ? parent_->ObjectStart() : nullptr,
-        child_header.GetName().value, child_header.ObjectStart());
+  if (expected_marked_bytes && verifier_found_marked_bytes_are_exact_) {
+    CHECK_EQ(expected_marked_bytes.value(), verifier_found_marked_bytes_);
   }
 }
 
@@ -93,6 +96,24 @@ bool MarkingVerifierBase::VisitHeapObjectHeader(HeapObjectHeader& header) {
 
   DCHECK(!header.IsFree());
 
+#if defined(CPPGC_YOUNG_GENERATION)
+  if (collection_type_ == Heap::Config::CollectionType::kMinor) {
+    const auto age = heap_.caged_heap()
+                         .local_data()
+                         .age_table[heap_.caged_heap().OffsetFromAddress(
+                             header.ObjectStart())];
+    if (age == AgeTable::Age::kOld) {
+      // Do not verify old objects.
+      return true;
+    } else if (age == AgeTable::Age::kUnknown) {
+      // If the age is not known, the marked bytes may not be exact as possibly
+      // old objects are verified as well.
+      verifier_found_marked_bytes_are_exact_ = false;
+    }
+    // Verify young and unknown objects.
+  }
+#endif  // defined(CPPGC_YOUNG_GENERATION)
+
   verification_state_.SetCurrentParent(&header);
 
   if (!header.IsInConstruction()) {
@@ -102,7 +123,8 @@ bool MarkingVerifierBase::VisitHeapObjectHeader(HeapObjectHeader& header) {
     TraceConservativelyIfNeeded(header);
   }
 
-  found_marked_bytes_ += ObjectView(header).Size() + sizeof(HeapObjectHeader);
+  verifier_found_marked_bytes_ +=
+      ObjectView(header).Size() + sizeof(HeapObjectHeader);
 
   verification_state_.SetCurrentParent(nullptr);
 
@@ -146,8 +168,9 @@ class VerificationVisitor final : public cppgc::Visitor {
 
 }  // namespace
 
-MarkingVerifier::MarkingVerifier(HeapBase& heap_base)
-    : MarkingVerifierBase(heap_base, state_,
+MarkingVerifier::MarkingVerifier(HeapBase& heap_base,
+                                 Heap::Config::CollectionType collection_type)
+    : MarkingVerifierBase(heap_base, collection_type, state_,
                           std::make_unique<VerificationVisitor>(state_)) {}
 
 }  // namespace internal

src/heap/cppgc/marking-verifier.h

@@ -44,8 +44,8 @@ class V8_EXPORT_PRIVATE MarkingVerifierBase
   void Run(Heap::Config::StackState, uintptr_t, v8::base::Optional<size_t>);
 
  protected:
-  MarkingVerifierBase(HeapBase&, VerificationState&,
-                      std::unique_ptr<cppgc::Visitor>);
+  MarkingVerifierBase(HeapBase&, Heap::Config::CollectionType,
+                      VerificationState&, std::unique_ptr<cppgc::Visitor>);
 
  private:
   void VisitInConstructionConservatively(HeapObjectHeader&,
@@ -61,12 +61,14 @@ class V8_EXPORT_PRIVATE MarkingVerifierBase
   std::unordered_set<const HeapObjectHeader*> in_construction_objects_stack_;
   std::unordered_set<const HeapObjectHeader*>* in_construction_objects_ =
       &in_construction_objects_heap_;
-  size_t found_marked_bytes_ = 0;
+  size_t verifier_found_marked_bytes_ = 0;
+  bool verifier_found_marked_bytes_are_exact_ = true;
+  Heap::Config::CollectionType collection_type_;
 };
 
 class V8_EXPORT_PRIVATE MarkingVerifier final : public MarkingVerifierBase {
  public:
-  explicit MarkingVerifier(HeapBase&);
+  MarkingVerifier(HeapBase&, Heap::Config::CollectionType);
   ~MarkingVerifier() final = default;
 
  private:

test/unittests/heap/cppgc/marking-verifier-unittest.cc

@@ -25,7 +25,7 @@ class MarkingVerifierTest : public testing::TestWithHeap {
   V8_NOINLINE void VerifyMarking(HeapBase& heap, StackState stack_state,
                                  size_t expected_marked_bytes) {
     Heap::From(GetHeap())->object_allocator().ResetLinearAllocationBuffers();
-    MarkingVerifier verifier(heap);
+    MarkingVerifier verifier(heap, Heap::Config::CollectionType::kMajor);
     verifier.Run(stack_state, v8::base::Stack::GetCurrentStackPosition(),
                  expected_marked_bytes);
   }

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

@@ -6,6 +6,7 @@
 
 #include "include/cppgc/allocation.h"
 #include "include/cppgc/heap-consistency.h"
+#include "include/cppgc/internal/caged-heap-local-data.h"
 #include "include/cppgc/persistent.h"
 #include "src/heap/cppgc/heap-object-header.h"
 #include "src/heap/cppgc/heap.h"
@@ -136,6 +137,7 @@ TYPED_TEST(MinorGCTestForType, OldObjectIsNotVisited) {
 
 template <typename Type1, typename Type2>
 void InterGenerationalPointerTest(MinorGCTest* test, cppgc::Heap* heap) {
+  auto* internal_heap = Heap::From(heap);
   Persistent<Type1> old =
       MakeGarbageCollected<Type1>(heap->GetAllocationHandle());
   test->CollectMinor();
@@ -152,6 +154,11 @@ void InterGenerationalPointerTest(MinorGCTest* test, cppgc::Heap* heap) {
       ptr->next = young;
       young = ptr;
       EXPECT_TRUE(HeapObjectHeader::FromObject(young).IsYoung());
+      const uintptr_t offset =
+          internal_heap->caged_heap().OffsetFromAddress(young);
+      // Age may be young or unknown.
+      EXPECT_NE(AgeTable::Age::kOld,
+                Heap::From(heap)->caged_heap().local_data().age_table[offset]);
     }
   }