Use the regular start incremental marking strategy in the idle notification.
authorhpayer@chromium.org <hpayer@chromium.org>
Tue, 16 Sep 2014 12:48:59 +0000 (12:48 +0000)
committerhpayer@chromium.org <hpayer@chromium.org>
Tue, 16 Sep 2014 12:48:59 +0000 (12:48 +0000)
BUG=
R=ulan@chromium.org

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

git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@23976 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/heap/gc-idle-time-handler-unittest.cc
src/heap/gc-idle-time-handler.cc
src/heap/gc-idle-time-handler.h
src/heap/heap.cc
src/heap/incremental-marking.cc
src/heap/incremental-marking.h
test/cctest/test-api.cc

index 165cac3..24438ae 100644 (file)
@@ -192,8 +192,11 @@ TEST_F(GCIdleTimeHandlerTest, StopEventually2) {
   for (int i = 0; i < GCIdleTimeHandler::kMaxMarkCompactsInIdleRound; i++) {
     GCIdleTimeAction action = handler()->Compute(idle_time_ms, heap_state);
     EXPECT_EQ(DO_INCREMENTAL_MARKING, action.type);
+    // In this case we emulate incremental marking steps that finish with a
+    // full gc.
     handler()->NotifyIdleMarkCompact();
   }
+  heap_state.can_start_incremental_marking = false;
   GCIdleTimeAction action = handler()->Compute(idle_time_ms, heap_state);
   EXPECT_EQ(DONE, action.type);
 }
@@ -228,14 +231,18 @@ TEST_F(GCIdleTimeHandlerTest, ContinueAfterStop2) {
     GCIdleTimeAction action = handler()->Compute(idle_time_ms, heap_state);
     if (action.type == DONE) break;
     EXPECT_EQ(DO_INCREMENTAL_MARKING, action.type);
+    // In this case we try to emulate incremental marking steps the finish with
+    // a full gc.
     handler()->NotifyIdleMarkCompact();
   }
+  heap_state.can_start_incremental_marking = false;
   GCIdleTimeAction action = handler()->Compute(idle_time_ms, heap_state);
   EXPECT_EQ(DONE, action.type);
   // Emulate mutator work.
   for (int i = 0; i < GCIdleTimeHandler::kIdleScavengeThreshold; i++) {
     handler()->NotifyScavenge();
   }
+  heap_state.can_start_incremental_marking = true;
   action = handler()->Compute(idle_time_ms, heap_state);
   EXPECT_EQ(DO_INCREMENTAL_MARKING, action.type);
 }
index bc182ae..93be16c 100644 (file)
@@ -80,6 +80,7 @@ GCIdleTimeAction GCIdleTimeHandler::Compute(size_t idle_time_in_ms,
       return GCIdleTimeAction::Done();
     }
   }
