Make sure that there is enough time left before finishing incremental marking in idle notification.

src/heap.cc

 // Copyright 2012 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/v8.h"
 
 #include "src/accessors.h"
 #include "src/api.h"
 #include "src/base/once.h"
 #include "src/base/utils/random-number-generator.h"
@@ skipping 4209 matching lines @@
   if (!IsHeapIterable()) {
     CollectAllGarbage(kMakeHeapIterableMask, "Heap::MakeHeapIterable");
   }
   if (mark_compact_collector()->sweeping_in_progress()) {
     mark_compact_collector()->EnsureSweepingCompleted();
   }
   ASSERT(IsHeapIterable());
 }
 
 
-void Heap::AdvanceIdleIncrementalMarking(intptr_t step_size) {
+int Heap::AdvanceIdleIncrementalMarking(intptr_t step_size) {
+  double start = base::OS::TimeCurrentMillis();
   incremental_marking()->Step(step_size,
                               IncrementalMarking::NO_GC_VIA_STACK_GUARD);
+  return static_cast<int>(base::OS::TimeCurrentMillis() - start);
+}
 
+
+void Heap::FinalizeIdleIncrementalMarking() {
   if (incremental_marking()->IsComplete()) {
     bool uncommit = false;
     if (gc_count_at_last_idle_gc_ == gc_count_) {
       // No GC since the last full GC, the mutator is probably not active.
       isolate_->compilation_cache()->Clear();
       uncommit = true;
     }
     CollectAllGarbage(kReduceMemoryFootprintMask,
                       "idle notification: finalize incremental");
     mark_sweeps_since_idle_round_started_++;
     gc_count_at_last_idle_gc_ = gc_count_;
     if (uncommit) {
       new_space_.Shrink();
       UncommitFromSpace();
     }
   }
 }
 
 
-bool Heap::IdleNotification(int hint) {
+bool Heap::IdleNotification(int idle_time_in_ms) {
   // If incremental marking is off, we do not perform idle notification.
   if (!FLAG_incremental_marking) return true;
 
   // Hints greater than this value indicate that
   // the embedder is requesting a lot of GC work.
   const int kMaxHint = 1000;
+  // TODO(hpayer): remove min hint
   const int kMinHintForIncrementalMarking = 10;
   // Minimal hint that allows to do full GC.
   const int kMinHintForFullGC = 100;
-  intptr_t size_factor = Min(Max(hint, 20), kMaxHint) / 4;
+  // TODO(hpayer, ernstm): use previous time measurements to calculate
+  // proper size_factor
+  intptr_t size_factor = Min(Max(idle_time_in_ms, 20), kMaxHint) / 4;
   // The size factor is in range [5..250]. The numbers here are chosen from
   // experiments. If you changes them, make sure to test with
   // chrome/performance_ui_tests --gtest_filter="GeneralMixMemoryTest.*
   intptr_t step_size =
       size_factor * IncrementalMarking::kAllocatedThreshold;
 
-  isolate()->counters()->gc_idle_time_allotted_in_ms()->AddSample(hint);
+  isolate()->counters()->gc_idle_time_allotted_in_ms()->AddSample(
+      idle_time_in_ms);
   HistogramTimerScope idle_notification_scope(
       isolate_->counters()->gc_idle_notification());
 
   if (contexts_disposed_ > 0) {
     contexts_disposed_ = 0;
     int mark_sweep_time = Min(TimeMarkSweepWouldTakeInMs(), 1000);
-    if (hint >= mark_sweep_time && !FLAG_expose_gc &&
+    if (idle_time_in_ms >= mark_sweep_time && !FLAG_expose_gc &&
         incremental_marking()->IsStopped()) {
       HistogramTimerScope scope(isolate_->counters()->gc_context());
       CollectAllGarbage(kReduceMemoryFootprintMask,
                         "idle notification: contexts disposed");
     } else {
-      AdvanceIdleIncrementalMarking(step_size);
+      int remaining_time = AdvanceIdleIncrementalMarking(step_size);

[ernstm, 2014/07/30 10:33:26]

Isn't the value returned by AdvanceIdleIncrementalMarking the time we already
spent? Maybe rename remaining_time to time_elapsed?

[Hannes Payer (out of office), 2014/07/30 11:48:52]

yes, that makes sense. actually, we should use the tracer and not a custom timer
here. WDYT?

+      if (idle_time_in_ms - remaining_time > kMinHintForIncrementalMarking) {
+        FinalizeIdleIncrementalMarking();
+      }
     }
 
     // After context disposal there is likely a lot of garbage remaining, reset
     // the idle notification counters in order to trigger more incremental GCs
     // on subsequent idle notifications.
     StartIdleRound();
     return false;
   }
 
   // By doing small chunks of GC work in each IdleNotification,
@@ skipping 14 matching lines @@
 
   int remaining_mark_sweeps = kMaxMarkSweepsInIdleRound -
                               mark_sweeps_since_idle_round_started_;
 
   if (incremental_marking()->IsStopped()) {
     // If there are no more than two GCs left in this idle round and we are
     // allowed to do a full GC, then make those GCs full in order to compact
     // the code space.
     // TODO(ulan): Once we enable code compaction for incremental marking,
     // we can get rid of this special case and always start incremental marking.
-    if (remaining_mark_sweeps <= 2 && hint >= kMinHintForFullGC) {
+    if (remaining_mark_sweeps <= 2 && idle_time_in_ms >= kMinHintForFullGC) {
       CollectAllGarbage(kReduceMemoryFootprintMask,
                         "idle notification: finalize idle round");
       mark_sweeps_since_idle_round_started_++;
-    } else if (hint > kMinHintForIncrementalMarking) {
+    } else if (idle_time_in_ms > kMinHintForIncrementalMarking) {
       incremental_marking()->Start();
     }
   }
   if (!incremental_marking()->IsStopped() &&
-      hint > kMinHintForIncrementalMarking) {
-    AdvanceIdleIncrementalMarking(step_size);
+      idle_time_in_ms > kMinHintForIncrementalMarking) {
+    int remaining_time = AdvanceIdleIncrementalMarking(step_size);

[ernstm, 2014/07/30 10:33:26]

remaining_time -> time_elapsed?

[Hannes Payer (out of office), 2014/07/30 11:48:52]

yes.

+    if (idle_time_in_ms - remaining_time > kMinHintForIncrementalMarking) {
+      FinalizeIdleIncrementalMarking();
+    }
   }
 
   if (mark_sweeps_since_idle_round_started_ >= kMaxMarkSweepsInIdleRound) {
     FinishIdleRound();
     return true;
   }
 
-  // If the IdleNotifcation is called with a large hint we will wait for
-  // the sweepter threads here.
-  if (hint >= kMinHintForFullGC &&
+  // If the IdleNotifcation is called with a large idle_time_in_ms we will wait
+  // for the sweepter threads here.
+  if (idle_time_in_ms >= kMinHintForFullGC &&
       mark_compact_collector()->sweeping_in_progress()) {
     mark_compact_collector()->EnsureSweepingCompleted();
   }
 
   return false;
 }
 
 
 #ifdef DEBUG
 
@@ skipping 1817 matching lines @@
       static_cast<int>(object_sizes_last_time_[index]));
   CODE_AGE_LIST_COMPLETE(ADJUST_LAST_TIME_OBJECT_COUNT)
 #undef ADJUST_LAST_TIME_OBJECT_COUNT
 
   MemCopy(object_counts_last_time_, object_counts_, sizeof(object_counts_));
   MemCopy(object_sizes_last_time_, object_sizes_, sizeof(object_sizes_));
   ClearObjectStats();
 }
 
 } }  // namespace v8::internal