Restore NothingOrDone action in idle time handler.

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

Index: src/heap/gc-idle-time-handler.cc
diff --git a/src/heap/gc-idle-time-handler.cc b/src/heap/gc-idle-time-handler.cc
index 40c2eff3f96c56ff8e63cc815029a13704545b82..352593e6c42605b3194e8eb22717614bf1399b48 100644
--- a/src/heap/gc-idle-time-handler.cc
+++ b/src/heap/gc-idle-time-handler.cc
@@ -192,6 +192,17 @@ bool GCIdleTimeHandler::ShouldDoOverApproximateWeakClosure(
 }
 
 
+GCIdleTimeAction GCIdleTimeHandler::NothingOrDone() {
+  if (idle_times_which_made_no_progress_per_mode_ >=
+      kMaxNoProgressIdleTimesPerMode) {
+    return GCIdleTimeAction::Done();
+  } else {
+    idle_times_which_made_no_progress_per_mode_++;
+    return GCIdleTimeAction::Nothing();
+  }
+}
+
+
 // The idle time handler has three modes and transitions between them
 // as shown in the diagram:
 //
@@ -306,13 +317,13 @@ GCIdleTimeAction GCIdleTimeHandler::Action(double idle_time_in_ms,
     if (heap_state.sweeping_completed) {
       return GCIdleTimeAction::FinalizeSweeping();
     } else {
-      return GCIdleTimeAction::Nothing();
+      return NothingOrDone();
     }
   }
 
   if (heap_state.incremental_marking_stopped &&
       !heap_state.can_start_incremental_marking && !reduce_memory) {
-    return GCIdleTimeAction::Nothing();
+    return NothingOrDone();
   }
 
   size_t step_size = EstimateMarkingStepSize(
@@ -324,19 +335,20 @@ GCIdleTimeAction GCIdleTimeHandler::Action(double idle_time_in_ms,
 
 void GCIdleTimeHandler::UpdateCounters(double idle_time_in_ms) {
   if (mode_ == kReduceLatency) {
-    int mutator_gcs = scavenges_ + mark_compacts_ - idle_mark_compacts_;
-    if (mutator_gcs > 0) {
+    int gcs = scavenges_ + mark_compacts_;
+    if (gcs > 0) {
       // There was a mutator GC since the last notification.
       long_idle_notifications_ = 0;
     }
     idle_mark_compacts_ = 0;
     mark_compacts_ = 0;
     scavenges_ = 0;
+    // Go to reduce memory mode after two large idle notifications.
+    const int kLargeIdleIncrement =
+        kLongIdleNotificationsBeforeMutatorIsIdle / 2;

[rmcilroy, 2015/05/19 10:26:57]

I don't understand this - what is kLongIdleNotificationsBeforeMutatorIsIdle
representing and why do we divide it by two here?

[ulan, 2015/05/19 11:21:52]

This was trying to encode two different counters: one for foreground tab and one
for background tab. For background tab we want to go faster in reduce memory
mode. I made it explicit in the new patch set.

[rmcilroy, 2015/05/19 14:36:33]

This is clearer, thanks.

     if (idle_time_in_ms >= kMinLongIdleTime) {
       long_idle_notifications_ +=
-          (idle_time_in_ms >= kLargeLongIdleTime)
-              ? kLongIdleNotificationsBeforeMutatorIsIdle
-              : 1;
+          (idle_time_in_ms >= kLargeLongIdleTime) ? kLargeIdleIncrement : 1;
     }
   }
 }
@@ -347,11 +359,14 @@ void GCIdleTimeHandler::ResetCounters() {
   idle_mark_compacts_ = 0;
   mark_compacts_ = 0;
   scavenges_ = 0;
+  idle_times_which_made_no_progress_per_mode_ = 0;
 }
 
 
-bool GCIdleTimeHandler::IsMutatorActive(int contexts_disposed, int gcs) {
-  return contexts_disposed > 0 || gcs >= kGCsBeforeMutatorIsActive;
+bool GCIdleTimeHandler::IsMutatorActive(int contexts_disposed,
+                                        int mark_compacts) {
+  return contexts_disposed > 0 ||
+         mark_compacts >= kMarkCompactsBeforeMutatorIsActive;
 }
 
 
@@ -368,7 +383,7 @@ GCIdleTimeHandler::Mode GCIdleTimeHandler::NextMode(
   switch (mode_) {
     case kDone:
       DCHECK(idle_mark_compacts_ == 0);
-      if (IsMutatorActive(heap_state.contexts_disposed, mutator_gcs)) {
+      if (IsMutatorActive(heap_state.contexts_disposed, mark_compacts_)) {
         return kReduceLatency;
       }
       break;