[heap] Add a guard for restarting the memory reducer after mark-compact.

Currently it is possible to get into a cycle of
mark-compact -> memory reducer -> mark-compact -> memory reducer ...
where the memory reducer does not free memory.

This patch ensures that the memory reducer restarts only if the
committed memory increased by sufficient amount after the last run.

BUG=

Review-Url: https://chromiumcodereview.appspot.com/2433933005
Cr-Commit-Position: refs/heads/master@{#40457}

src/heap/heap.cc

@@ -1016,6 +1016,7 @@ bool Heap::CollectGarbage(GarbageCollector collector,
           (committed_memory_before - committed_memory_after) > MB ||
           HasHighFragmentation(used_memory_after, committed_memory_after) ||
           (detached_contexts()->length() > 0);
+      event.committed_memory = committed_memory_after;
       if (deserialization_complete_) {
         memory_reducer_->NotifyMarkCompact(event);
       }

src/heap/memory-reducer.cc

@@ -17,6 +17,8 @@ const int MemoryReducer::kLongDelayMs = 8000;
 const int MemoryReducer::kShortDelayMs = 500;
 const int MemoryReducer::kWatchdogDelayMs = 100000;
 const int MemoryReducer::kMaxNumberOfGCs = 3;
+const double MemoryReducer::kCommittedMemoryFactor = 1.1;
+const size_t MemoryReducer::kCommittedMemoryDelta = 10 * MB;
 
 MemoryReducer::TimerTask::TimerTask(MemoryReducer* memory_reducer)
     : CancelableTask(memory_reducer->heap()->isolate()),
@@ -48,6 +50,7 @@ void MemoryReducer::TimerTask::RunInternal() {
   event.can_start_incremental_gc =
       heap->incremental_marking()->IsStopped() &&
       (heap->incremental_marking()->CanBeActivated() || optimize_for_memory);
+  event.committed_memory = heap->CommittedOldGenerationMemory();
   memory_reducer_->NotifyTimer(event);
 }
 
@@ -138,17 +141,30 @@ bool MemoryReducer::WatchdogGC(const State& state, const Event& event) {
 MemoryReducer::State MemoryReducer::Step(const State& state,
                                          const Event& event) {
   if (!FLAG_incremental_marking || !FLAG_memory_reducer) {
-    return State(kDone, 0, 0, state.last_gc_time_ms);
+    return State(kDone, 0, 0, state.last_gc_time_ms, 0);
   }
   switch (state.action) {
     case kDone:
       if (event.type == kTimer) {
         return state;
+      } else if (event.type == kMarkCompact) {
+        if (event.committed_memory <
+            Max(static_cast<size_t>(state.committed_memory_at_last_run *
+                                    kCommittedMemoryFactor),
+                state.committed_memory_at_last_run + kCommittedMemoryDelta)) {
+          return state;
+        } else {
+          return State(kWait, 0, event.time_ms + kLongDelayMs,
+                       event.type == kMarkCompact ? event.time_ms
+                                                  : state.last_gc_time_ms,
+                       0);
+        }
       } else {
-        DCHECK(event.type == kPossibleGarbage || event.type == kMarkCompact);
+        DCHECK_EQ(kPossibleGarbage, event.type);
         return State(
             kWait, 0, event.time_ms + kLongDelayMs,
-            event.type == kMarkCompact ? event.time_ms : state.last_gc_time_ms);
+            event.type == kMarkCompact ? event.time_ms : state.last_gc_time_ms,
+            0);
       }
     case kWait:
       switch (event.type) {
@@ -156,23 +172,24 @@ MemoryReducer::State MemoryReducer::Step(const State& state,
           return state;
         case kTimer:
           if (state.started_gcs >= kMaxNumberOfGCs) {
-            return State(kDone, kMaxNumberOfGCs, 0.0, state.last_gc_time_ms);
+            return State(kDone, kMaxNumberOfGCs, 0.0, state.last_gc_time_ms,
+                         event.committed_memory);
           } else if (event.can_start_incremental_gc &&
                      (event.should_start_incremental_gc ||
                       WatchdogGC(state, event))) {
             if (state.next_gc_start_ms <= event.time_ms) {
               return State(kRun, state.started_gcs + 1, 0.0,
-                           state.last_gc_time_ms);
+                           state.last_gc_time_ms, 0);
             } else {
               return state;
             }
           } else {
             return State(kWait, state.started_gcs, event.time_ms + kLongDelayMs,
-                         state.last_gc_time_ms);
+                         state.last_gc_time_ms, 0);
           }
         case kMarkCompact:
           return State(kWait, state.started_gcs, event.time_ms + kLongDelayMs,
-                       event.time_ms);
+                       event.time_ms, 0);
       }
     case kRun:
       if (event.type != kMarkCompact) {
@@ -181,14 +198,15 @@ MemoryReducer::State MemoryReducer::Step(const State& state,
         if (state.started_gcs < kMaxNumberOfGCs &&
             (event.next_gc_likely_to_collect_more || state.started_gcs == 1)) {
           return State(kWait, state.started_gcs, event.time_ms + kShortDelayMs,
-                       event.time_ms);
+                       event.time_ms, 0);
         } else {
-          return State(kDone, kMaxNumberOfGCs, 0.0, event.time_ms);
+          return State(kDone, kMaxNumberOfGCs, 0.0, event.time_ms,
+                       event.committed_memory);
         }
       }
   }
   UNREACHABLE();
-  return State(kDone, 0, 0, 0.0);  // Make the compiler happy.
+  return State(kDone, 0, 0, 0.0, 0);  // Make the compiler happy.
 }
 
 
