Use actual incremental marking throughput in IdleNotification to estimate marking step size.

BUG=
R=jochen@chromium.org, ulan@chromium.org

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

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

BUILD.gn

@@ -628,6 +628,8 @@ source_set("v8_base") {
     "src/heap-snapshot-generator-inl.h",
     "src/heap-snapshot-generator.cc",
     "src/heap-snapshot-generator.h",
+    "src/heap/gc-idle-time-handler.cc",
+    "src/heap/gc-idle-time-handler.h",
     "src/heap/gc-tracer.cc",
     "src/heap/gc-tracer.h",
     "src/heap/heap-inl.h",

src/heap/gc-idle-time-handler.cc

@@ -0,0 +1,37 @@
+// Copyright 2014 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 <climits>
+
+#include "src/v8.h"
+
+#include "src/heap/gc-idle-time-handler.h"
+
+namespace v8 {
+namespace internal {
+
+
+const double GCIdleTimeHandler::kConservativeTimeRatio = 0.9;
+
+
+intptr_t GCIdleTimeHandler::EstimateMarkingStepSize(
+    int idle_time_in_ms, intptr_t marking_speed_in_bytes_per_ms) {
+  DCHECK(idle_time_in_ms > 0);
+
+  if (marking_speed_in_bytes_per_ms == 0) {
+    marking_speed_in_bytes_per_ms =
+        GCIdleTimeHandler::kInitialConservativeMarkingSpeed;
+  }
+
+  intptr_t marking_step_size = marking_speed_in_bytes_per_ms * idle_time_in_ms;
+  if (static_cast<intptr_t>(marking_step_size / idle_time_in_ms) !=
+      marking_speed_in_bytes_per_ms) {
+    // In the case of an overflow we return maximum marking step size.
+    return INT_MAX;
+  }
+
+  return static_cast<intptr_t>(marking_step_size *
+                               GCIdleTimeHandler::kConservativeTimeRatio);
+}
+}
+}

src/heap/gc-idle-time-handler.h

@@ -0,0 +1,35 @@
+// Copyright 2014 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.
+
+#ifndef V8_HEAP_GC_IDLE_TIME_HANDLER_H_
+#define V8_HEAP_GC_IDLE_TIME_HANDLER_H_
+
+#include "src/globals.h"
+
+namespace v8 {
+namespace internal {
+
+// The idle time handler makes decisions about which garbage collection
+// operations are executing during IdleNotification.
+class GCIdleTimeHandler {
+ public:
+  static intptr_t EstimateMarkingStepSize(
+      int idle_time_in_ms, intptr_t marking_speed_in_bytes_per_ms);
+
+  // If we haven't recorded any incremental marking events yet, we carefully
+  // mark with a conservative lower bound for the marking speed.
+  static const intptr_t kInitialConservativeMarkingSpeed = 100 * KB;
+
+  // We have to make sure that we finish the IdleNotification before
+  // idle_time_in_ms. Hence, we conservatively prune our workload estimate.
+  static const double kConservativeTimeRatio;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(GCIdleTimeHandler);
+};
+
+}  // namespace internal
+}  // namespace v8
+
+#endif  // V8_HEAP_GC_IDLE_TIME_HANDLER_H_

src/heap/heap.cc

@@ -16,6 +16,7 @@
 #include "src/debug.h"
 #include "src/deoptimizer.h"
 #include "src/global-handles.h"
+#include "src/heap/gc-idle-time-handler.h"
 #include "src/heap/incremental-marking.h"
 #include "src/heap/mark-compact.h"
 #include "src/heap/objects-visiting-inl.h"
@@ -4263,7 +4264,10 @@ void Heap::MakeHeapIterable() {
 }
 
 
-void Heap::AdvanceIdleIncrementalMarking(intptr_t step_size) {
+void Heap::AdvanceIdleIncrementalMarking(int idle_time_in_ms) {
+  intptr_t step_size = GCIdleTimeHandler::EstimateMarkingStepSize(
+      idle_time_in_ms, tracer_.IncrementalMarkingSpeedInBytesPerMillisecond());
+
   incremental_marking()->Step(step_size,
                               IncrementalMarking::NO_GC_VIA_STACK_GUARD, true);
 
@@ -4286,36 +4290,27 @@ void Heap::AdvanceIdleIncrementalMarking(intptr_t step_size) {
 }
 
 
-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;
-  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;
+  isolate()->counters()->gc_idle_time_allotted_in_ms()->AddSample(
-  // The size factor is in range [5..250]. The numbers here are chosen from
+      idle_time_in_ms);
-  // 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);
   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);
+      AdvanceIdleIncrementalMarking(idle_time_in_ms);
     }
 
     // After context disposal there is likely a lot of garbage remaining, reset
@@ -4350,17 +4345,16 @@ bool Heap::IdleNotification(int hint) {
     // 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 {
       incremental_marking()->Start();
     }
   }
