Revert of [heap] Add a guard for restarting the memory reducer after...

Revert of [heap] Add a guard for restarting the memory reducer after mark-compact. (patchset #1 id:1 of https://chromiumcodereview.appspot.com/2433933005/ )

Reason for revert:
Speculative revert to see impact on crbug.com/659531

Original issue's description:
> [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=

TBR=hpayer@chromium.org,davidroutier17@gmail.com
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=

Review-Url: https://codereview.chromium.org/2472053003
Cr-Commit-Position: refs/heads/master@{#40737}

src/heap/heap.cc

@@ -1017,7 +1017,6 @@ 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,8 +17,6 @@ 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()),
@@ -50,7 +48,6 @@ 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);
 }
 
@@ -141,30 +138,17 @@ 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, 0);
+    return State(kDone, 0, 0, state.last_gc_time_ms);
   }
   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_EQ(kPossibleGarbage, event.type);
+        DCHECK(event.type == kPossibleGarbage || event.type == kMarkCompact);
         return State(
             kWait, 0, event.time_ms + kLongDelayMs,
-            event.type == kMarkCompact ? event.time_ms : state.last_gc_time_ms,
-            0);
+            event.type == kMarkCompact ? event.time_ms : state.last_gc_time_ms);
       }
     case kWait:
       switch (event.type) {
@@ -172,24 +156,23 @@ 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,
-                         event.committed_memory);
+            return State(kDone, kMaxNumberOfGCs, 0.0, state.last_gc_time_ms);
           } 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, 0);
+                           state.last_gc_time_ms);
             } else {
               return state;
             }
           } else {
             return State(kWait, state.started_gcs, event.time_ms + kLongDelayMs,
-                         state.last_gc_time_ms, 0);
+                         state.last_gc_time_ms);
           }
         case kMarkCompact:
           return State(kWait, state.started_gcs, event.time_ms + kLongDelayMs,
-                       event.time_ms, 0);
+                       event.time_ms);
       }
     case kRun:
       if (event.type != kMarkCompact) {
@@ -198,15 +181,14 @@ 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, 0);
+                       event.time_ms);
         } else {
-          return State(kDone, kMaxNumberOfGCs, 0.0, event.time_ms,
-                       event.committed_memory);
+          return State(kDone, kMaxNumberOfGCs, 0.0, event.time_ms);
         }
       }
   }
   UNREACHABLE();
-  return State(kDone, 0, 0, 0.0, 0);  // Make the compiler happy.
+  return State(kDone, 0, 0, 0.0);  // Make the compiler happy.
 }
 
 
@@ -222,7 +204,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, 0); }
+void MemoryReducer::TearDown() { state_ = State(kDone, 0, 0, 0.0); }
 
 }  // namespace internal
 }  // namespace v8

src/heap/memory-reducer.h

@@ -86,17 +86,15 @@ class V8_EXPORT_PRIVATE MemoryReducer {
 
   struct State {
     State(Action action, int started_gcs, double next_gc_start_ms,
-          double last_gc_time_ms, size_t committed_memory_at_last_run)
+          double last_gc_time_ms)
         : action(action),
           started_gcs(started_gcs),
           next_gc_start_ms(next_gc_start_ms),
-          last_gc_time_ms(last_gc_time_ms),
-          committed_memory_at_last_run(committed_memory_at_last_run) {}
+          last_gc_time_ms(last_gc_time_ms) {}
     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 };
@@ -104,7 +102,6 @@ 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;
@@ -112,7 +109,7 @@ class V8_EXPORT_PRIVATE MemoryReducer {
 
   explicit MemoryReducer(Heap* heap)
       : heap_(heap),
-        state_(kDone, 0, 0.0, 0.0, 0),
+        state_(kDone, 0, 0.0, 0.0),
         js_calls_counter_(0),
         js_calls_sample_time_ms_(0.0) {}
   // Callbacks.
@@ -129,12 +126,6 @@ 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,44 +12,36 @@ namespace v8 {
 namespace internal {
 
 MemoryReducer::State DoneState() {
-  return MemoryReducer::State(MemoryReducer::kDone, 0, 0.0, 1.0, 0);
+  return MemoryReducer::State(MemoryReducer::kDone, 0, 0.0, 1.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, 0);
+                              next_gc_start_ms, 1.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, 0);
+                              next_gc_start_ms, 1.0);
 }
 
 MemoryReducer::Event MarkCompactEvent(double time_ms,
-                                      bool next_gc_likely_to_collect_more,
-                                      size_t committed_memory) {
+                                      bool next_gc_likely_to_collect_more) {
   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,
-                                                 size_t committed_memory) {
-  return MarkCompactEvent(time_ms, true, committed_memory);
+MemoryReducer::Event MarkCompactEventGarbageLeft(double time_ms) {
+  return MarkCompactEvent(time_ms, true);
 }
 
-MemoryReducer::Event MarkCompactEventNoGarbageLeft(double time_ms,
-                                                   size_t committed_memory) {
-  return MarkCompactEvent(time_ms, false, committed_memory);
+MemoryReducer::Event MarkCompactEventNoGarbageLeft(double time_ms) {
+  return MarkCompactEvent(time_ms, false);
 }
 
 
@@ -98,19 +90,6 @@ 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);
 }
 
 
@@ -119,17 +98,13 @@ TEST(MemoryReducer, FromDoneToWait) {
 
   MemoryReducer::State state0(DoneState()), state1(DoneState());
 
-  state1 = MemoryReducer::Step(
-      state0,
-      MarkCompactEventGarbageLeft(2, MemoryReducer::kCommittedMemoryDelta));
+  state1 = MemoryReducer::Step(state0, MarkCompactEventGarbageLeft(2));
   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, MemoryReducer::kCommittedMemoryDelta));
