Restore NothingOrDone action in idle time handler.

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

 // Copyright 2014 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/heap/gc-idle-time-handler.h"
 #include "src/heap/gc-tracer.h"
 #include "src/utils.h"
 
 namespace v8 {
 namespace internal {
@@ skipping 174 matching lines @@
 }
 
 
 bool GCIdleTimeHandler::ShouldDoOverApproximateWeakClosure(
     size_t idle_time_in_ms) {
   // TODO(jochen): Estimate the time it will take to build the object groups.
   return idle_time_in_ms >= kMinTimeForOverApproximatingWeakClosureInMs;
 }
 
 
+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:
 //
 //  kReduceLatency -----> kReduceMemory -----> kDone
 //      ^    ^                  |                |
 //      |    |                  |                |
 //      |    +------------------+                |
 //      |                                        |
 //      +----------------------------------------+
 //
@@ skipping 71 matching lines @@
         return GCIdleTimeAction::FullGC(false);
       }
     }
     return GCIdleTimeAction::Nothing();
   }
 
   // We are in a context disposal GC scenario. Don't do anything if we do not
   // get the right idle signal.
   if (ShouldDoContextDisposalMarkCompact(heap_state.contexts_disposed,
                                          heap_state.contexts_disposal_rate)) {
     return GCIdleTimeAction::Nothing();

[Hannes Payer (out of office), 2015/05/19 09:50:32]

What about this one and the one above? Also nothing or done?

[ulan, 2015/05/19 10:10:34]

Replaced this one with NothingOrDone. Leaving the one above as Nothing() like it
was before refactoring.

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

Just this one I think (the one above should probably still be Nothing since it
only happens if we get a negative idle time).

   }
 
   if (ShouldDoScavenge(
           static_cast<size_t>(idle_time_in_ms), heap_state.new_space_capacity,
           heap_state.used_new_space_size,
           heap_state.scavenge_speed_in_bytes_per_ms,
           heap_state.new_space_allocation_throughput_in_bytes_per_ms)) {
     return GCIdleTimeAction::Scavenge();
   }
 
   if (heap_state.incremental_marking_stopped && reduce_memory) {
     if (ShouldDoMarkCompact(static_cast<size_t>(idle_time_in_ms),
                             heap_state.size_of_objects,
                             heap_state.mark_compact_speed_in_bytes_per_ms)) {
       return GCIdleTimeAction::FullGC(reduce_memory);
     }
   }
 
   if (heap_state.sweeping_in_progress) {
     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(
       static_cast<size_t>(kIncrementalMarkingStepTimeInMs),
       heap_state.incremental_marking_speed_in_bytes_per_ms);
   return GCIdleTimeAction::IncrementalMarking(step_size, reduce_memory);
 }
 
 
 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;
     }
   }
 }
 
 
 void GCIdleTimeHandler::ResetCounters() {
   long_idle_notifications_ = 0;
   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;
 }
 
 
 bool GCIdleTimeHandler::IsMutatorIdle(int long_idle_notifications, int gcs) {
   return gcs == 0 &&
          long_idle_notifications >= kLongIdleNotificationsBeforeMutatorIsIdle;
 }
 
 
 GCIdleTimeHandler::Mode GCIdleTimeHandler::NextMode(
     const HeapState& heap_state) {
   DCHECK(mark_compacts_ >= idle_mark_compacts_);
   int mutator_gcs = scavenges_ + mark_compacts_ - idle_mark_compacts_;
   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;
     case kReduceLatency:
       if (IsMutatorIdle(long_idle_notifications_, mutator_gcs)) {
         return kReduceMemory;
       }
       break;
     case kReduceMemory:
       if (idle_mark_compacts_ >= kMaxIdleMarkCompacts) {
         return kDone;
       }
       if (mutator_gcs > idle_mark_compacts_) {
         return kReduceLatency;
       }
       break;
   }
   return mode_;
 }
 }
 }