@@ -204,8 +222,7 @@ void MemoryReducer::ScheduleTimer(double time_ms, double delay_ms) {
       isolate, timer_task, (delay_ms + kSlackMs) / 1000.0);
 }
 
-
-void MemoryReducer::TearDown() { state_ = State(kDone, 0, 0, 0.0); }
+void MemoryReducer::TearDown() { state_ = State(kDone, 0, 0, 0.0, 0); }
 
 }  // namespace internal
 }  // namespace v8

src/heap/memory-reducer.h

@@ -86,15 +86,17 @@ class V8_EXPORT_PRIVATE MemoryReducer {
 
   struct State {
     State(Action action, int started_gcs, double next_gc_start_ms,
-          double last_gc_time_ms)
+          double last_gc_time_ms, size_t committed_memory_at_last_run)
         : action(action),
           started_gcs(started_gcs),
           next_gc_start_ms(next_gc_start_ms),
-          last_gc_time_ms(last_gc_time_ms) {}
+          last_gc_time_ms(last_gc_time_ms),
+          committed_memory_at_last_run(committed_memory_at_last_run) {}
     Action action;
     int started_gcs;
     double next_gc_start_ms;
     double last_gc_time_ms;
+    size_t committed_memory_at_last_run;
   };
 
   enum EventType { kTimer, kMarkCompact, kPossibleGarbage };
