Ensure that GC idle notifications either make progress or stop requesting more GCs.

The V8::IdleNotification will only return 'True' when the gc idle time handler
thinks there is no more GC which can be done. However, the gc idle task handler
can end up repeatedly making no progress (e.g., if it can't finalize a sweep)
which causes idle tasks to be repeatedly scheduled in Chrome which do nothing
but wake up Chrome. Fix this by returning Done if we can't make any progress
within an Idle Round.

BUG=chromium:470615
LOG=Y

Review URL: https://codereview.chromium.org/1042483002

Cr-Commit-Position: refs/heads/master@{#27529}

src/heap/gc-idle-time-handler.cc

@@ -187,11 +187,22 @@ bool GCIdleTimeHandler::ShouldDoOverApproximateWeakClosure(
 }
 
 
+GCIdleTimeAction GCIdleTimeHandler::NothingOrDone() {
+  if (idle_times_which_made_no_progress_since_last_idle_round_ >=
+      kMaxNoProgressIdleTimesPerIdleRound) {
+    return GCIdleTimeAction::Done();
+  } else {
+    idle_times_which_made_no_progress_since_last_idle_round_++;
+    return GCIdleTimeAction::Nothing();
+  }
+}
+
+
 // 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
 // a full GC.
-// (2) If the new space is almost full and we can affort a Scavenge or if the
+// (2) If the new space is almost full and we can afford a Scavenge or if the
 // next Scavenge will very likely take long, then a Scavenge is performed.
 // (3) If there is currently no MarkCompact idle round going on, we start a
 // new idle round if enough garbage was created. Otherwise we do not perform
@@ -243,15 +254,18 @@ GCIdleTimeAction GCIdleTimeHandler::Compute(double idle_time_in_ms,
       return GCIdleTimeAction::FullGC();
     }
   }
-
-  if (heap_state.sweeping_in_progress && heap_state.sweeping_completed) {
-    return GCIdleTimeAction::FinalizeSweeping();
+  if (heap_state.sweeping_in_progress) {
+    if (heap_state.sweeping_completed) {
+      return GCIdleTimeAction::FinalizeSweeping();
+    } else {
+      return NothingOrDone();
+    }
   }
-
   if (heap_state.incremental_marking_stopped &&
       !heap_state.can_start_incremental_marking) {
-    return GCIdleTimeAction::Nothing();
+    return NothingOrDone();
   }
+
   size_t step_size = EstimateMarkingStepSize(
       static_cast<size_t>(kIncrementalMarkingStepTimeInMs),
       heap_state.incremental_marking_speed_in_bytes_per_ms);

src/heap/gc-idle-time-handler.h

@@ -138,6 +138,11 @@ class GCIdleTimeHandler {
 
   static const size_t kMinTimeForOverApproximatingWeakClosureInMs;
 
+  // Number of times we will return a Nothing action per Idle round despite
+  // having idle time available before we returning a Done action to ensure we
+  // don't keep scheduling idle tasks and making no progress.
+  static const int kMaxNoProgressIdleTimesPerIdleRound = 10;
+
   class HeapState {
    public:
     void Print();
@@ -160,7 +165,8 @@ class GCIdleTimeHandler {
 
   GCIdleTimeHandler()
       : mark_compacts_since_idle_round_started_(0),
-        scavenges_since_last_idle_round_(0) {}
+        scavenges_since_last_idle_round_(0),
+        idle_times_which_made_no_progress_since_last_idle_round_(0) {}
 
   GCIdleTimeAction Compute(double idle_time_in_ms, HeapState heap_state);
 
@@ -204,7 +210,12 @@ class GCIdleTimeHandler {
       size_t new_space_allocation_throughput_in_bytes_per_ms);
 
  private:
-  void StartIdleRound() { mark_compacts_since_idle_round_started_ = 0; }
+  GCIdleTimeAction NothingOrDone();
+
+  void StartIdleRound() {
+    mark_compacts_since_idle_round_started_ = 0;
+    idle_times_which_made_no_progress_since_last_idle_round_ = 0;
+  }
   bool IsMarkCompactIdleRoundFinished() {
     return mark_compacts_since_idle_round_started_ ==
            kMaxMarkCompactsInIdleRound;
@@ -215,6 +226,7 @@ class GCIdleTimeHandler {
 
   int mark_compacts_since_idle_round_started_;
   int scavenges_since_last_idle_round_;
+  int idle_times_which_made_no_progress_since_last_idle_round_;
 
   DISALLOW_COPY_AND_ASSIGN(GCIdleTimeHandler);
 };

test/unittests/heap/gc-idle-time-handler-unittest.cc

@@ -465,5 +465,56 @@ TEST_F(GCIdleTimeHandlerTest, ZeroIdleTimeDoNothingButStartIdleRound) {
   EXPECT_EQ(DO_NOTHING, action.type);
 }
 
+
+TEST_F(GCIdleTimeHandlerTest, KeepDoingDoNothingWithZeroIdleTime) {
+  GCIdleTimeHandler::HeapState heap_state = DefaultHeapState();
+  for (int i = 0; i < GCIdleTimeHandler::kMaxNoProgressIdleTimesPerIdleRound;
+       i++) {
+    GCIdleTimeAction action = handler()->Compute(0, heap_state);
+    EXPECT_EQ(DO_NOTHING, action.type);
+  }
+  // Should still return DO_NOTHING if we have been given 0 deadline yet.
+  GCIdleTimeAction action = handler()->Compute(0, heap_state);
+  EXPECT_EQ(DO_NOTHING, action.type);
+}
+
+
+TEST_F(GCIdleTimeHandlerTest, DoneIfNotMakingProgressOnSweeping) {
+  GCIdleTimeHandler::HeapState heap_state = DefaultHeapState();
+
+  // Simulate sweeping being in-progress but not complete.
+  heap_state.incremental_marking_stopped = true;
+  heap_state.can_start_incremental_marking = false;
+  heap_state.sweeping_in_progress = true;
+  heap_state.sweeping_completed = false;
+  double idle_time_ms = 10.0;
+  for (int i = 0; i < GCIdleTimeHandler::kMaxNoProgressIdleTimesPerIdleRound;
+       i++) {
+    GCIdleTimeAction action = handler()->Compute(idle_time_ms, heap_state);
+    EXPECT_EQ(DO_NOTHING, action.type);
+  }
+  // We should return DONE after not making progress for some time.
+  GCIdleTimeAction action = handler()->Compute(idle_time_ms, heap_state);
+  EXPECT_EQ(DONE, action.type);
+}
+
+
+TEST_F(GCIdleTimeHandlerTest, DoneIfNotMakingProgressOnIncrementalMarking) {
+  GCIdleTimeHandler::HeapState heap_state = DefaultHeapState();
+
+  // Simulate incremental marking stopped and not eligible to start.
+  heap_state.incremental_marking_stopped = true;
+  heap_state.can_start_incremental_marking = false;
+  double idle_time_ms = 10.0;
+  for (int i = 0; i < GCIdleTimeHandler::kMaxNoProgressIdleTimesPerIdleRound;
+       i++) {
+    GCIdleTimeAction action = handler()->Compute(idle_time_ms, heap_state);
+    EXPECT_EQ(DO_NOTHING, action.type);
+  }
+  // We should return DONE after not making progress for some time.
+  GCIdleTimeAction action = handler()->Compute(idle_time_ms, heap_state);
+  EXPECT_EQ(DONE, action.type);
+}
+
 }  // namespace internal
 }  // namespace v8