Reviewers: ulan, rmcilroy, jochen (slow),

Description:
Limit rate of full garbage collections triggered by idle notification.

Idle notification can trigger a bunch of full garbage collections, especially with the larger idle times of 50ms. We have to limit the rate of these events,
since there is no need to get rid of garbage on a high rate.

BUG=

Please review this at https://codereview.chromium.org/1023153002/

Base URL: https://chromium.googlesource.com/v8/v8.git@master

Affected files (+52, -13 lines):
  M src/heap/gc-idle-time-handler.h
  M src/heap/gc-idle-time-handler.cc
  M src/heap/heap.cc
  M src/heap/mark-compact.h
  M src/heap/mark-compact.cc
  M test/unittests/heap/gc-idle-time-handler-unittest.cc


Index: src/heap/gc-idle-time-handler.cc
diff --git a/src/heap/gc-idle-time-handler.cc b/src/heap/gc-idle-time-handler.cc index c74b46ea9b6d126b0770df551a830593198d63ef..8b8f306b7ffbd6d84167872562670b222e6f45e0 100644
--- a/src/heap/gc-idle-time-handler.cc
+++ b/src/heap/gc-idle-time-handler.cc
@@ -17,6 +17,7 @@ const int GCIdleTimeHandler::kMaxMarkCompactsInIdleRound = 7;
 const int GCIdleTimeHandler::kIdleScavengeThreshold = 5;
 const double GCIdleTimeHandler::kHighContextDisposalRate = 100;
