cppgc: young-gen: Extract and report metrics for young GC cycles

The CL makes sure to extract and copy Oilpan young GC metrics to
v8::metrics::GarbageCollectionYoungCycle. In addition, it makes sure
that metrics are not reported twice by bailing out from
GCTracer::NotifyCppGCCompleted() for young GC cycles (the metrics are
reported later in Heap::CollectGarbage() by calling
GCTracer::StopCycle()).

Bug: chromium:1029379
Change-Id: I07bf51e85a76a7cdbeeb8d87c9072edf2634158b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3545168Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Anton Bikineev <bikineev@chromium.org>
Cr-Commit-Position: refs/heads/main@{#79766}

include/v8-metrics.h

@@ -82,9 +82,17 @@ struct GarbageCollectionYoungCycle {
   int reason = -1;
   int64_t total_wall_clock_duration_in_us = -1;
   int64_t main_thread_wall_clock_duration_in_us = -1;
-  double collection_rate_in_percent;
-  double efficiency_in_bytes_per_us;
-  double main_thread_efficiency_in_bytes_per_us;
+  double collection_rate_in_percent = -1.0;
+  double efficiency_in_bytes_per_us = -1.0;
+  double main_thread_efficiency_in_bytes_per_us = -1.0;
+#if defined(CPPGC_YOUNG_GENERATION)
+  GarbageCollectionPhases total_cpp;
+  GarbageCollectionSizes objects_cpp;
+  GarbageCollectionSizes memory_cpp;
+  double collection_rate_cpp_in_percent = -1.0;
+  double efficiency_cpp_in_bytes_per_us = -1.0;
+  double main_thread_efficiency_cpp_in_bytes_per_us = -1.0;
+#endif  // defined(CPPGC_YOUNG_GENERATION)
 };
 
 struct WasmModuleDecoded {

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

@@ -302,10 +302,16 @@ void UnifiedHeapMarker::AddObject(void* object) {
 }
 
 void CppHeap::MetricRecorderAdapter::AddMainThreadEvent(
-    const FullCycle& cppgc_event) {
-  DCHECK(!last_full_gc_event_.has_value());
-  last_full_gc_event_ = cppgc_event;
-  GetIsolate()->heap()->tracer()->NotifyCppGCCompleted();
+    const GCCycle& cppgc_event) {
+  if (cppgc_event.type == MetricRecorder::GCCycle::Type::kMinor) {
+    last_young_gc_event_ = cppgc_event;
+  } else {
+    last_full_gc_event_ = cppgc_event;
+  }
+  GetIsolate()->heap()->tracer()->NotifyCppGCCompleted(
+      cppgc_event.type == MetricRecorder::GCCycle::Type::kMinor
+          ? GCTracer::CppType::kMinor
+          : GCTracer::CppType::kMajor);
 }
 
 void CppHeap::MetricRecorderAdapter::AddMainThreadEvent(
@@ -366,17 +372,28 @@ void CppHeap::MetricRecorderAdapter::FlushBatchedIncrementalEvents() {
   }
 }
 
-bool CppHeap::MetricRecorderAdapter::MetricsReportPending() const {
+bool CppHeap::MetricRecorderAdapter::FullGCMetricsReportPending() const {
   return last_full_gc_event_.has_value();
 }
 
-const base::Optional<cppgc::internal::MetricRecorder::FullCycle>
+bool CppHeap::MetricRecorderAdapter::YoungGCMetricsReportPending() const {
+  return last_young_gc_event_.has_value();
+}
+
+const base::Optional<cppgc::internal::MetricRecorder::GCCycle>
 CppHeap::MetricRecorderAdapter::ExtractLastFullGcEvent() {
   auto res = std::move(last_full_gc_event_);
   last_full_gc_event_.reset();
   return res;
 }
 
+const base::Optional<cppgc::internal::MetricRecorder::GCCycle>
+CppHeap::MetricRecorderAdapter::ExtractLastYoungGcEvent() {
+  auto res = std::move(last_young_gc_event_);
+  last_young_gc_event_.reset();
+  return res;
+}
+
 const base::Optional<cppgc::internal::MetricRecorder::MainThreadIncrementalMark>
 CppHeap::MetricRecorderAdapter::ExtractLastIncrementalMarkEvent() {
   auto res = std::move(last_incremental_mark_event_);
@@ -389,6 +406,7 @@ void CppHeap::MetricRecorderAdapter::ClearCachedEvents() {
   incremental_sweep_batched_events_.events.clear();
   last_incremental_mark_event_.reset();
   last_full_gc_event_.reset();
+  last_young_gc_event_.reset();
 }
 
 Isolate* CppHeap::MetricRecorderAdapter::GetIsolate() const {
@@ -516,9 +534,15 @@ void CppHeap::InitializeTracing(
     GarbageCollectionFlags gc_flags) {
   CHECK(!sweeper_.IsSweepingInProgress());
 
-  // Check that previous cycle metrics have been reported.
-  DCHECK_IMPLIES(GetMetricRecorder(),
-                 !GetMetricRecorder()->MetricsReportPending());
+  // Check that previous cycle metrics for the same collection type have been
+  // reported.
+  if (GetMetricRecorder()) {
+    if (collection_type ==
+        cppgc::internal::GarbageCollector::Config::CollectionType::kMajor)
+      DCHECK(!GetMetricRecorder()->FullGCMetricsReportPending());
+    else
+      DCHECK(!GetMetricRecorder()->YoungGCMetricsReportPending());
+  }
 
   DCHECK(!collection_type_);
   collection_type_ = collection_type;

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

@@ -49,19 +49,22 @@ class V8_EXPORT_PRIVATE CppHeap final
 
     explicit MetricRecorderAdapter(CppHeap& cpp_heap) : cpp_heap_(cpp_heap) {}
 
-    void AddMainThreadEvent(const FullCycle& cppgc_event) final;
+    void AddMainThreadEvent(const GCCycle& cppgc_event) final;
     void AddMainThreadEvent(const MainThreadIncrementalMark& cppgc_event) final;
     void AddMainThreadEvent(
         const MainThreadIncrementalSweep& cppgc_event) final;
 
     void FlushBatchedIncrementalEvents();
 
-    // The following 3 methods are only used for reporting nested cpp events
+    // The following methods are only used for reporting nested cpp events
     // through V8. Standalone events are reported directly.
-    bool MetricsReportPending() const;
+    bool FullGCMetricsReportPending() const;
+    bool YoungGCMetricsReportPending() const;
 
-    const base::Optional<cppgc::internal::MetricRecorder::FullCycle>
+    const base::Optional<cppgc::internal::MetricRecorder::GCCycle>
     ExtractLastFullGcEvent();
+    const base::Optional<cppgc::internal::MetricRecorder::GCCycle>
+    ExtractLastYoungGcEvent();
     const base::Optional<
         cppgc::internal::MetricRecorder::MainThreadIncrementalMark>
     ExtractLastIncrementalMarkEvent();
@@ -78,8 +81,10 @@ class V8_EXPORT_PRIVATE CppHeap final
         incremental_mark_batched_events_;
     v8::metrics::GarbageCollectionFullMainThreadBatchedIncrementalSweep
         incremental_sweep_batched_events_;
-    base::Optional<cppgc::internal::MetricRecorder::FullCycle>
+    base::Optional<cppgc::internal::MetricRecorder::GCCycle>
         last_full_gc_event_;
+    base::Optional<cppgc::internal::MetricRecorder::GCCycle>
+        last_young_gc_event_;
     base::Optional<cppgc::internal::MetricRecorder::MainThreadIncrementalMark>
         last_incremental_mark_event_;
   };

src/heap/cppgc/metric-recorder.h

@@ -20,7 +20,8 @@ class StatsCollector;
  */
 class MetricRecorder {
  public:
-  struct FullCycle {
+  struct GCCycle {
+    enum class Type { kMinor, kMajor };
     struct IncrementalPhases {
       int64_t mark_duration_us = -1;
       int64_t sweep_duration_us = -1;
@@ -35,6 +36,7 @@ class MetricRecorder {
       int64_t freed_bytes = -1;
     };
 
+    Type type = Type::kMajor;
     Phases total;
     Phases main_thread;
     Phases main_thread_atomic;
@@ -56,7 +58,7 @@ class MetricRecorder {
 
   virtual ~MetricRecorder() = default;
 
-  virtual void AddMainThreadEvent(const FullCycle& event) {}
+  virtual void AddMainThreadEvent(const GCCycle& event) {}
   virtual void AddMainThreadEvent(const MainThreadIncrementalMark& event) {}
   virtual void AddMainThreadEvent(const MainThreadIncrementalSweep& event) {}
 };

src/heap/cppgc/stats-collector.cc

@@ -159,20 +159,23 @@ double StatsCollector::GetRecentAllocationSpeedInBytesPerMs() const {
 
 namespace {
 
-int64_t SumPhases(const MetricRecorder::FullCycle::Phases& phases) {
+int64_t SumPhases(const MetricRecorder::GCCycle::Phases& phases) {
   return phases.mark_duration_us + phases.weak_duration_us +
          phases.compact_duration_us + phases.sweep_duration_us;
 }
 
-MetricRecorder::FullCycle GetFullCycleEventForMetricRecorder(
-    int64_t atomic_mark_us, int64_t atomic_weak_us, int64_t atomic_compact_us,
-    int64_t atomic_sweep_us, int64_t incremental_mark_us,
-    int64_t incremental_sweep_us, int64_t concurrent_mark_us,
-    int64_t concurrent_sweep_us, int64_t objects_before_bytes,
-    int64_t objects_after_bytes, int64_t objects_freed_bytes,
-    int64_t memory_before_bytes, int64_t memory_after_bytes,
-    int64_t memory_freed_bytes) {
-  MetricRecorder::FullCycle event;
+MetricRecorder::GCCycle GetCycleEventForMetricRecorder(
+    StatsCollector::CollectionType type, int64_t atomic_mark_us,
+    int64_t atomic_weak_us, int64_t atomic_compact_us, int64_t atomic_sweep_us,
+    int64_t incremental_mark_us, int64_t incremental_sweep_us,
+    int64_t concurrent_mark_us, int64_t concurrent_sweep_us,
+    int64_t objects_before_bytes, int64_t objects_after_bytes,
+    int64_t objects_freed_bytes, int64_t memory_before_bytes,
+    int64_t memory_after_bytes, int64_t memory_freed_bytes) {
+  MetricRecorder::GCCycle event;
+  event.type = (type == StatsCollector::CollectionType::kMajor)
+                   ? MetricRecorder::GCCycle::Type::kMajor
+                   : MetricRecorder::GCCycle::Type::kMinor;
   // MainThread.Incremental:
   event.main_thread_incremental.mark_duration_us = incremental_mark_us;
   event.main_thread_incremental.sweep_duration_us = incremental_sweep_us;
@@ -228,7 +231,8 @@ void StatsCollector::NotifySweepingCompleted() {
   previous_ = std::move(current_);
   current_ = Event();
   if (metric_recorder_) {
-    MetricRecorder::FullCycle event = GetFullCycleEventForMetricRecorder(
+    MetricRecorder::GCCycle event = GetCycleEventForMetricRecorder(
+        previous_.collection_type,
         previous_.scope_data[kAtomicMark].InMicroseconds(),
         previous_.scope_data[kAtomicWeak].InMicroseconds(),
         previous_.scope_data[kAtomicCompact].InMicroseconds(),

src/heap/cppgc/stats-collector.h

@@ -66,10 +66,11 @@ namespace internal {
 
 // Sink for various time and memory statistics.
 class V8_EXPORT_PRIVATE StatsCollector final {
-  using CollectionType = GarbageCollector::Config::CollectionType;
   using IsForcedGC = GarbageCollector::Config::IsForcedGC;
 
  public:
+  using CollectionType = GarbageCollector::Config::CollectionType;
+
 #if defined(CPPGC_DECLARE_ENUM)
   static_assert(false, "CPPGC_DECLARE_ENUM macro is already defined");
 #endif

src/heap/gc-tracer.cc

@@ -589,13 +589,23 @@ void GCTracer::NotifySweepingCompleted() {
   StopCycleIfNeeded();
 }
 
-void GCTracer::NotifyCppGCCompleted() {
+void GCTracer::NotifyCppGCCompleted(CppType collection_type) {
   // Stop a full GC cycle only when both v8 and cppgc (if available) GCs have
   // finished sweeping. This method is invoked by cppgc.
   DCHECK(heap_->cpp_heap());
-  DCHECK(CppHeap::From(heap_->cpp_heap())
-             ->GetMetricRecorder()
-             ->MetricsReportPending());
+  const auto* metric_recorder =
+      CppHeap::From(heap_->cpp_heap())->GetMetricRecorder();
+  USE(metric_recorder);
+  if (collection_type == CppType::kMinor) {
+    DCHECK(metric_recorder->YoungGCMetricsReportPending());
+    // Young generation GCs in Oilpan run together with Scavenger. Check that
+    // collection types are in sync.
+    DCHECK(Event::IsYoungGenerationEvent(current_.type));
+    // Don't stop the cycle (i.e. call StopCycle/StopCycleIfNeeded) for young
+    // generation events - this is performed later in Heap::CollectGarbage().
+    return;
+  }
+  DCHECK(metric_recorder->FullGCMetricsReportPending());
   DCHECK(!notified_cppgc_completed_);
   notified_cppgc_completed_ = true;
   StopCycleIfNeeded();
@@ -1446,7 +1456,7 @@ namespace {
 
 void CopyTimeMetrics(
     ::v8::metrics::GarbageCollectionPhases& metrics,
-    const cppgc::internal::MetricRecorder::FullCycle::IncrementalPhases&
+    const cppgc::internal::MetricRecorder::GCCycle::IncrementalPhases&
         cppgc_metrics) {
   DCHECK_NE(-1, cppgc_metrics.mark_duration_us);
   metrics.mark_wall_clock_duration_in_us = cppgc_metrics.mark_duration_us;
@@ -1459,7 +1469,7 @@ void CopyTimeMetrics(
 
 void CopyTimeMetrics(
     ::v8::metrics::GarbageCollectionPhases& metrics,
-    const cppgc::internal::MetricRecorder::FullCycle::Phases& cppgc_metrics) {
+    const cppgc::internal::MetricRecorder::GCCycle::Phases& cppgc_metrics) {
   DCHECK_NE(-1, cppgc_metrics.compact_duration_us);
   metrics.compact_wall_clock_duration_in_us = cppgc_metrics.compact_duration_us;
   DCHECK_NE(-1, cppgc_metrics.mark_duration_us);
@@ -1477,7 +1487,7 @@ void CopyTimeMetrics(
 
 void CopySizeMetrics(
     ::v8::metrics::GarbageCollectionSizes& metrics,
-    const cppgc::internal::MetricRecorder::FullCycle::Sizes& cppgc_metrics) {
+    const cppgc::internal::MetricRecorder::GCCycle::Sizes& cppgc_metrics) {
   DCHECK_NE(-1, cppgc_metrics.after_bytes);
   metrics.bytes_after = cppgc_metrics.after_bytes;
   DCHECK_NE(-1, cppgc_metrics.before_bytes);
@@ -1513,7 +1523,7 @@ void GCTracer::ReportFullCycleToRecorder() {
   DCHECK_EQ(Event::State::NOT_RUNNING, current_.state);
   auto* cpp_heap = v8::internal::CppHeap::From(heap_->cpp_heap());
   DCHECK_IMPLIES(cpp_heap,
-                 cpp_heap->GetMetricRecorder()->MetricsReportPending());
+                 cpp_heap->GetMetricRecorder()->FullGCMetricsReportPending());
   const std::shared_ptr<metrics::Recorder>& recorder =
       heap_->isolate()->metrics_recorder();
   DCHECK_NOT_NULL(recorder);
@@ -1538,13 +1548,15 @@ void GCTracer::ReportFullCycleToRecorder() {
   // Managed C++ heap statistics:
   if (cpp_heap) {
     cpp_heap->GetMetricRecorder()->FlushBatchedIncrementalEvents();
-    const base::Optional<cppgc::internal::MetricRecorder::FullCycle>
+    const base::Optional<cppgc::internal::MetricRecorder::GCCycle>
         optional_cppgc_event =
             cpp_heap->GetMetricRecorder()->ExtractLastFullGcEvent();
     DCHECK(optional_cppgc_event.has_value());
-    DCHECK(!cpp_heap->GetMetricRecorder()->MetricsReportPending());
-    const cppgc::internal::MetricRecorder::FullCycle& cppgc_event =
+    DCHECK(!cpp_heap->GetMetricRecorder()->FullGCMetricsReportPending());
+    const cppgc::internal::MetricRecorder::GCCycle& cppgc_event =
         optional_cppgc_event.value();
+    DCHECK_EQ(cppgc_event.type,
+              cppgc::internal::MetricRecorder::GCCycle::Type::kMajor);
     CopyTimeMetrics(event.total_cpp, cppgc_event.total);
     CopyTimeMetrics(event.main_thread_cpp, cppgc_event.main_thread);
     CopyTimeMetrics(event.main_thread_atomic_cpp,
@@ -1699,9 +1711,41 @@ void GCTracer::ReportYoungCycleToRecorder() {
       heap_->isolate()->metrics_recorder();
   DCHECK_NOT_NULL(recorder);
   if (!recorder->HasEmbedderRecorder()) return;
+
   v8::metrics::GarbageCollectionYoungCycle event;
   // Reason:
   event.reason = static_cast<int>(current_.gc_reason);
+#if defined(CPPGC_YOUNG_GENERATION)
+  // Managed C++ heap statistics:
+  auto* cpp_heap = v8::internal::CppHeap::From(heap_->cpp_heap());
+  if (cpp_heap) {
+    auto* metric_recorder = cpp_heap->GetMetricRecorder();
+    const base::Optional<cppgc::internal::MetricRecorder::GCCycle>
+        optional_cppgc_event = metric_recorder->ExtractLastYoungGcEvent();
+    // We bail out from Oilpan's young GC if the full GC is already in progress.
+    // Check here if the young generation event was reported.
+    if (optional_cppgc_event) {
+      DCHECK(!metric_recorder->YoungGCMetricsReportPending());
+      const cppgc::internal::MetricRecorder::GCCycle& cppgc_event =
+          optional_cppgc_event.value();
+      DCHECK_EQ(cppgc_event.type,
+                cppgc::internal::MetricRecorder::GCCycle::Type::kMinor);
+      CopyTimeMetrics(event.total_cpp, cppgc_event.total);
+      CopySizeMetrics(event.objects_cpp, cppgc_event.objects);
+      CopySizeMetrics(event.memory_cpp, cppgc_event.memory);
+      DCHECK_NE(-1, cppgc_event.collection_rate_in_percent);
+      event.collection_rate_cpp_in_percent =
+          cppgc_event.collection_rate_in_percent;
+      DCHECK_NE(-1, cppgc_event.efficiency_in_bytes_per_us);
+      event.efficiency_cpp_in_bytes_per_us =
+          cppgc_event.efficiency_in_bytes_per_us;
+      DCHECK_NE(-1, cppgc_event.main_thread_efficiency_in_bytes_per_us);
+      event.main_thread_efficiency_cpp_in_bytes_per_us =
+          cppgc_event.main_thread_efficiency_in_bytes_per_us;
+    }
+  }
+#endif  // defined(CPPGC_YOUNG_GENERATION)
+
   // Total:
   const double total_wall_clock_duration_in_us =
       (current_.scopes[Scope::SCAVENGER] +

src/heap/gc-tracer.h

@@ -139,13 +139,11 @@ class V8_EXPORT_PRIVATE GCTracer {
       START = 4
     };
 
-#ifdef DEBUG
     // Returns true if the event corresponds to a young generation GC.
     static constexpr bool IsYoungGenerationEvent(Type type) {
       DCHECK_NE(START, type);
       return type == SCAVENGER || type == MINOR_MARK_COMPACTOR;
     }
-#endif
 
     // The state diagram for a GC cycle:
     //   (NOT_RUNNING) -----(StartCycle)----->
@@ -237,6 +235,11 @@ class V8_EXPORT_PRIVATE GCTracer {
     TimedHistogram* type_priority_timer;
   };
 
+  enum class CppType {
+    kMinor,
+    kMajor,
+  };
+
   static const int kThroughputTimeFrameMs = 5000;
   static constexpr double kConservativeSpeedInBytesPerMillisecond = 128 * KB;
 
@@ -279,7 +282,7 @@ class V8_EXPORT_PRIVATE GCTracer {
   void StopInSafepoint();
 
   void NotifySweepingCompleted();
-  void NotifyCppGCCompleted();
+  void NotifyCppGCCompleted(CppType);
 
   void NotifyYoungGenerationHandling(
       YoungGenerationHandling young_generation_handling);