From f2f8001f1b3df3e7f9492222b3f996af5e64d329 Mon Sep 17 00:00:00 2001 From: ulan Date: Wed, 20 May 2015 05:10:09 -0700 Subject: [PATCH] Take freed handles into account when scheduling idle GCs. BUG= Review URL: https://codereview.chromium.org/1145103002 Cr-Commit-Position: refs/heads/master@{#28508} --- src/heap/gc-idle-time-handler.cc | 3 ++- src/heap/gc-idle-time-handler.h | 8 ++++++- src/heap/heap.cc | 2 +- .../heap/gc-idle-time-handler-unittest.cc | 28 +++++++++++++++++----- 4 files changed, 32 insertions(+), 9 deletions(-) diff --git a/src/heap/gc-idle-time-handler.cc b/src/heap/gc-idle-time-handler.cc index 0a174d5..e88710b 100644 --- a/src/heap/gc-idle-time-handler.cc +++ b/src/heap/gc-idle-time-handler.cc @@ -399,7 +399,8 @@ GCIdleTimeHandler::Mode GCIdleTimeHandler::NextMode( } break; case kReduceMemory: - if (idle_mark_compacts_ >= kMaxIdleMarkCompacts) { + if (idle_mark_compacts_ >= kMaxIdleMarkCompacts || + (idle_mark_compacts_ > 0 && !next_gc_likely_to_collect_more_)) { return kDone; } if (mutator_gcs > idle_mark_compacts_) { diff --git a/src/heap/gc-idle-time-handler.h b/src/heap/gc-idle-time-handler.h index efa4869..5c0a80a 100644 --- a/src/heap/gc-idle-time-handler.h +++ b/src/heap/gc-idle-time-handler.h @@ -197,13 +197,17 @@ class GCIdleTimeHandler { long_idle_notifications_(0), background_idle_notifications_(0), idle_times_which_made_no_progress_per_mode_(0), + next_gc_likely_to_collect_more_(false), mode_(kReduceLatency) {} GCIdleTimeAction Compute(double idle_time_in_ms, HeapState heap_state); void NotifyIdleMarkCompact() { ++idle_mark_compacts_; } - void NotifyMarkCompact() { ++mark_compacts_; } + void NotifyMarkCompact(bool next_gc_likely_to_collect_more) { + next_gc_likely_to_collect_more_ = next_gc_likely_to_collect_more; + ++mark_compacts_; + } void NotifyScavenge() { ++scavenges_; } @@ -261,6 +265,8 @@ class GCIdleTimeHandler { // Idle notifications with no progress in the current mode. int idle_times_which_made_no_progress_per_mode_; + bool next_gc_likely_to_collect_more_; + Mode mode_; DISALLOW_COPY_AND_ASSIGN(GCIdleTimeHandler); diff --git a/src/heap/heap.cc b/src/heap/heap.cc index b585027..88ed748 100644 --- a/src/heap/heap.cc +++ b/src/heap/heap.cc @@ -932,7 +932,7 @@ bool Heap::CollectGarbage(GarbageCollector collector, const char* gc_reason, } if (collector == MARK_COMPACTOR) { - gc_idle_time_handler_.NotifyMarkCompact(); + gc_idle_time_handler_.NotifyMarkCompact(next_gc_likely_to_collect_more); } else { gc_idle_time_handler_.NotifyScavenge(); } diff --git a/test/unittests/heap/gc-idle-time-handler-unittest.cc b/test/unittests/heap/gc-idle-time-handler-unittest.cc index 0f1e498..d4833b3 100644 --- a/test/unittests/heap/gc-idle-time-handler-unittest.cc +++ b/test/unittests/heap/gc-idle-time-handler-unittest.cc @@ -67,7 +67,7 @@ class GCIdleTimeHandlerTest : public ::testing::Test { GCIdleTimeAction action = handler()->Compute(idle_time_ms, heap_state); EXPECT_EQ(expected, action.type); EXPECT_TRUE(action.reduce_memory); - handler()->NotifyMarkCompact(); + handler()->NotifyMarkCompact(true); handler()->NotifyIdleMarkCompact(); } handler()->Compute(idle_time_ms, heap_state); @@ -82,7 +82,7 @@ class GCIdleTimeHandlerTest : public ::testing::Test { for (int i = 0; i < limit; i++) { GCIdleTimeAction action = handler()->Compute(idle_time_ms, heap_state); EXPECT_EQ(DONE, action.type); - handler()->NotifyMarkCompact(); + handler()->NotifyMarkCompact(true); } handler()->Compute(idle_time_ms, heap_state); EXPECT_EQ(GCIdleTimeHandler::kReduceLatency, handler()->mode()); @@ -462,7 +462,7 @@ TEST_F(GCIdleTimeHandlerTest, StopEventually1) { for (int i = 0; i < kMaxNotifications && !stopped; i++) { GCIdleTimeAction action = handler()->Compute(idle_time_ms, heap_state); if (action.type == DO_INCREMENTAL_MARKING || action.type == DO_FULL_GC) { - handler()->NotifyMarkCompact(); + handler()->NotifyMarkCompact(true); handler()->NotifyIdleMarkCompact(); } if (action.type == DONE) stopped = true; @@ -579,7 +579,7 @@ TEST_F(GCIdleTimeHandlerTest, StayInReduceLatencyModeBecauseOfMarkCompacts) { for (int i = 0; i < kMaxNotifications; i++) { GCIdleTimeAction action = handler()->Compute(idle_time_ms, heap_state); EXPECT_TRUE(DO_NOTHING == action.type || DONE == action.type); - if ((i + 1) % limit == 0) handler()->NotifyMarkCompact(); + if ((i + 1) % limit == 0) handler()->NotifyMarkCompact(true); EXPECT_EQ(GCIdleTimeHandler::kReduceLatency, handler()->mode()); } } @@ -604,7 +604,7 @@ TEST_F(GCIdleTimeHandlerTest, ReduceMemoryToReduceLatency) { // ReduceMemory mode should tolerate one mutator GC per idle GC. handler()->NotifyScavenge(); // Notify idle GC. - handler()->NotifyMarkCompact(); + handler()->NotifyMarkCompact(true); handler()->NotifyIdleMarkCompact(); } // Transition to ReduceLatency mode after doing |idle_gc| idle GCs. @@ -635,7 +635,7 @@ TEST_F(GCIdleTimeHandlerTest, ReduceMemoryToDone) { // ReduceMemory mode should tolerate one mutator GC per idle GC. handler()->NotifyScavenge(); // Notify idle GC. - handler()->NotifyMarkCompact(); + handler()->NotifyMarkCompact(true); handler()->NotifyIdleMarkCompact(); } action = handler()->Compute(idle_time_ms, heap_state); @@ -698,5 +698,21 @@ TEST_F(GCIdleTimeHandlerTest, BackgroundReduceLatencyToReduceMemory) { EXPECT_EQ(GCIdleTimeHandler::kReduceMemory, handler()->mode()); } + +TEST_F(GCIdleTimeHandlerTest, SkipUselessGCs) { + GCIdleTimeHandler::HeapState heap_state = DefaultHeapState(); + heap_state.incremental_marking_stopped = false; + heap_state.can_start_incremental_marking = true; + TransitionToReduceMemoryMode(heap_state); + EXPECT_EQ(GCIdleTimeHandler::kReduceMemory, handler()->mode()); + double idle_time_ms = GCIdleTimeHandler::kMinLongIdleTime; + GCIdleTimeAction action = handler()->Compute(idle_time_ms, heap_state); + EXPECT_EQ(DO_INCREMENTAL_MARKING, action.type); + handler()->NotifyMarkCompact(false); + handler()->NotifyIdleMarkCompact(); + action = handler()->Compute(idle_time_ms, heap_state); + EXPECT_EQ(DONE, action.type); +} + } // namespace internal } // namespace v8 -- 2.7.4