const size_t GCIdleTimeHandler::kMinTimeForOverApproximatingWeakClosureInMs = 1; +const double GCIdleTimeHandler::kMinMillisecondsInBetweenMarkCompact = 1000;


 void GCIdleTimeAction::Print() {
@@ -152,10 +153,15 @@ bool GCIdleTimeHandler::ShouldDoScavenge(

 bool GCIdleTimeHandler::ShouldDoMarkCompact(
     size_t idle_time_in_ms, size_t size_of_objects,
-    size_t mark_compact_speed_in_bytes_per_ms) {
-  return idle_time_in_ms >=
-         EstimateMarkCompactTime(size_of_objects,
-                                 mark_compact_speed_in_bytes_per_ms);
+ size_t mark_compact_speed_in_bytes_per_ms, double last_mark_compact_time,
+    double current_time) {
+  bool mark_compact_rate_high =
+      current_time <
+      last_mark_compact_time + kMinMillisecondsInBetweenMarkCompact;
+  return !mark_compact_rate_high &&
+         idle_time_in_ms >=
+             EstimateMarkCompactTime(size_of_objects,
+                                     mark_compact_speed_in_bytes_per_ms);
 }


@@ -204,6 +210,7 @@ GCIdleTimeAction GCIdleTimeHandler::Compute(double idle_time_in_ms,
                                             HeapState heap_state) {
   if (static_cast<int>(idle_time_in_ms) <= 0) {
     if (heap_state.contexts_disposed > 0) {
+      printf("start idle round, contexts disposed\n");
       StartIdleRound();
     }
     if (heap_state.incremental_marking_stopped) {
@@ -226,6 +233,7 @@ GCIdleTimeAction GCIdleTimeHandler::Compute(double idle_time_in_ms,

   if (IsMarkCompactIdleRoundFinished()) {
     if (EnoughGarbageSinceLastIdleRound()) {
+      printf("start idle round, enough garbage\n");
       StartIdleRound();
     } else {
       return GCIdleTimeAction::Done();
@@ -233,9 +241,10 @@ GCIdleTimeAction GCIdleTimeHandler::Compute(double idle_time_in_ms,
   }

   if (heap_state.incremental_marking_stopped) {
-    if (ShouldDoMarkCompact(static_cast<size_t>(idle_time_in_ms),
-                            heap_state.size_of_objects,
- heap_state.mark_compact_speed_in_bytes_per_ms)) {
+    if (ShouldDoMarkCompact(
+ static_cast<size_t>(idle_time_in_ms), heap_state.size_of_objects,
+            heap_state.mark_compact_speed_in_bytes_per_ms,
+            heap_state.last_mark_compact_time, heap_state.current_time)) {
// 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.
Index: src/heap/gc-idle-time-handler.h
diff --git a/src/heap/gc-idle-time-handler.h b/src/heap/gc-idle-time-handler.h index 6a39b78aaa2fcc24fd5210236f177bf0f9fffc14..3d82686748ad782f116495f388b413d35ccfbfd7 100644
--- a/src/heap/gc-idle-time-handler.h
+++ b/src/heap/gc-idle-time-handler.h
@@ -138,6 +138,8 @@ class GCIdleTimeHandler {

   static const size_t kMinTimeForOverApproximatingWeakClosureInMs;

+  static const double kMinMillisecondsInBetweenMarkCompact;
+
   class HeapState {
    public:
     void Print();
@@ -155,6 +157,8 @@ class GCIdleTimeHandler {
     size_t used_new_space_size;
     size_t new_space_capacity;
     size_t new_space_allocation_throughput_in_bytes_per_ms;
+    double last_mark_compact_time;
+    double current_time;
   };

   GCIdleTimeHandler()
@@ -186,7 +190,9 @@ class GCIdleTimeHandler {

   static bool ShouldDoMarkCompact(size_t idle_time_in_ms,
                                   size_t size_of_objects,
- size_t mark_compact_speed_in_bytes_per_ms); + size_t mark_compact_speed_in_bytes_per_ms,
+                                  double last_mark_comact_time,
+                                  double current_time);

   static bool ShouldDoContextDisposalMarkCompact(int context_disposed,
double contexts_disposal_rate);
Index: src/heap/heap.cc
diff --git a/src/heap/heap.cc b/src/heap/heap.cc
index 483b091035fb61fc1023c3ceae383a97d071b5bc..555481a1ff2a510eb85dbf29bdc1af6fb73133d6 100644
--- a/src/heap/heap.cc
+++ b/src/heap/heap.cc
@@ -4597,6 +4597,7 @@ bool Heap::IdleNotification(double deadline_in_seconds) {
       static_cast<double>(base::Time::kMillisecondsPerSecond);
   HistogramTimerScope idle_notification_scope(
       isolate_->counters()->gc_idle_notification());
+  double current_time = MonotonicallyIncreasingTimeInMs();

   GCIdleTimeHandler::HeapState heap_state;
   heap_state.contexts_disposed = contexts_disposed_;
@@ -4623,8 +4624,11 @@ bool Heap::IdleNotification(double deadline_in_seconds) {
   heap_state.new_space_allocation_throughput_in_bytes_per_ms =
       static_cast<size_t>(
           tracer()->NewSpaceAllocationThroughputInBytesPerMillisecond());
+  heap_state.last_mark_compact_time =
+      mark_compact_collector()->last_mark_compact_time();
+  heap_state.current_time = current_time;

- double idle_time_in_ms = deadline_in_ms - MonotonicallyIncreasingTimeInMs();
+  double idle_time_in_ms = deadline_in_ms - current_time;
   GCIdleTimeAction action =
       gc_idle_time_handler_.Compute(idle_time_in_ms, heap_state);
   isolate()->counters()->gc_idle_time_allotted_in_ms()->AddSample(
@@ -4679,7 +4683,7 @@ bool Heap::IdleNotification(double deadline_in_seconds) {
       break;
   }

-  double current_time = MonotonicallyIncreasingTimeInMs();
+  current_time = MonotonicallyIncreasingTimeInMs();
   last_idle_notification_time_ = current_time;
   double deadline_difference = deadline_in_ms - current_time;

Index: src/heap/mark-compact.cc
diff --git a/src/heap/mark-compact.cc b/src/heap/mark-compact.cc
index de381aab356657374f1e62f083173045d949ded0..3d63bd12d1fe1285248f995bba18f024acd73c50 100644
--- a/src/heap/mark-compact.cc
+++ b/src/heap/mark-compact.cc
@@ -53,7 +53,8 @@ MarkCompactCollector::MarkCompactCollector(Heap* heap)
       marking_deque_memory_(NULL),
       marking_deque_memory_committed_(false),
       code_flusher_(NULL),
-      have_code_to_deoptimize_(false) {
+      have_code_to_deoptimize_(false),
+      last_mark_compact_time_(0.0) {
 }

 #ifdef VERIFY_HEAP
@@ -878,6 +879,8 @@ void MarkCompactCollector::Finish() {
   }

   heap_->incremental_marking()->ClearIdleMarkingDelayCounter();
+  last_mark_compact_time_ =
+      V8::GetCurrentPlatform()->MonotonicallyIncreasingTime();
 }


Index: src/heap/mark-compact.h
diff --git a/src/heap/mark-compact.h b/src/heap/mark-compact.h
index ddb993f5a2f676491e732da2a1a39cae26c48b59..fddffb19a26242e990ad9bec4611e2cc098a23ca 100644
--- a/src/heap/mark-compact.h
+++ b/src/heap/mark-compact.h
@@ -684,6 +684,8 @@ class MarkCompactCollector {
   bool IsSlotInLiveObject(HeapObject** address, HeapObject* object);
   void VerifyIsSlotInLiveObject(HeapObject** address, HeapObject* object);

+  double last_mark_compact_time() const { return last_mark_compact_time_; }
+
  private:
   class SweeperTask;

@@ -910,6 +912,8 @@ class MarkCompactCollector {
   SmartPointer<FreeList> free_list_old_data_space_;
   SmartPointer<FreeList> free_list_old_pointer_space_;

+  double last_mark_compact_time_;
+
   friend class Heap;
 };

Index: test/unittests/heap/gc-idle-time-handler-unittest.cc
diff --git a/test/unittests/heap/gc-idle-time-handler-unittest.cc b/test/unittests/heap/gc-idle-time-handler-unittest.cc index 2076e604faa06d92e29e499c5d12a5eae4420694..6638b31e042e39dddbe6a58c8f61f50efcab4ea8 100644
--- a/test/unittests/heap/gc-idle-time-handler-unittest.cc
+++ b/test/unittests/heap/gc-idle-time-handler-unittest.cc
@@ -34,6 +34,9 @@ class GCIdleTimeHandlerTest : public ::testing::Test {
     result.new_space_capacity = kNewSpaceCapacity;
     result.new_space_allocation_throughput_in_bytes_per_ms =
         kNewSpaceAllocationThroughput;
+    result.last_mark_compact_time = 0;
+    result.current_time =
+        GCIdleTimeHandler::kMinMillisecondsInBetweenMarkCompact;
     return result;
   }

@@ -169,14 +172,24 @@ TEST_F(GCIdleTimeHandlerTest, DoScavengeHighScavengeSpeed) {

 TEST_F(GCIdleTimeHandlerTest, ShouldDoMarkCompact) {
   size_t idle_time_in_ms = 16;
- EXPECT_TRUE(GCIdleTimeHandler::ShouldDoMarkCompact(idle_time_in_ms, 0, 0));
+  EXPECT_TRUE(GCIdleTimeHandler::ShouldDoMarkCompact(
+      idle_time_in_ms, 0, 0, 0,
+      GCIdleTimeHandler::kMinMillisecondsInBetweenMarkCompact));
 }


 TEST_F(GCIdleTimeHandlerTest, DontDoMarkCompact) {
   size_t idle_time_in_ms = 1;
   EXPECT_FALSE(GCIdleTimeHandler::ShouldDoMarkCompact(
-      idle_time_in_ms, kSizeOfObjects, kMarkingSpeed));
+      idle_time_in_ms, kSizeOfObjects, kMarkingSpeed, 0,
+      GCIdleTimeHandler::kMinMillisecondsInBetweenMarkCompact));
+}
+
+
+TEST_F(GCIdleTimeHandlerTest, DontDoMarkCompactRate) {
+  size_t idle_time_in_ms = 16;
+  EXPECT_FALSE(
+      GCIdleTimeHandler::ShouldDoMarkCompact(idle_time_in_ms, 0, 0, 0, 0));
 }




--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to