@@ -102,6 +104,7 @@ class V8_EXPORT_PRIVATE MemoryReducer {
   struct Event {
     EventType type;
     double time_ms;
+    size_t committed_memory;
     bool next_gc_likely_to_collect_more;
     bool should_start_incremental_gc;
     bool can_start_incremental_gc;
@@ -109,7 +112,7 @@ class V8_EXPORT_PRIVATE MemoryReducer {
 
   explicit MemoryReducer(Heap* heap)
       : heap_(heap),
-        state_(kDone, 0, 0.0, 0.0),
+        state_(kDone, 0, 0.0, 0.0, 0),
         js_calls_counter_(0),
         js_calls_sample_time_ms_(0.0) {}
   // Callbacks.
@@ -126,6 +129,12 @@ class V8_EXPORT_PRIVATE MemoryReducer {
   static const int kShortDelayMs;
   static const int kWatchdogDelayMs;
   static const int kMaxNumberOfGCs;
+  // The committed memory has to increase by at least this factor since the
+  // last run in order to trigger a new run after mark-compact.
+  static const double kCommittedMemoryFactor;
+  // The committed memory has to increase by at least this amount since the
+  // last run in order to trigger a new run after mark-compact.
+  static const size_t kCommittedMemoryDelta;
 
   Heap* heap() { return heap_; }
 

test/unittests/heap/memory-reducer-unittest.cc

@@ -12,39 +12,44 @@ namespace v8 {
 namespace internal {
 
 MemoryReducer::State DoneState() {
-  return MemoryReducer::State(MemoryReducer::kDone, 0, 0.0, 1.0);
+  return MemoryReducer::State(MemoryReducer::kDone, 0, 0.0, 1.0, 0);
 }
 
+MemoryReducer::State DoneState(size_t committed_memory) {
+  return MemoryReducer::State(MemoryReducer::kDone, 0, 0.0, 1.0,
+                              committed_memory);
+}
 
 MemoryReducer::State WaitState(int started_gcs, double next_gc_start_ms) {
   return MemoryReducer::State(MemoryReducer::kWait, started_gcs,
-                              next_gc_start_ms, 1.0);
+                              next_gc_start_ms, 1.0, 0);
 }
 
 
 MemoryReducer::State RunState(int started_gcs, double next_gc_start_ms) {
   return MemoryReducer::State(MemoryReducer::kRun, started_gcs,
-                              next_gc_start_ms, 1.0);
+                              next_gc_start_ms, 1.0, 0);
 }
 
-
 MemoryReducer::Event MarkCompactEvent(double time_ms,
-                                      bool next_gc_likely_to_collect_more) {
+                                      bool next_gc_likely_to_collect_more,
+                                      size_t committed_memory) {
   MemoryReducer::Event event;
   event.type = MemoryReducer::kMarkCompact;
   event.time_ms = time_ms;
   event.next_gc_likely_to_collect_more = next_gc_likely_to_collect_more;
+  event.committed_memory = committed_memory;
   return event;
 }
 
-
-MemoryReducer::Event MarkCompactEventGarbageLeft(double time_ms) {
-  return MarkCompactEvent(time_ms, true);
+MemoryReducer::Event MarkCompactEventGarbageLeft(double time_ms,
+                                                 size_t committed_memory) {
+  return MarkCompactEvent(time_ms, true, committed_memory);
 }
 
-
-MemoryReducer::Event MarkCompactEventNoGarbageLeft(double time_ms) {
-  return MarkCompactEvent(time_ms, false);
+MemoryReducer::Event MarkCompactEventNoGarbageLeft(double time_ms,
+                                                   size_t committed_memory) {
+  return MarkCompactEvent(time_ms, false, committed_memory);
 }
 
 
@@ -93,6 +98,19 @@ TEST(MemoryReducer, FromDoneToDone) {
 
   state1 = MemoryReducer::Step(state0, TimerEventPendingGC(0));
   EXPECT_EQ(MemoryReducer::kDone, state1.action);
+
+  state1 = MemoryReducer::Step(
+      state0,
+      MarkCompactEventGarbageLeft(0, MemoryReducer::kCommittedMemoryDelta - 1));
+  EXPECT_EQ(MemoryReducer::kDone, state1.action);
+
+  state0 = DoneState(1000 * MB);
+  state1 = MemoryReducer::Step(
+      state0, MarkCompactEventGarbageLeft(
+                  0, static_cast<size_t>(
+                         1000 * MB * MemoryReducer::kCommittedMemoryFactor) -
+                         1));
+  EXPECT_EQ(MemoryReducer::kDone, state1.action);
 }
 
 
@@ -101,13 +119,17 @@ TEST(MemoryReducer, FromDoneToWait) {
 
   MemoryReducer::State state0(DoneState()), state1(DoneState());
 
-  state1 = MemoryReducer::Step(state0, MarkCompactEventGarbageLeft(2));
+  state1 = MemoryReducer::Step(
+      state0,
+      MarkCompactEventGarbageLeft(2, MemoryReducer::kCommittedMemoryDelta));
   EXPECT_EQ(MemoryReducer::kWait, state1.action);
   EXPECT_EQ(MemoryReducer::kLongDelayMs + 2, state1.next_gc_start_ms);
   EXPECT_EQ(0, state1.started_gcs);
   EXPECT_EQ(2, state1.last_gc_time_ms);
 
-  state1 = MemoryReducer::Step(state0, MarkCompactEventNoGarbageLeft(2));
+  state1 = MemoryReducer::Step(
+      state0,
+      MarkCompactEventNoGarbageLeft(2, MemoryReducer::kCommittedMemoryDelta));
   EXPECT_EQ(MemoryReducer::kWait, state1.action);
   EXPECT_EQ(MemoryReducer::kLongDelayMs + 2, state1.next_gc_start_ms);
   EXPECT_EQ(0, state1.started_gcs);
@@ -118,6 +140,16 @@ TEST(MemoryReducer, FromDoneToWait) {
   EXPECT_EQ(MemoryReducer::kLongDelayMs, state1.next_gc_start_ms);
   EXPECT_EQ(0, state1.started_gcs);
   EXPECT_EQ(state0.last_gc_time_ms, state1.last_gc_time_ms);
+
+  state0 = DoneState(1000 * MB);
+  state1 = MemoryReducer::Step(
+      state0, MarkCompactEventGarbageLeft(
+                  2, static_cast<size_t>(
+                         1000 * MB * MemoryReducer::kCommittedMemoryFactor)));
+  EXPECT_EQ(MemoryReducer::kWait, state1.action);
+  EXPECT_EQ(MemoryReducer::kLongDelayMs + 2, state1.next_gc_start_ms);
+  EXPECT_EQ(0, state1.started_gcs);
+  EXPECT_EQ(2, state1.last_gc_time_ms);
 }
 
 
@@ -147,13 +179,13 @@ TEST(MemoryReducer, FromWaitToWait) {
   EXPECT_EQ(2000 + MemoryReducer::kLongDelayMs, state1.next_gc_start_ms);
   EXPECT_EQ(state0.started_gcs, state1.started_gcs);
 
-  state1 = MemoryReducer::Step(state0, MarkCompactEventGarbageLeft(2000));
+  state1 = MemoryReducer::Step(state0, MarkCompactEventGarbageLeft(2000, 0));
   EXPECT_EQ(MemoryReducer::kWait, state1.action);
   EXPECT_EQ(2000 + MemoryReducer::kLongDelayMs, state1.next_gc_start_ms);
   EXPECT_EQ(state0.started_gcs, state1.started_gcs);
   EXPECT_EQ(2000, state1.last_gc_time_ms);
 
-  state1 = MemoryReducer::Step(state0, MarkCompactEventNoGarbageLeft(2000));
+  state1 = MemoryReducer::Step(state0, MarkCompactEventNoGarbageLeft(2000, 0));
   EXPECT_EQ(MemoryReducer::kWait, state1.action);
   EXPECT_EQ(2000 + MemoryReducer::kLongDelayMs, state1.next_gc_start_ms);
   EXPECT_EQ(state0.started_gcs, state1.started_gcs);
@@ -262,7 +294,7 @@ TEST(MemoryReducer, FromRunToDone) {
 
   MemoryReducer::State state0(RunState(2, 0.0)), state1(DoneState());
 
-  state1 = MemoryReducer::Step(state0, MarkCompactEventNoGarbageLeft(2000));
+  state1 = MemoryReducer::Step(state0, MarkCompactEventNoGarbageLeft(2000, 0));
   EXPECT_EQ(MemoryReducer::kDone, state1.action);
   EXPECT_EQ(0, state1.next_gc_start_ms);
   EXPECT_EQ(MemoryReducer::kMaxNumberOfGCs, state1.started_gcs);
@@ -270,7 +302,7 @@ TEST(MemoryReducer, FromRunToDone) {
 
   state0.started_gcs = MemoryReducer::kMaxNumberOfGCs;
 
-  state1 = MemoryReducer::Step(state0, MarkCompactEventGarbageLeft(2000));
+  state1 = MemoryReducer::Step(state0, MarkCompactEventGarbageLeft(2000, 0));
   EXPECT_EQ(MemoryReducer::kDone, state1.action);
   EXPECT_EQ(0, state1.next_gc_start_ms);
   EXPECT_EQ(2000, state1.last_gc_time_ms);
@@ -282,7 +314,7 @@ TEST(MemoryReducer, FromRunToWait) {
 
   MemoryReducer::State state0(RunState(2, 0.0)), state1(DoneState());
 
-  state1 = MemoryReducer::Step(state0, MarkCompactEventGarbageLeft(2000));
+  state1 = MemoryReducer::Step(state0, MarkCompactEventGarbageLeft(2000, 0));
   EXPECT_EQ(MemoryReducer::kWait, state1.action);
   EXPECT_EQ(2000 + MemoryReducer::kShortDelayMs, state1.next_gc_start_ms);
   EXPECT_EQ(state0.started_gcs, state1.started_gcs);
@@ -290,7 +322,7 @@ TEST(MemoryReducer, FromRunToWait) {
 
   state0.started_gcs = 1;
 
-  state1 = MemoryReducer::Step(state0, MarkCompactEventNoGarbageLeft(2000));
+  state1 = MemoryReducer::Step(state0, MarkCompactEventNoGarbageLeft(2000, 0));
   EXPECT_EQ(MemoryReducer::kWait, state1.action);
   EXPECT_EQ(2000 + MemoryReducer::kShortDelayMs, state1.next_gc_start_ms);
   EXPECT_EQ(state0.started_gcs, state1.started_gcs);