heap: Remove context disposal GCs

mlippautz · Commit Bot · commit ced669daf98e · 2021-04-06T09:19:09.000Z
Full GCs on non-main-frame context disposals show up on real-world web workloads and often cause missed frames. Remove and let the regular scheduler take over these workloads. Bug: chromium:1191325 Change-Id: Ib58419e4623c096321860db05c36ddf9c8e9f4e4 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2773347 Commit-Queue: Michael Lippautz <mlippautz@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Cr-Commit-Position: refs/heads/master@{#73799}
diff --git a/src/heap/gc-idle-time-handler.cc b/src/heap/gc-idle-time-handler.cc
@@ -12,11 +12,8 @@ namespace v8 {
 namespace internal {
 
 const double GCIdleTimeHandler::kConservativeTimeRatio = 0.9;
-const double GCIdleTimeHandler::kHighContextDisposalRate = 100;
 
 void GCIdleTimeHeapState::Print() {
-  PrintF("contexts_disposed=%d ", contexts_disposed);
-  PrintF("contexts_disposal_rate=%f ", contexts_disposal_rate);
   PrintF("size_of_objects=%zu ", size_of_objects);
   PrintF("incremental_marking_stopped=%d ", incremental_marking_stopped);
 }
@@ -36,14 +33,6 @@ size_t GCIdleTimeHandler::EstimateMarkingStepSize(
   return static_cast<size_t>(marking_step_size * kConservativeTimeRatio);
 }
 
-bool GCIdleTimeHandler::ShouldDoContextDisposalMarkCompact(
-    int contexts_disposed, double contexts_disposal_rate,
-    size_t size_of_objects) {
-  return contexts_disposed > 0 && contexts_disposal_rate > 0 &&
-         contexts_disposal_rate < kHighContextDisposalRate &&
-         size_of_objects <= kMaxHeapSizeForContextDisposalMarkCompact;
-}
-
 // The following logic is implemented by the controller:
 // (1) If we don't have any idle time, do nothing, unless a context was
 // disposed, incremental marking is stopped, and the heap is small. Then do
@@ -54,13 +43,6 @@ bool GCIdleTimeHandler::ShouldDoContextDisposalMarkCompact(
 GCIdleTimeAction GCIdleTimeHandler::Compute(double idle_time_in_ms,
                                             GCIdleTimeHeapState heap_state) {
   if (static_cast<int>(idle_time_in_ms) <= 0) {
-    if (heap_state.incremental_marking_stopped) {
-      if (ShouldDoContextDisposalMarkCompact(heap_state.contexts_disposed,
-                                             heap_state.contexts_disposal_rate,
-                                             heap_state.size_of_objects)) {
-        return GCIdleTimeAction::kFullGC;
-      }
-    }
     return GCIdleTimeAction::kDone;
   }
 
diff --git a/src/heap/gc-idle-time-handler.h b/src/heap/gc-idle-time-handler.h
@@ -13,15 +13,12 @@ namespace internal {
 enum class GCIdleTimeAction : uint8_t {
   kDone,
   kIncrementalStep,
-  kFullGC,
 };
 
 class GCIdleTimeHeapState {
  public:
   void Print();
 
-  int contexts_disposed;
-  double contexts_disposal_rate;
   size_t size_of_objects;
   bool incremental_marking_stopped;
 };
@@ -46,11 +43,6 @@ class V8_EXPORT_PRIVATE GCIdleTimeHandler {
   // 16.66 ms when there is currently no rendering going on.
   static const size_t kMaxScheduledIdleTime = 50;
 
-  static const size_t kMaxHeapSizeForContextDisposalMarkCompact = 100 * MB;
-
-  // If contexts are disposed at a higher rate a full gc is triggered.
-  static const double kHighContextDisposalRate;
-
   GCIdleTimeHandler() = default;
   GCIdleTimeHandler(const GCIdleTimeHandler&) = delete;
   GCIdleTimeHandler& operator=(const GCIdleTimeHandler&) = delete;
@@ -65,10 +57,6 @@ class V8_EXPORT_PRIVATE GCIdleTimeHandler {
 
   static double EstimateFinalIncrementalMarkCompactTime(
       size_t size_of_objects, double mark_compact_speed_in_bytes_per_ms);
-
-  static bool ShouldDoContextDisposalMarkCompact(int context_disposed,
-                                                 double contexts_disposal_rate,
-                                                 size_t size_of_objects);
 };
 
 }  // namespace internal
diff --git a/src/heap/gc-tracer.cc b/src/heap/gc-tracer.cc
@@ -208,7 +208,6 @@ void GCTracer::ResetForTesting() {
   recorded_new_generation_allocations_.Reset();
   recorded_old_generation_allocations_.Reset();
   recorded_embedder_generation_allocations_.Reset();
-  recorded_context_disposal_times_.Reset();
   recorded_survival_ratios_.Reset();
   start_counter_ = 0;
   average_mutator_duration_ = 0;
@@ -472,11 +471,6 @@ void GCTracer::AddAllocation(double current_ms) {
   embedder_allocation_in_bytes_since_gc_ = 0;
 }
 
-
-void GCTracer::AddContextDisposalTime(double time) {
-  recorded_context_disposal_times_.Push(time);
-}
-
 void GCTracer::AddCompactionEvent(double duration,
                                   size_t live_bytes_compacted) {
   recorded_compactions_.Push(
@@ -612,8 +606,7 @@ void GCTracer::PrintNVP() const {
           "promotion_rate=%.1f%% "
           "semi_space_copy_rate=%.1f%% "
           "new_space_allocation_throughput=%.1f "
-          "unmapper_chunks=%d "
-          "context_disposal_rate=%.1f\n",
+          "unmapper_chunks=%d\n",
           duration, spent_in_mutator, current_.TypeName(true),
           current_.reduce_memory, current_.scopes[Scope::TIME_TO_SAFEPOINT],
           current_.scopes[Scope::HEAP_PROLOGUE],
@@ -651,8 +644,7 @@ void GCTracer::PrintNVP() const {
           AverageSurvivalRatio(), heap_->promotion_rate_,
           heap_->semi_space_copied_rate_,
           NewSpaceAllocationThroughputInBytesPerMillisecond(),
-          heap_->memory_allocator()->unmapper()->NumberOfChunks(),
-          ContextDisposalRateInMilliseconds());
+          heap_->memory_allocator()->unmapper()->NumberOfChunks());
       break;
     case Event::MINOR_MARK_COMPACTOR:
       heap_->isolate()->PrintWithTimestamp(
@@ -804,7 +796,6 @@ void GCTracer::PrintNVP() const {
           "semi_space_copy_rate=%.1f%% "
           "new_space_allocation_throughput=%.1f "
           "unmapper_chunks=%d "
-          "context_disposal_rate=%.1f "
           "compaction_speed=%.f\n",
           duration, spent_in_mutator, current_.TypeName(true),
           current_.reduce_memory, current_.scopes[Scope::TIME_TO_SAFEPOINT],
@@ -896,7 +887,6 @@ void GCTracer::PrintNVP() const {
           heap_->semi_space_copied_rate_,
           NewSpaceAllocationThroughputInBytesPerMillisecond(),
           heap_->memory_allocator()->unmapper()->NumberOfChunks(),
-          ContextDisposalRateInMilliseconds(),
           CompactionSpeedInBytesPerMillisecond());
       break;
     case Event::START:
@@ -1118,16 +1108,6 @@ double GCTracer::CurrentEmbedderAllocationThroughputInBytesPerMillisecond()
       kThroughputTimeFrameMs);
 }
 
-double GCTracer::ContextDisposalRateInMilliseconds() const {
-  if (recorded_context_disposal_times_.Count() <
-      recorded_context_disposal_times_.kSize)
-    return 0.0;
-  double begin = heap_->MonotonicallyIncreasingTimeInMs();
-  double end = recorded_context_disposal_times_.Sum(
-      [](double a, double b) { return b; }, 0.0);
-  return (begin - end) / recorded_context_disposal_times_.Count();
-}
-
 double GCTracer::AverageSurvivalRatio() const {
   if (recorded_survival_ratios_.Count() == 0) return 0.0;
   double sum = recorded_survival_ratios_.Sum(
diff --git a/src/heap/gc-tracer.h b/src/heap/gc-tracer.h
@@ -221,8 +221,6 @@ class V8_EXPORT_PRIVATE GCTracer {
   // Log the accumulated new space allocation bytes.
   void AddAllocation(double current_ms);
 
-  void AddContextDisposalTime(double time);
-
   void AddCompactionEvent(double duration, size_t live_bytes_compacted);
 
   void AddSurvivalRatio(double survival_ratio);
@@ -297,12 +295,6 @@ class V8_EXPORT_PRIVATE GCTracer {
   // Returns 0 if no allocation events have been recorded.
   double CurrentEmbedderAllocationThroughputInBytesPerMillisecond() const;
 
-  // Computes the context disposal rate in milliseconds. It takes the time
-  // frame of the first recorded context disposal to the current time and
-  // divides it by the number of recorded events.
-  // Returns 0 if no events have been recorded.
-  double ContextDisposalRateInMilliseconds() const;
-
   // Computes the average survival ratio based on the last recorded survival
   // events.
   // Returns 0 if no events have been recorded.
@@ -479,7 +471,6 @@ class V8_EXPORT_PRIVATE GCTracer {
   base::RingBuffer<BytesAndDuration> recorded_new_generation_allocations_;
   base::RingBuffer<BytesAndDuration> recorded_old_generation_allocations_;
   base::RingBuffer<BytesAndDuration> recorded_embedder_generation_allocations_;
-  base::RingBuffer<double> recorded_context_disposal_times_;
   base::RingBuffer<double> recorded_survival_ratios_;
 
   base::Mutex background_counter_mutex_;
diff --git a/src/heap/heap.cc b/src/heap/heap.cc
@@ -1750,8 +1750,6 @@ int Heap::NotifyContextDisposed(bool dependant_context) {
     isolate()->raw_native_context().set_retained_maps(
         ReadOnlyRoots(this).empty_weak_array_list());
   }
-
-  tracer()->AddContextDisposalTime(MonotonicallyIncreasingTimeInMs());
   return ++contexts_disposed_;
 }
 
@@ -2252,6 +2250,7 @@ void Heap::MarkCompact() {
   mark_compact_collector()->Prepare();
 
   ms_count_++;
+  contexts_disposed_ = 0;
 
   MarkCompactPrologue();
 
@@ -3623,9 +3622,6 @@ void Heap::VerifyObjectLayoutChange(HeapObject object, Map new_map) {
 
 GCIdleTimeHeapState Heap::ComputeHeapState() {
   GCIdleTimeHeapState heap_state;
-  heap_state.contexts_disposed = contexts_disposed_;
-  heap_state.contexts_disposal_rate =
-      tracer()->ContextDisposalRateInMilliseconds();
   heap_state.size_of_objects = static_cast<size_t>(SizeOfObjects());
   heap_state.incremental_marking_stopped = incremental_marking()->IsStopped();
   return heap_state;
@@ -3648,13 +3644,6 @@ bool Heap::PerformIdleTimeAction(GCIdleTimeAction action,
       result = incremental_marking()->IsStopped();
       break;
     }
-    case GCIdleTimeAction::kFullGC: {
-      DCHECK_LT(0, contexts_disposed_);
-      HistogramTimerScope scope(isolate_->counters()->gc_context());
-      TRACE_EVENT0("v8", "V8.GCContext");
-      CollectAllGarbage(kNoGCFlags, GarbageCollectionReason::kContextDisposal);
-      break;
-    }
   }
 
   return result;
@@ -3668,8 +3657,6 @@ void Heap::IdleNotificationEpilogue(GCIdleTimeAction action,
   last_idle_notification_time_ = current_time;
   double deadline_difference = deadline_in_ms - current_time;
 
-  contexts_disposed_ = 0;
-
   if (FLAG_trace_idle_notification) {
     isolate_->PrintWithTimestamp(
         "Idle notification: requested idle time %.2f ms, used idle time %.2f "
@@ -3683,9 +3670,6 @@ void Heap::IdleNotificationEpilogue(GCIdleTimeAction action,
       case GCIdleTimeAction::kIncrementalStep:
         PrintF("incremental step");
         break;
-      case GCIdleTimeAction::kFullGC:
-        PrintF("full GC");
-        break;
     }
     PrintF("]");
     if (FLAG_trace_idle_notification_verbose) {
@@ -3727,12 +3711,9 @@ bool Heap::IdleNotification(double deadline_in_seconds) {
                              EmbedderAllocationCounter());
 
   GCIdleTimeHeapState heap_state = ComputeHeapState();
-
   GCIdleTimeAction action =
       gc_idle_time_handler_->Compute(idle_time_in_ms, heap_state);
-
   bool result = PerformIdleTimeAction(action, heap_state, deadline_in_ms);
-
   IdleNotificationEpilogue(action, heap_state, start_ms, deadline_in_ms);
   return result;
 }
diff --git a/src/logging/counters-definitions.h b/src/logging/counters-definitions.h
@@ -100,8 +100,6 @@ namespace internal {
 #define HISTOGRAM_TIMER_LIST(HT)                                               \
   /* Timer histograms, not thread safe: HT(name, caption, max, unit) */        \
   /* Garbage collection timers. */                                             \
-  HT(gc_context, V8.GCContext, 10000,                                          \
-     MILLISECOND) /* GC context cleanup time */                                \
   HT(gc_idle_notification, V8.GCIdleNotification, 10000, MILLISECOND)          \
   HT(gc_incremental_marking, V8.GCIncrementalMarking, 10000, MILLISECOND)      \
   HT(gc_incremental_marking_start, V8.GCIncrementalMarkingStart, 10000,        \
diff --git a/test/unittests/heap/gc-idle-time-handler-unittest.cc b/test/unittests/heap/gc-idle-time-handler-unittest.cc
@@ -21,8 +21,6 @@ class GCIdleTimeHandlerTest : public ::testing::Test {
 
   GCIdleTimeHeapState DefaultHeapState() {
     GCIdleTimeHeapState result;
-    result.contexts_disposed = 0;
-    result.contexts_disposal_rate = GCIdleTimeHandler::kHighContextDisposalRate;
     result.incremental_marking_stopped = false;
     result.size_of_objects = kSizeOfObjects;
     return result;
@@ -72,80 +70,6 @@ TEST(GCIdleTimeHandler, EstimateMarkingStepSizeOverflow2) {
             step_size);
 }
 
-
-TEST_F(GCIdleTimeHandlerTest, ContextDisposeLowRate) {
-  if (!handler()->Enabled()) return;
-  GCIdleTimeHeapState heap_state = DefaultHeapState();
-  heap_state.contexts_disposed = 1;
-  heap_state.incremental_marking_stopped = true;
-  double idle_time_ms = 0;
-  EXPECT_EQ(GCIdleTimeAction::kDone,
-            handler()->Compute(idle_time_ms, heap_state));
-}
-
-
-TEST_F(GCIdleTimeHandlerTest, ContextDisposeHighRate) {
-  if (!handler()->Enabled()) return;
-  GCIdleTimeHeapState heap_state = DefaultHeapState();
-  heap_state.contexts_disposed = 1;
-  heap_state.contexts_disposal_rate =
-      GCIdleTimeHandler::kHighContextDisposalRate - 1;
-  heap_state.incremental_marking_stopped = true;
-  double idle_time_ms = 0;
-  EXPECT_EQ(GCIdleTimeAction::kFullGC,
-            handler()->Compute(idle_time_ms, heap_state));
-}
-
-
-TEST_F(GCIdleTimeHandlerTest, AfterContextDisposeZeroIdleTime) {
-  if (!handler()->Enabled()) return;
-  GCIdleTimeHeapState heap_state = DefaultHeapState();
-  heap_state.contexts_disposed = 1;
-  heap_state.contexts_disposal_rate = 1.0;
-  heap_state.incremental_marking_stopped = true;
-  double idle_time_ms = 0;
-  EXPECT_EQ(GCIdleTimeAction::kFullGC,
-            handler()->Compute(idle_time_ms, heap_state));
-}
-
-
-TEST_F(GCIdleTimeHandlerTest, AfterContextDisposeSmallIdleTime1) {
-  if (!handler()->Enabled()) return;
-  GCIdleTimeHeapState heap_state = DefaultHeapState();
-  heap_state.contexts_disposed = 1;
-  heap_state.contexts_disposal_rate =
-      GCIdleTimeHandler::kHighContextDisposalRate;
-  size_t speed = kMarkCompactSpeed;
-  double idle_time_ms = static_cast<double>(kSizeOfObjects / speed - 1);
-  EXPECT_EQ(GCIdleTimeAction::kIncrementalStep,
-            handler()->Compute(idle_time_ms, heap_state));
-}
-
-
-TEST_F(GCIdleTimeHandlerTest, AfterContextDisposeSmallIdleTime2) {
-  if (!handler()->Enabled()) return;
-  GCIdleTimeHeapState heap_state = DefaultHeapState();
-  heap_state.contexts_disposed = 1;
-  heap_state.contexts_disposal_rate =
-      GCIdleTimeHandler::kHighContextDisposalRate;
-  size_t speed = kMarkCompactSpeed;
-  double idle_time_ms = static_cast<double>(kSizeOfObjects / speed - 1);
-  EXPECT_EQ(GCIdleTimeAction::kIncrementalStep,
-            handler()->Compute(idle_time_ms, heap_state));
-}
-
-TEST_F(GCIdleTimeHandlerTest, AfterContextDisposeLargeHeap) {
-  if (!handler()->Enabled()) return;
-  GCIdleTimeHeapState heap_state = DefaultHeapState();
-  heap_state.contexts_disposed = 1;
-  heap_state.contexts_disposal_rate = 1.0;
-  heap_state.incremental_marking_stopped = true;
-  heap_state.size_of_objects = 101 * MB;
-  double idle_time_ms = 0;
-  EXPECT_EQ(GCIdleTimeAction::kDone,
-            handler()->Compute(idle_time_ms, heap_state));
-}
-
 TEST_F(GCIdleTimeHandlerTest, IncrementalMarking1) {
   if (!handler()->Enabled()) return;
   GCIdleTimeHeapState heap_state = DefaultHeapState();