+
   if (heap_state.incremental_marking_stopped) {
     size_t estimated_time_in_ms =
         EstimateMarkCompactTime(heap_state.size_of_objects,
@@ -94,8 +95,10 @@ GCIdleTimeAction GCIdleTimeHandler::Compute(size_t idle_time_in_ms,
       // can get rid of this special case and always start incremental marking.
       int remaining_mark_sweeps =
           kMaxMarkCompactsInIdleRound - mark_compacts_since_idle_round_started_;
-      if (heap_state.contexts_disposed > 0 || remaining_mark_sweeps <= 2 ||
-          !heap_state.can_start_incremental_marking) {
+      if (heap_state.contexts_disposed > 0 ||
+          (idle_time_in_ms > kMaxFrameRenderingIdleTime &&
+           (remaining_mark_sweeps <= 2 ||
+            !heap_state.can_start_incremental_marking))) {
         return GCIdleTimeAction::FullGC();
       }
     }
@@ -109,6 +112,10 @@ GCIdleTimeAction GCIdleTimeHandler::Compute(size_t idle_time_in_ms,
     return GCIdleTimeAction::FinalizeSweeping();
   }
 
+  if (heap_state.incremental_marking_stopped &&
+      !heap_state.can_start_incremental_marking) {
+    return GCIdleTimeAction::Nothing();
+  }
   size_t step_size = EstimateMarkingStepSize(
       idle_time_in_ms, heap_state.incremental_marking_speed_in_bytes_per_ms);
   return GCIdleTimeAction::IncrementalMarking(step_size);
index 899f289..2d557df 100644 (file)
@@ -110,6 +110,9 @@ class GCIdleTimeHandler {
   // step.
   static const size_t kSmallHeapSize = 2 * kPointerSize * MB;
 
+  // That is the maximum idle time we will have during frame rendering.
+  static const size_t kMaxFrameRenderingIdleTime = 16;
+
   struct HeapState {
     int contexts_disposed;
     size_t size_of_objects;
index e6770e5..2bcd583 100644 (file)
@@ -1548,8 +1548,6 @@ void Heap::Scavenge() {
   LOG(isolate_, ResourceEvent("scavenge", "end"));
 
   gc_state_ = NOT_IN_GC;
-
-  gc_idle_time_handler_.NotifyScavenge();
 }
 
 
@@ -4305,7 +4303,8 @@ bool Heap::IdleNotification(int idle_time_in_ms) {
   heap_state.size_of_objects = static_cast<size_t>(SizeOfObjects());
   heap_state.incremental_marking_stopped = incremental_marking()->IsStopped();
   // TODO(ulan): Start incremental marking only for large heaps.
-  heap_state.can_start_incremental_marking = true;
+  heap_state.can_start_incremental_marking =
+      incremental_marking()->ShouldActivate();
   heap_state.sweeping_in_progress =
       mark_compact_collector()->sweeping_in_progress();
   heap_state.mark_compact_speed_in_bytes_per_ms =
index 8f92976..d72423a 100644 (file)
@@ -421,6 +421,11 @@ void IncrementalMarking::ActivateIncrementalWriteBarrier() {
 }
 
 
+bool IncrementalMarking::ShouldActivate() {
+  return WorthActivating() && heap_->NextGCIsLikelyToBeFull();
+}
+
+
 bool IncrementalMarking::WorthActivating() {
 #ifndef DEBUG
   static const intptr_t kActivationThreshold = 8 * MB;
@@ -811,7 +816,7 @@ void IncrementalMarking::MarkingComplete(CompletionAction action) {
 
 
 void IncrementalMarking::OldSpaceStep(intptr_t allocated) {
-  if (IsStopped() && WorthActivating() && heap_->NextGCIsLikelyToBeFull()) {
+  if (IsStopped() && ShouldActivate()) {
     // TODO(hpayer): Let's play safe for now, but compaction should be
     // in principle possible.
     Start(PREVENT_COMPACTION);
index 3ccb12a..e4a8e97 100644 (file)
@@ -44,6 +44,8 @@ class IncrementalMarking {
 
   bool WorthActivating();
 
+  bool ShouldActivate();
+
   enum CompactionFlag { ALLOW_COMPACTION, PREVENT_COMPACTION };
 
   void Start(CompactionFlag flag = ALLOW_COMPACTION);
index a6e2290..9eddbac 100644 (file)
@@ -17821,14 +17821,13 @@ TEST(IdleNotificationWithLargeHint) {
 
 TEST(Regress2107) {
   const intptr_t MB = 1024 * 1024;
-  const int kShortIdlePauseInMs = 100;
-  const int kLongIdlePauseInMs = 1000;
+  const int kIdlePauseInMs = 1000;
   LocalContext env;
   v8::Isolate* isolate = env->GetIsolate();
   v8::HandleScope scope(env->GetIsolate());
   intptr_t initial_size = CcTest::heap()->SizeOfObjects();
   // Send idle notification to start a round of incremental GCs.
-  env->GetIsolate()->IdleNotification(kShortIdlePauseInMs);
+  env->GetIsolate()->IdleNotification(kIdlePauseInMs);
   // Emulate 7 page reloads.
   for (int i = 0; i < 7; i++) {
     {
@@ -17839,7 +17838,7 @@ TEST(Regress2107) {
       ctx->Exit();
     }
     env->GetIsolate()->ContextDisposedNotification();
-    env->GetIsolate()->IdleNotification(kLongIdlePauseInMs);
+    env->GetIsolate()->IdleNotification(kIdlePauseInMs);
   }
   // Create garbage and check that idle notification still collects it.
   CreateGarbageInOldSpace();
@@ -17847,7 +17846,7 @@ TEST(Regress2107) {
   CHECK_GT(size_with_garbage, initial_size + MB);
   bool finished = false;
   for (int i = 0; i < 200 && !finished; i++) {
-    finished = env->GetIsolate()->IdleNotification(kShortIdlePauseInMs);
+    finished = env->GetIsolate()->IdleNotification(kIdlePauseInMs);
   }
   intptr_t final_size = CcTest::heap()->SizeOfObjects();
   CHECK_LT(final_size, initial_size + 1);