+  state1 = MemoryReducer::Step(state0, MarkCompactEventNoGarbageLeft(2));
   EXPECT_EQ(MemoryReducer::kWait, state1.action);
   EXPECT_EQ(MemoryReducer::kLongDelayMs + 2, state1.next_gc_start_ms);
   EXPECT_EQ(0, state1.started_gcs);
@@ -140,16 +115,6 @@ 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);
 }
 
 
@@ -179,13 +144,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, 0));
+  state1 = MemoryReducer::Step(state0, MarkCompactEventGarbageLeft(2000));
   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, 0));
+  state1 = MemoryReducer::Step(state0, MarkCompactEventNoGarbageLeft(2000));
   EXPECT_EQ(MemoryReducer::kWait, state1.action);
   EXPECT_EQ(2000 + MemoryReducer::kLongDelayMs, state1.next_gc_start_ms);
   EXPECT_EQ(state0.started_gcs, state1.started_gcs);
@@ -294,7 +259,7 @@ TEST(MemoryReducer, FromRunToDone) {
 
   MemoryReducer::State state0(RunState(2, 0.0)), state1(DoneState());
 
-  state1 = MemoryReducer::Step(state0, MarkCompactEventNoGarbageLeft(2000, 0));
+  state1 = MemoryReducer::Step(state0, MarkCompactEventNoGarbageLeft(2000));
   EXPECT_EQ(MemoryReducer::kDone, state1.action);
   EXPECT_EQ(0, state1.next_gc_start_ms);
   EXPECT_EQ(MemoryReducer::kMaxNumberOfGCs, state1.started_gcs);
@@ -302,7 +267,7 @@ TEST(MemoryReducer, FromRunToDone) {
 
   state0.started_gcs = MemoryReducer::kMaxNumberOfGCs;
 
-  state1 = MemoryReducer::Step(state0, MarkCompactEventGarbageLeft(2000, 0));
+  state1 = MemoryReducer::Step(state0, MarkCompactEventGarbageLeft(2000));
   EXPECT_EQ(MemoryReducer::kDone, state1.action);
   EXPECT_EQ(0, state1.next_gc_start_ms);
   EXPECT_EQ(2000, state1.last_gc_time_ms);
@@ -314,7 +279,7 @@ TEST(MemoryReducer, FromRunToWait) {
 
   MemoryReducer::State state0(RunState(2, 0.0)), state1(DoneState());
 
-  state1 = MemoryReducer::Step(state0, MarkCompactEventGarbageLeft(2000, 0));
+  state1 = MemoryReducer::Step(state0, MarkCompactEventGarbageLeft(2000));
   EXPECT_EQ(MemoryReducer::kWait, state1.action);
   EXPECT_EQ(2000 + MemoryReducer::kShortDelayMs, state1.next_gc_start_ms);
   EXPECT_EQ(state0.started_gcs, state1.started_gcs);
@@ -322,7 +287,7 @@ TEST(MemoryReducer, FromRunToWait) {
 
   state0.started_gcs = 1;
 
-  state1 = MemoryReducer::Step(state0, MarkCompactEventNoGarbageLeft(2000, 0));
+  state1 = MemoryReducer::Step(state0, MarkCompactEventNoGarbageLeft(2000));
   EXPECT_EQ(MemoryReducer::kWait, state1.action);
   EXPECT_EQ(2000 + MemoryReducer::kShortDelayMs, state1.next_gc_start_ms);
   EXPECT_EQ(state0.started_gcs, state1.started_gcs);