heap: Refactor GC type timer methods

This CL refactors Heap::GCTypeTimer and Heap::GCTypePriorityTimer
and moves them to a GCTracer::RecordGCPhasesInfo class. This is
a necessary change for deprecating counters that are used for
old style GC metrics, like gc_scavenger. When all such counters
are deprecated, GCTracer::RecordGCPhasesInfo will no longer be
necessary and will be removed.

Bug: chromium:1154636
Change-Id: I04504a0f6c7a0955f4300a1c94c969aaeb23b77f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3486556Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Nikolaos Papaspyrou <nikolaos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#79257}

src/heap/gc-tracer.cc

@@ -195,6 +195,45 @@ const char* GCTracer::Event::TypeName(bool short_name) const {
   return "Unknown Event Type";
 }
 
+GCTracer::RecordGCPhasesInfo::RecordGCPhasesInfo(Heap* heap,
+                                                 GarbageCollector collector) {
+  Counters* counters = heap->isolate()->counters();
+  const bool in_background = heap->isolate()->IsIsolateInBackground();
+  if (Heap::IsYoungGenerationCollector(collector)) {
+    mode = Mode::Scavenger;
+    type_timer = counters->gc_scavenger();
+    type_priority_timer = in_background ? counters->gc_scavenger_background()
+                                        : counters->gc_scavenger_foreground();
+  } else {
+    DCHECK_EQ(GarbageCollector::MARK_COMPACTOR, collector);
+    if (heap->incremental_marking()->IsStopped()) {
+      mode = Mode::None;
+      type_timer = counters->gc_compactor();
+      type_priority_timer = in_background ? counters->gc_compactor_background()
+                                          : counters->gc_compactor_foreground();
+    } else if (heap->ShouldReduceMemory()) {
+      mode = Mode::None;
+      type_timer = counters->gc_finalize_reduce_memory();
+      type_priority_timer =
+          in_background ? counters->gc_finalize_reduce_memory_background()
+                        : counters->gc_finalize_reduce_memory_foreground();
+    } else {
+      if (heap->incremental_marking()->IsMarking() &&
+          heap->incremental_marking()
+              ->local_marking_worklists()
+              ->IsPerContextMode()) {
+        mode = Mode::None;
+        type_timer = counters->gc_finalize_measure_memory();
+      } else {
+        mode = Mode::Finalize;
+        type_timer = counters->gc_finalize();
+      }
+      type_priority_timer = in_background ? counters->gc_finalize_background()
+                                          : counters->gc_finalize_foreground();
+    }
+  }
+}
+
 GCTracer::GCTracer(Heap* heap)
     : heap_(heap),
       current_(Event::START, Event::State::NOT_RUNNING,
@@ -1272,9 +1311,9 @@ void GCTracer::AddScopeSampleBackground(Scope::ScopeId scope, double duration) {
   counter.total_duration_ms += duration;
 }
 
-void GCTracer::RecordGCPhasesHistograms(TimedHistogram* gc_timer) {
+void GCTracer::RecordGCPhasesHistograms(RecordGCPhasesInfo::Mode mode) {
   Counters* counters = heap_->isolate()->counters();
-  if (gc_timer == counters->gc_finalize()) {
+  if (mode == RecordGCPhasesInfo::Mode::Finalize) {
     DCHECK_EQ(Scope::FIRST_TOP_MC_SCOPE, Scope::MC_CLEAR);
     counters->gc_finalize_clear()->AddSample(
         static_cast<int>(current_.scopes[Scope::MC_CLEAR]));
@@ -1323,7 +1362,7 @@ void GCTracer::RecordGCPhasesHistograms(TimedHistogram* gc_timer) {
     }
 
     DCHECK_EQ(Scope::LAST_TOP_MC_SCOPE, Scope::MC_SWEEP);
-  } else if (gc_timer == counters->gc_scavenger()) {
+  } else if (mode == RecordGCPhasesInfo::Mode::Scavenger) {
     counters->gc_scavenger_scavenge_main()->AddSample(
         static_cast<int>(current_.scopes[Scope::SCAVENGER_SCAVENGE_PARALLEL]));
     counters->gc_scavenger_scavenge_roots()->AddSample(

src/heap/gc-tracer.h

@@ -219,6 +219,24 @@ class V8_EXPORT_PRIVATE GCTracer {
         incremental_marking_scopes[Scope::NUMBER_OF_INCREMENTAL_SCOPES];
   };
 
+  class RecordGCPhasesInfo {
+   public:
+    RecordGCPhasesInfo(Heap* heap, GarbageCollector collector);
+
+    enum class Mode { None, Scavenger, Finalize };
+
+    Mode mode;
+
+    // The timer used for a given GC type:
+    // - GCScavenger: young generation GC
+    // - GCCompactor: full GC
+    // - GCFinalizeMC: finalization of incremental full GC
+    // - GCFinalizeMCReduceMemory: finalization of incremental full GC with
+    //   memory reduction.
+    TimedHistogram* type_timer;
+    TimedHistogram* type_priority_timer;
+  };
+
   static const int kThroughputTimeFrameMs = 5000;
   static constexpr double kConservativeSpeedInBytesPerMillisecond = 128 * KB;
 
@@ -393,7 +411,7 @@ class V8_EXPORT_PRIVATE GCTracer {
 
   void AddScopeSampleBackground(Scope::ScopeId scope, double duration);
 
-  void RecordGCPhasesHistograms(TimedHistogram* gc_timer);
+  void RecordGCPhasesHistograms(RecordGCPhasesInfo::Mode mode);
 
   void RecordEmbedderSpeed(size_t bytes, double duration);
 

src/heap/heap.cc

@@ -1539,50 +1539,6 @@ void Heap::ScheduleScavengeTaskIfNeeded() {
   scavenge_job_->ScheduleTaskIfNeeded(this);
 }
 
-TimedHistogram* Heap::GCTypePriorityTimer(GarbageCollector collector) {
-  if (IsYoungGenerationCollector(collector)) {
-    if (isolate_->IsIsolateInBackground()) {
-      return isolate_->counters()->gc_scavenger_background();
-    }
-    return isolate_->counters()->gc_scavenger_foreground();
-  }
-  DCHECK_EQ(GarbageCollector::MARK_COMPACTOR, collector);
-  if (incremental_marking()->IsStopped()) {
-    if (isolate_->IsIsolateInBackground()) {
-      return isolate_->counters()->gc_compactor_background();
-    }
-    return isolate_->counters()->gc_compactor_foreground();
-  }
-  if (ShouldReduceMemory()) {
-    if (isolate_->IsIsolateInBackground()) {
-      return isolate_->counters()->gc_finalize_reduce_memory_background();
-    }
-    return isolate_->counters()->gc_finalize_reduce_memory_foreground();
-  }
-  if (isolate_->IsIsolateInBackground()) {
-    return isolate_->counters()->gc_finalize_background();
-  }
-  return isolate_->counters()->gc_finalize_foreground();
-}
-
-TimedHistogram* Heap::GCTypeTimer(GarbageCollector collector) {
-  if (IsYoungGenerationCollector(collector)) {
-    return isolate_->counters()->gc_scavenger();
-  }
-  DCHECK_EQ(GarbageCollector::MARK_COMPACTOR, collector);
-  if (incremental_marking()->IsStopped()) {
-    return isolate_->counters()->gc_compactor();
-  }
-  if (ShouldReduceMemory()) {
-    return isolate_->counters()->gc_finalize_reduce_memory();
-  }
-  if (incremental_marking()->IsMarking() &&
-      incremental_marking()->local_marking_worklists()->IsPerContextMode()) {
-    return isolate_->counters()->gc_finalize_measure_memory();
-  }
-  return isolate_->counters()->gc_finalize();
-}
-
 void Heap::CollectAllGarbage(int flags, GarbageCollectionReason gc_reason,
                              const v8::GCCallbackFlags gc_callback_flags) {
   // Since we are ignoring the return value, the exact choice of space does
@@ -1877,17 +1833,23 @@ bool Heap::CollectGarbage(AllocationSpace space,
 
     GarbageCollectionPrologue(gc_reason, gc_callback_flags);
     {
-      TimedHistogram* gc_type_timer = GCTypeTimer(collector);
-      TimedHistogramScope histogram_timer_scope(gc_type_timer, isolate_);
-      TRACE_EVENT0("v8", gc_type_timer->name());
-
-      TimedHistogram* gc_type_priority_timer = GCTypePriorityTimer(collector);
-      OptionalTimedHistogramScopeMode mode =
-          isolate_->IsMemorySavingsModeActive()
-              ? OptionalTimedHistogramScopeMode::DONT_TAKE_TIME
-              : OptionalTimedHistogramScopeMode::TAKE_TIME;
-      OptionalTimedHistogramScope histogram_timer_priority_scope(
-          gc_type_priority_timer, isolate_, mode);
+      GCTracer::RecordGCPhasesInfo record_gc_phases_info(this, collector);
+      base::Optional<TimedHistogramScope> histogram_timer_scope;
+      base::Optional<OptionalTimedHistogramScope>
+          histogram_timer_priority_scope;
+      if (record_gc_phases_info.type_timer) {
+        histogram_timer_scope.emplace(record_gc_phases_info.type_timer,
+                                      isolate_);
+        TRACE_EVENT0("v8", record_gc_phases_info.type_timer->name());
+      }
+      if (record_gc_phases_info.type_priority_timer) {
+        OptionalTimedHistogramScopeMode mode =
+            isolate_->IsMemorySavingsModeActive()
+                ? OptionalTimedHistogramScopeMode::DONT_TAKE_TIME
+                : OptionalTimedHistogramScopeMode::TAKE_TIME;
+        histogram_timer_priority_scope.emplace(
+            record_gc_phases_info.type_priority_timer, isolate_, mode);
+      }
 
       if (V8_ENABLE_THIRD_PARTY_HEAP_BOOL) {
         tp_heap_->CollectGarbage();
@@ -1903,7 +1865,7 @@ bool Heap::CollectGarbage(AllocationSpace space,
 
       if (collector == GarbageCollector::MARK_COMPACTOR ||
           collector == GarbageCollector::SCAVENGER) {
-        tracer()->RecordGCPhasesHistograms(gc_type_timer);
+        tracer()->RecordGCPhasesHistograms(record_gc_phases_info.mode);
       }
     }
 

src/heap/heap.h

@@ -1864,15 +1864,6 @@ class Heap {
   void InvokeIncrementalMarkingPrologueCallbacks();
   void InvokeIncrementalMarkingEpilogueCallbacks();
 
-  // Returns the timer used for a given GC type.
-  // - GCScavenger: young generation GC
-  // - GCCompactor: full GC
-  // - GCFinalzeMC: finalization of incremental full GC
-  // - GCFinalizeMCReduceMemory: finalization of incremental full GC with
-  // memory reduction
-  TimedHistogram* GCTypeTimer(GarbageCollector collector);
-  TimedHistogram* GCTypePriorityTimer(GarbageCollector collector);
-
   // ===========================================================================
   // Pretenuring. ==============================================================
   // ===========================================================================

test/unittests/heap/gc-tracer-unittest.cc

@@ -541,7 +541,8 @@ TEST_F(GCTracerTest, RecordMarkCompactHistograms) {
   tracer->current_.scopes[GCTracer::Scope::MC_MARK] = 5;
   tracer->current_.scopes[GCTracer::Scope::MC_PROLOGUE] = 6;
   tracer->current_.scopes[GCTracer::Scope::MC_SWEEP] = 7;
-  tracer->RecordGCPhasesHistograms(i_isolate()->counters()->gc_finalize());
+  tracer->RecordGCPhasesHistograms(
+      GCTracer::RecordGCPhasesInfo::Mode::Finalize);
   EXPECT_EQ(1, GcHistogram::Get("V8.GCFinalizeMC.Clear")->Total());
   EXPECT_EQ(2, GcHistogram::Get("V8.GCFinalizeMC.Epilogue")->Total());
   EXPECT_EQ(3, GcHistogram::Get("V8.GCFinalizeMC.Evacuate")->Total());
@@ -560,7 +561,8 @@ TEST_F(GCTracerTest, RecordScavengerHistograms) {
   tracer->ResetForTesting();
   tracer->current_.scopes[GCTracer::Scope::SCAVENGER_SCAVENGE_ROOTS] = 1;
   tracer->current_.scopes[GCTracer::Scope::SCAVENGER_SCAVENGE_PARALLEL] = 2;
-  tracer->RecordGCPhasesHistograms(i_isolate()->counters()->gc_scavenger());
+  tracer->RecordGCPhasesHistograms(
+      GCTracer::RecordGCPhasesInfo::Mode::Scavenger);
   EXPECT_EQ(1, GcHistogram::Get("V8.GCScavenger.ScavengeRoots")->Total());
   EXPECT_EQ(2, GcHistogram::Get("V8.GCScavenger.ScavengeMain")->Total());
   GcHistogram::CleanUp();