-  if (!incremental_marking()->IsStopped() &&
+  if (!incremental_marking()->IsStopped()) {
-      hint > kMinHintForIncrementalMarking) {
+    AdvanceIdleIncrementalMarking(idle_time_in_ms);
-    AdvanceIdleIncrementalMarking(step_size);
   }
 
   if (mark_sweeps_since_idle_round_started_ >= kMaxMarkSweepsInIdleRound) {
@@ -4370,7 +4364,7 @@ bool Heap::IdleNotification(int hint) {
 
   // If the IdleNotifcation is called with a large hint we will wait for
   // the sweepter threads here.
-  if (hint >= kMinHintForFullGC &&
+  if (idle_time_in_ms >= kMinHintForFullGC &&
       mark_compact_collector()->sweeping_in_progress()) {
     mark_compact_collector()->EnsureSweepingCompleted();
   }

src/heap/heap.h

@@ -1050,7 +1050,7 @@ class Heap {
   void DisableInlineAllocation();
 
   // Implements the corresponding V8 API function.
-  bool IdleNotification(int hint);
+  bool IdleNotification(int idle_time_in_ms);
 
   // Declare all the root indices.  This defines the root list order.
   enum RootListIndex {
@@ -1951,7 +1951,7 @@ class Heap {
     return heap_size_mb / kMbPerMs;
   }
 
-  void AdvanceIdleIncrementalMarking(intptr_t step_size);
+  void AdvanceIdleIncrementalMarking(int idle_time_in_ms);
 
   void ClearObjectStats(bool clear_last_time_stats = false);
 

test/heap-unittests/heap-unittest.cc

@@ -2,14 +2,43 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <climits>
+
+#include "src/heap/gc-idle-time-handler.h"
+
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace v8 {
 namespace internal {
 
-TEST(HeapTest, Dummy) {
+TEST(EstimateMarkingStepSizeTest, EstimateMarkingStepSizeInitial) {
-  EXPECT_FALSE(false);
+  intptr_t step_size = GCIdleTimeHandler::EstimateMarkingStepSize(1, 0);
-  EXPECT_TRUE(true);
+  EXPECT_EQ(static_cast<intptr_t>(
+                GCIdleTimeHandler::kInitialConservativeMarkingSpeed *
+                GCIdleTimeHandler::kConservativeTimeRatio),
+            step_size);
+}
+
+
+TEST(EstimateMarkingStepSizeTest, EstimateMarkingStepSizeNonZero) {
+  intptr_t marking_speed_in_bytes_per_millisecond = 100;
+  intptr_t step_size = GCIdleTimeHandler::EstimateMarkingStepSize(
+      1, marking_speed_in_bytes_per_millisecond);
+  EXPECT_EQ(static_cast<intptr_t>(marking_speed_in_bytes_per_millisecond *
+                                  GCIdleTimeHandler::kConservativeTimeRatio),
+            step_size);
+}
+
+
+TEST(EstimateMarkingStepSizeTest, EstimateMarkingStepSizeOverflow1) {
+  intptr_t step_size = GCIdleTimeHandler::EstimateMarkingStepSize(10, INT_MAX);
+  EXPECT_EQ(INT_MAX, step_size);
+}
+
+
+TEST(EstimateMarkingStepSizeTest, EstimateMarkingStepSizeOverflow2) {
+  intptr_t step_size = GCIdleTimeHandler::EstimateMarkingStepSize(INT_MAX, 10);
+  EXPECT_EQ(INT_MAX, step_size);
 }
 
 }  // namespace internal

tools/gyp/v8.gyp

@@ -515,6 +515,8 @@
         '../../src/heap-snapshot-generator-inl.h',
         '../../src/heap-snapshot-generator.cc',
         '../../src/heap-snapshot-generator.h',
+        '../../src/heap/gc-idle-time-handler.cc',
+        '../../src/heap/gc-idle-time-handler.h',
         '../../src/heap/gc-tracer.cc',
         '../../src/heap/gc-tracer.h',
         '../../src/heap/heap-inl.h',