Use smaller heap growing factor in idle notification to start incremental marking when there is idl…

src/heap/heap.cc

Index: src/heap/heap.cc
diff --git a/src/heap/heap.cc b/src/heap/heap.cc
index 6ee6c094bd05ebefb228697f0ca778e3b94ff906..22e812bf0c2d508d162f5d910067eb220f1afb5b 100644
--- a/src/heap/heap.cc
+++ b/src/heap/heap.cc
@@ -101,6 +101,11 @@ Heap::Heap()
       allocation_timeout_(0),
 #endif  // DEBUG
       old_generation_allocation_limit_(initial_old_generation_size_),
+      min_old_generation_growing_factor_(1.1),
+      max_old_generation_growing_factor_(4.0),
+      idle_old_generation_growing_factor_(1.5),
+      current_old_generation_growing_factor_(
+          max_old_generation_growing_factor_),
       old_gen_exhausted_(false),
       inline_allocation_disabled_(false),
       store_buffer_rebuilder_(store_buffer()),
@@ -4548,7 +4553,7 @@ bool Heap::TryFinalizeIdleIncrementalMarking(
 
 bool Heap::WorthActivatingIncrementalMarking() {
   return incremental_marking()->IsStopped() &&
-         incremental_marking()->WorthActivating() && NextGCIsLikelyToBeFull();
+         incremental_marking()->ShouldActivate();
 }
 
 
@@ -4573,6 +4578,7 @@ bool Heap::IdleNotification(double deadline_in_seconds) {
       static_cast<double>(base::Time::kMillisecondsPerSecond);
   HistogramTimerScope idle_notification_scope(
       isolate_->counters()->gc_idle_notification());
+  double idle_time_in_ms = deadline_in_ms - MonotonicallyIncreasingTimeInMs();
 
   GCIdleTimeHandler::HeapState heap_state;
   heap_state.contexts_disposed = contexts_disposed_;
@@ -4581,8 +4587,17 @@ bool Heap::IdleNotification(double deadline_in_seconds) {
   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.
+  intptr_t limit = old_generation_allocation_limit_;
+  if (static_cast<size_t>(idle_time_in_ms) >
+      GCIdleTimeHandler::kMaxFrameRenderingIdleTime) {
+    limit =

[ulan, 2015/04/16 14:17:21]

Instead of storing current_old_generation_growing_factor_ and reversing the
limit, can we just compute and store the idle_limit at the same time as we
compute the old_generation_allocation_limit?

[Hannes Payer (out of office), 2015/04/16 14:46:45]

Done.

+        static_cast<intptr_t>((limit / current_old_generation_growing_factor_) *
+                              idle_old_generation_growing_factor_);
+  }
+
   heap_state.can_start_incremental_marking =
-      incremental_marking()->ShouldActivate() && FLAG_incremental_marking &&
+      incremental_marking()->WorthActivating() &&
+      NextGCIsLikelyToBeFull(limit) && FLAG_incremental_marking &&
       !mark_compact_collector()->sweeping_in_progress();
   heap_state.sweeping_in_progress =
       mark_compact_collector()->sweeping_in_progress();
@@ -4603,7 +4618,6 @@ bool Heap::IdleNotification(double deadline_in_seconds) {
       static_cast<size_t>(
           tracer()->NewSpaceAllocationThroughputInBytesPerMillisecond());
 
-  double idle_time_in_ms = deadline_in_ms - MonotonicallyIncreasingTimeInMs();
   GCIdleTimeAction action =
       gc_idle_time_handler_.Compute(idle_time_in_ms, heap_state);
   isolate()->counters()->gc_idle_time_allotted_in_ms()->AddSample(
@@ -5107,6 +5121,13 @@ bool Heap::ConfigureHeap(int max_semi_space_size, int max_old_space_size,
       Max(static_cast<intptr_t>(paged_space_count * Page::kPageSize),
           max_old_generation_size_);
 
+
+  // We set the old generation growing factor to 2 to grow the heap slower on
+  // memory-constrained devices.
+  if (max_old_generation_size_ <= kMaxOldSpaceSizeMediumMemoryDevice) {
+    max_old_generation_growing_factor_ = 2;
+  }
+
   if (FLAG_initial_old_space_size > 0) {
     initial_old_generation_size_ = FLAG_initial_old_space_size * MB;
   } else {
@@ -5182,35 +5203,38 @@ intptr_t Heap::OldGenerationAllocationLimit(intptr_t old_gen_size,
                                             int freed_global_handles) {
   const int kMaxHandles = 1000;
   const int kMinHandles = 100;
-  double min_factor = 1.1;
-  double max_factor = 1.5;
-  // We set the old generation growing factor to 2 to grow the heap slower on
-  // memory-constrained devices.
-  if (max_old_generation_size_ <= kMaxOldSpaceSizeMediumMemoryDevice) {
-    max_factor = 1.5;
-  }
+
   // If there are many freed global handles, then the next full GC will
   // likely collect a lot of garbage. Choose the heap growing factor
   // depending on freed global handles.
   // TODO(ulan, hpayer): Take into account mutator utilization.
+  // TODO(hpayer): The idle factor could make the handles heuristic obsolete.
+  // Look into that.
   double factor;
   if (freed_global_handles <= kMinHandles) {
-    factor = max_factor;
+    factor = max_old_generation_growing_factor_;
   } else if (freed_global_handles >= kMaxHandles) {
-    factor = min_factor;
+    factor = min_old_generation_growing_factor_;
   } else {
     // Compute factor using linear interpolation between points
     // (kMinHandles, max_factor) and (kMaxHandles, min_factor).
-    factor = max_factor -
-             (freed_global_handles - kMinHandles) * (max_factor - min_factor) /
+    factor = max_old_generation_growing_factor_ -
+             (freed_global_handles - kMinHandles) *
+                 (max_old_generation_growing_factor_ -
+                  min_old_generation_growing_factor_) /
                  (kMaxHandles - kMinHandles);
   }
 
   if (FLAG_stress_compaction ||
       mark_compact_collector()->reduce_memory_footprint_) {
-    factor = min_factor;
+    factor = min_old_generation_growing_factor_;
   }
 
+  // When the limit is taken from halfway_to_the_max, the current factor
+  // does not correspond to the original old generation size. However, it
+  // may be close.

[Erik Corry Chromium.org, 2015/04/16 14:16:44]

But it may not be close.

I'm not very happy with setting the limit to Min(halfway_ttm, x * factor +
stuff) and then later changing the limit by dividing by the factor and
multiplying by another factor.  It would be cleaner to have two limits,
calculated with the two factors and set them both in this routine.

[Hannes Payer (out of office), 2015/04/16 14:46:45]

Done.

+  current_old_generation_growing_factor_ = factor;

[ulan, 2015/04/16 14:17:22]

Since it is used for just for the idle limit computation, I would store the idle
limit directly here.

[Hannes Payer (out of office), 2015/04/16 14:46:45]

Done.

+
   intptr_t limit = static_cast<intptr_t>(old_gen_size * factor);
   limit = Max(limit, kMinimumOldGenerationAllocationLimit);
   limit += new_space_.Capacity();