From ca5780690ddf3bd79723a2eb492e797fe64d5405 Mon Sep 17 00:00:00 2001 From: mstarzinger Date: Fri, 25 Sep 2015 06:55:11 -0700 Subject: [PATCH] [heap] No leakage of gc-idle-time-handler.h outside of heap. This prevents the internal gc-idle-time-handler.h to be usable outisde of the "heap" directory. The logic inside that component is only useful within the GC and is now properly encapsulated. R=ulan@chromium.org Review URL: https://codereview.chromium.org/1368983002 Cr-Commit-Position: refs/heads/master@{#30939} --- src/heap/gc-idle-time-handler.cc | 4 +- src/heap/gc-idle-time-handler.h | 44 ++++++++++----------- src/heap/heap.cc | 24 ++++++----- src/heap/heap.h | 15 ++++--- .../heap/gc-idle-time-handler-unittest.cc | 46 +++++++++++----------- 5 files changed, 71 insertions(+), 62 deletions(-) diff --git a/src/heap/gc-idle-time-handler.cc b/src/heap/gc-idle-time-handler.cc index f9783b3..f836a06 100644 --- a/src/heap/gc-idle-time-handler.cc +++ b/src/heap/gc-idle-time-handler.cc @@ -42,7 +42,7 @@ void GCIdleTimeAction::Print() { } -void GCIdleTimeHandler::HeapState::Print() { +void GCIdleTimeHeapState::Print() { PrintF("contexts_disposed=%d ", contexts_disposed); PrintF("contexts_disposal_rate=%f ", contexts_disposal_rate); PrintF("size_of_objects=%" V8_PTR_PREFIX "d ", size_of_objects); @@ -240,7 +240,7 @@ GCIdleTimeAction GCIdleTimeHandler::NothingOrDone(double idle_time_in_ms) { // (5) If incremental marking is in progress, we perform a marking step. Note, // that this currently may trigger a full garbage collection. GCIdleTimeAction GCIdleTimeHandler::Compute(double idle_time_in_ms, - HeapState heap_state) { + GCIdleTimeHeapState heap_state) { if (static_cast(idle_time_in_ms) <= 0) { if (heap_state.incremental_marking_stopped) { if (ShouldDoContextDisposalMarkCompact( diff --git a/src/heap/gc-idle-time-handler.h b/src/heap/gc-idle-time-handler.h index 3f7f02d..ce4aeee 100644 --- a/src/heap/gc-idle-time-handler.h +++ b/src/heap/gc-idle-time-handler.h @@ -63,7 +63,26 @@ class GCIdleTimeAction { }; -class GCTracer; +class GCIdleTimeHeapState { + public: + void Print(); + + int contexts_disposed; + double contexts_disposal_rate; + size_t size_of_objects; + bool incremental_marking_stopped; + bool sweeping_in_progress; + bool sweeping_completed; + bool has_low_allocation_rate; + size_t mark_compact_speed_in_bytes_per_ms; + size_t incremental_marking_speed_in_bytes_per_ms; + size_t final_incremental_mark_compact_speed_in_bytes_per_ms; + size_t scavenge_speed_in_bytes_per_ms; + size_t used_new_space_size; + size_t new_space_capacity; + size_t new_space_allocation_throughput_in_bytes_per_ms; +}; + // The idle time handler makes decisions about which garbage collection // operations are executing during IdleNotification. @@ -133,29 +152,10 @@ class GCIdleTimeHandler { // ensure we don't keep scheduling idle tasks and making no progress. static const int kMaxNoProgressIdleTimes = 10; - class HeapState { - public: - void Print(); - - int contexts_disposed; - double contexts_disposal_rate; - size_t size_of_objects; - bool incremental_marking_stopped; - bool sweeping_in_progress; - bool sweeping_completed; - bool has_low_allocation_rate; - size_t mark_compact_speed_in_bytes_per_ms; - size_t incremental_marking_speed_in_bytes_per_ms; - size_t final_incremental_mark_compact_speed_in_bytes_per_ms; - size_t scavenge_speed_in_bytes_per_ms; - size_t used_new_space_size; - size_t new_space_capacity; - size_t new_space_allocation_throughput_in_bytes_per_ms; - }; - GCIdleTimeHandler() : idle_times_which_made_no_progress_(0) {} - GCIdleTimeAction Compute(double idle_time_in_ms, HeapState heap_state); + GCIdleTimeAction Compute(double idle_time_in_ms, + GCIdleTimeHeapState heap_state); void ResetNoProgressCounter() { idle_times_which_made_no_progress_ = 0; } diff --git a/src/heap/heap.cc b/src/heap/heap.cc index bafce6f..ae77e1e 100644 --- a/src/heap/heap.cc +++ b/src/heap/heap.cc @@ -125,6 +125,7 @@ Heap::Heap() mark_compact_collector_(this), store_buffer_(this), incremental_marking_(this), + gc_idle_time_handler_(nullptr), memory_reducer_(nullptr), object_stats_(nullptr), full_codegen_bytes_generated_(0), @@ -1012,7 +1013,7 @@ void Heap::StartIncrementalMarking(int gc_flags, void Heap::StartIdleIncrementalMarking() { - gc_idle_time_handler_.ResetNoProgressCounter(); + gc_idle_time_handler_->ResetNoProgressCounter(); StartIncrementalMarking(kReduceMemoryFootprintMask, kNoGCCallbackFlags, "idle"); } @@ -4071,14 +4072,14 @@ bool Heap::TryFinalizeIdleIncrementalMarking(double idle_time_in_ms) { (incremental_marking()->IsReadyToOverApproximateWeakClosure() || (!incremental_marking()->weak_closure_was_overapproximated() && mark_compact_collector_.marking_deque()->IsEmpty() && - gc_idle_time_handler_.ShouldDoOverApproximateWeakClosure( + gc_idle_time_handler_->ShouldDoOverApproximateWeakClosure( static_cast(idle_time_in_ms))))) { OverApproximateWeakClosure( "Idle notification: overapproximate weak closure"); return true; } else if (incremental_marking()->IsComplete() || (mark_compact_collector_.marking_deque()->IsEmpty() && - gc_idle_time_handler_.ShouldDoFinalIncrementalMarkCompact( + gc_idle_time_handler_->ShouldDoFinalIncrementalMarkCompact( static_cast(idle_time_in_ms), size_of_objects, final_incremental_mark_compact_speed_in_bytes_per_ms))) { CollectAllGarbage(current_gc_flags_, @@ -4089,8 +4090,8 @@ bool Heap::TryFinalizeIdleIncrementalMarking(double idle_time_in_ms) { } -GCIdleTimeHandler::HeapState Heap::ComputeHeapState() { - GCIdleTimeHandler::HeapState heap_state; +GCIdleTimeHeapState Heap::ComputeHeapState() { + GCIdleTimeHeapState heap_state; heap_state.contexts_disposed = contexts_disposed_; heap_state.contexts_disposal_rate = tracer()->ContextDisposalRateInMilliseconds(); @@ -4134,7 +4135,7 @@ double Heap::AdvanceIncrementalMarking( bool Heap::PerformIdleTimeAction(GCIdleTimeAction action, - GCIdleTimeHandler::HeapState heap_state, + GCIdleTimeHeapState heap_state, double deadline_in_ms) { bool result = false; switch (action.type) { @@ -4171,7 +4172,7 @@ bool Heap::PerformIdleTimeAction(GCIdleTimeAction action, void Heap::IdleNotificationEpilogue(GCIdleTimeAction action, - GCIdleTimeHandler::HeapState heap_state, + GCIdleTimeHeapState heap_state, double start_ms, double deadline_in_ms) { double idle_time_in_ms = deadline_in_ms - start_ms; double current_time = MonotonicallyIncreasingTimeInMs(); @@ -4268,10 +4269,10 @@ bool Heap::IdleNotification(double deadline_in_seconds) { tracer()->SampleAllocation(start_ms, NewSpaceAllocationCounter(), OldGenerationAllocationCounter()); - GCIdleTimeHandler::HeapState heap_state = ComputeHeapState(); + GCIdleTimeHeapState heap_state = ComputeHeapState(); GCIdleTimeAction action = - gc_idle_time_handler_.Compute(idle_time_in_ms, heap_state); + gc_idle_time_handler_->Compute(idle_time_in_ms, heap_state); bool result = PerformIdleTimeAction(action, heap_state, deadline_in_ms); @@ -5073,6 +5074,8 @@ bool Heap::SetUp() { scavenge_collector_ = new Scavenger(this); + gc_idle_time_handler_ = new GCIdleTimeHandler(); + memory_reducer_ = new MemoryReducer(this); object_stats_ = new ObjectStats(this); @@ -5187,6 +5190,9 @@ void Heap::TearDown() { delete scavenge_collector_; scavenge_collector_ = nullptr; + delete gc_idle_time_handler_; + gc_idle_time_handler_ = nullptr; + if (memory_reducer_ != nullptr) { memory_reducer_->TearDown(); delete memory_reducer_; diff --git a/src/heap/heap.h b/src/heap/heap.h index 442d4de..8fdc670 100644 --- a/src/heap/heap.h +++ b/src/heap/heap.h @@ -12,7 +12,6 @@ #include "src/assert-scope.h" #include "src/atomic-utils.h" #include "src/globals.h" -#include "src/heap/gc-idle-time-handler.h" #include "src/heap/incremental-marking.h" #include "src/heap/mark-compact.h" #include "src/heap/spaces.h" @@ -422,6 +421,10 @@ namespace internal { // Forward declarations. class ArrayBufferTracker; +class GCIdleTimeAction; +class GCIdleTimeHandler; +class GCIdleTimeHeapState; +class GCTracer; class HeapObjectsFilter; class HeapStats; class Isolate; @@ -1780,15 +1783,15 @@ class Heap { double idle_time_in_ms, size_t size_of_objects, size_t mark_compact_speed_in_bytes_per_ms); - GCIdleTimeHandler::HeapState ComputeHeapState(); + GCIdleTimeHeapState ComputeHeapState(); bool PerformIdleTimeAction(GCIdleTimeAction action, - GCIdleTimeHandler::HeapState heap_state, + GCIdleTimeHeapState heap_state, double deadline_in_ms); void IdleNotificationEpilogue(GCIdleTimeAction action, - GCIdleTimeHandler::HeapState heap_state, - double start_ms, double deadline_in_ms); + GCIdleTimeHeapState heap_state, double start_ms, + double deadline_in_ms); void CheckAndNotifyBackgroundIdleNotification(double idle_time_in_ms, double now_ms); @@ -2258,7 +2261,7 @@ class Heap { IncrementalMarking incremental_marking_; - GCIdleTimeHandler gc_idle_time_handler_; + GCIdleTimeHandler* gc_idle_time_handler_; MemoryReducer* memory_reducer_; diff --git a/test/unittests/heap/gc-idle-time-handler-unittest.cc b/test/unittests/heap/gc-idle-time-handler-unittest.cc index d87a921..cbd5741 100644 --- a/test/unittests/heap/gc-idle-time-handler-unittest.cc +++ b/test/unittests/heap/gc-idle-time-handler-unittest.cc @@ -19,8 +19,8 @@ class GCIdleTimeHandlerTest : public ::testing::Test { GCIdleTimeHandler* handler() { return &handler_; } - GCIdleTimeHandler::HeapState DefaultHeapState() { - GCIdleTimeHandler::HeapState result; + GCIdleTimeHeapState DefaultHeapState() { + GCIdleTimeHeapState result; result.contexts_disposed = 0; result.contexts_disposal_rate = GCIdleTimeHandler::kHighContextDisposalRate; result.incremental_marking_stopped = false; @@ -108,7 +108,7 @@ TEST(GCIdleTimeHandler, EstimateMarkCompactTimeMax) { TEST_F(GCIdleTimeHandlerTest, DoScavengeEmptyNewSpace) { - GCIdleTimeHandler::HeapState heap_state = DefaultHeapState(); + GCIdleTimeHeapState heap_state = DefaultHeapState(); int idle_time_ms = 16; EXPECT_FALSE(GCIdleTimeHandler::ShouldDoScavenge( idle_time_ms, heap_state.new_space_capacity, @@ -118,7 +118,7 @@ TEST_F(GCIdleTimeHandlerTest, DoScavengeEmptyNewSpace) { TEST_F(GCIdleTimeHandlerTest, DoScavengeFullNewSpace) { - GCIdleTimeHandler::HeapState heap_state = DefaultHeapState(); + GCIdleTimeHeapState heap_state = DefaultHeapState(); heap_state.used_new_space_size = kNewSpaceCapacity; int idle_time_ms = 16; EXPECT_TRUE(GCIdleTimeHandler::ShouldDoScavenge( @@ -129,7 +129,7 @@ TEST_F(GCIdleTimeHandlerTest, DoScavengeFullNewSpace) { TEST_F(GCIdleTimeHandlerTest, DoScavengeUnknownScavengeSpeed) { - GCIdleTimeHandler::HeapState heap_state = DefaultHeapState(); + GCIdleTimeHeapState heap_state = DefaultHeapState(); heap_state.used_new_space_size = kNewSpaceCapacity; heap_state.scavenge_speed_in_bytes_per_ms = 0; int idle_time_ms = 8; @@ -141,7 +141,7 @@ TEST_F(GCIdleTimeHandlerTest, DoScavengeUnknownScavengeSpeed) { TEST_F(GCIdleTimeHandlerTest, DoScavengeLowScavengeSpeed) { - GCIdleTimeHandler::HeapState heap_state = DefaultHeapState(); + GCIdleTimeHeapState heap_state = DefaultHeapState(); heap_state.used_new_space_size = kNewSpaceCapacity; heap_state.scavenge_speed_in_bytes_per_ms = 1 * KB; int idle_time_ms = 16; @@ -153,7 +153,7 @@ TEST_F(GCIdleTimeHandlerTest, DoScavengeLowScavengeSpeed) { TEST_F(GCIdleTimeHandlerTest, DoScavengeLowAllocationRate) { - GCIdleTimeHandler::HeapState heap_state = DefaultHeapState(); + GCIdleTimeHeapState heap_state = DefaultHeapState(); heap_state.used_new_space_size = kNewSpaceCapacity; heap_state.new_space_allocation_throughput_in_bytes_per_ms = GCIdleTimeHandler::kLowAllocationThroughput - 1; @@ -166,7 +166,7 @@ TEST_F(GCIdleTimeHandlerTest, DoScavengeLowAllocationRate) { TEST_F(GCIdleTimeHandlerTest, DoScavengeHighScavengeSpeed) { - GCIdleTimeHandler::HeapState heap_state = DefaultHeapState(); + GCIdleTimeHeapState heap_state = DefaultHeapState(); heap_state.used_new_space_size = kNewSpaceCapacity; heap_state.scavenge_speed_in_bytes_per_ms = kNewSpaceCapacity; int idle_time_ms = 16; @@ -178,7 +178,7 @@ TEST_F(GCIdleTimeHandlerTest, DoScavengeHighScavengeSpeed) { TEST_F(GCIdleTimeHandlerTest, DoNotScavengeSmallNewSpaceSize) { - GCIdleTimeHandler::HeapState heap_state = DefaultHeapState(); + GCIdleTimeHeapState heap_state = DefaultHeapState(); heap_state.used_new_space_size = (MB / 2) - 1; heap_state.scavenge_speed_in_bytes_per_ms = kNewSpaceCapacity; int idle_time_ms = 16; @@ -217,7 +217,7 @@ TEST_F(GCIdleTimeHandlerTest, DontDoFinalIncrementalMarkCompact) { TEST_F(GCIdleTimeHandlerTest, ContextDisposeLowRate) { - GCIdleTimeHandler::HeapState heap_state = DefaultHeapState(); + GCIdleTimeHeapState heap_state = DefaultHeapState(); heap_state.contexts_disposed = 1; heap_state.incremental_marking_stopped = true; double idle_time_ms = 0; @@ -227,7 +227,7 @@ TEST_F(GCIdleTimeHandlerTest, ContextDisposeLowRate) { TEST_F(GCIdleTimeHandlerTest, ContextDisposeHighRate) { - GCIdleTimeHandler::HeapState heap_state = DefaultHeapState(); + GCIdleTimeHeapState heap_state = DefaultHeapState(); heap_state.contexts_disposed = 1; heap_state.contexts_disposal_rate = GCIdleTimeHandler::kHighContextDisposalRate - 1; @@ -239,7 +239,7 @@ TEST_F(GCIdleTimeHandlerTest, ContextDisposeHighRate) { TEST_F(GCIdleTimeHandlerTest, AfterContextDisposeZeroIdleTime) { - GCIdleTimeHandler::HeapState heap_state = DefaultHeapState(); + GCIdleTimeHeapState heap_state = DefaultHeapState(); heap_state.contexts_disposed = 1; heap_state.contexts_disposal_rate = 1.0; heap_state.incremental_marking_stopped = true; @@ -250,7 +250,7 @@ TEST_F(GCIdleTimeHandlerTest, AfterContextDisposeZeroIdleTime) { TEST_F(GCIdleTimeHandlerTest, AfterContextDisposeSmallIdleTime1) { - GCIdleTimeHandler::HeapState heap_state = DefaultHeapState(); + GCIdleTimeHeapState heap_state = DefaultHeapState(); heap_state.contexts_disposed = 1; heap_state.contexts_disposal_rate = GCIdleTimeHandler::kHighContextDisposalRate; @@ -262,7 +262,7 @@ TEST_F(GCIdleTimeHandlerTest, AfterContextDisposeSmallIdleTime1) { TEST_F(GCIdleTimeHandlerTest, AfterContextDisposeSmallIdleTime2) { - GCIdleTimeHandler::HeapState heap_state = DefaultHeapState(); + GCIdleTimeHeapState heap_state = DefaultHeapState(); heap_state.contexts_disposed = 1; heap_state.contexts_disposal_rate = GCIdleTimeHandler::kHighContextDisposalRate; @@ -274,7 +274,7 @@ TEST_F(GCIdleTimeHandlerTest, AfterContextDisposeSmallIdleTime2) { TEST_F(GCIdleTimeHandlerTest, IncrementalMarking1) { - GCIdleTimeHandler::HeapState heap_state = DefaultHeapState(); + GCIdleTimeHeapState heap_state = DefaultHeapState(); double idle_time_ms = 10; GCIdleTimeAction action = handler()->Compute(idle_time_ms, heap_state); EXPECT_EQ(DO_INCREMENTAL_STEP, action.type); @@ -282,7 +282,7 @@ TEST_F(GCIdleTimeHandlerTest, IncrementalMarking1) { TEST_F(GCIdleTimeHandlerTest, NotEnoughTime) { - GCIdleTimeHandler::HeapState heap_state = DefaultHeapState(); + GCIdleTimeHeapState heap_state = DefaultHeapState(); heap_state.incremental_marking_stopped = true; size_t speed = heap_state.mark_compact_speed_in_bytes_per_ms; double idle_time_ms = static_cast(kSizeOfObjects / speed - 1); @@ -292,7 +292,7 @@ TEST_F(GCIdleTimeHandlerTest, NotEnoughTime) { TEST_F(GCIdleTimeHandlerTest, Scavenge) { - GCIdleTimeHandler::HeapState heap_state = DefaultHeapState(); + GCIdleTimeHeapState heap_state = DefaultHeapState(); int idle_time_ms = 10; heap_state.used_new_space_size = heap_state.new_space_capacity - @@ -305,7 +305,7 @@ TEST_F(GCIdleTimeHandlerTest, Scavenge) { TEST_F(GCIdleTimeHandlerTest, ScavengeAndDone) { - GCIdleTimeHandler::HeapState heap_state = DefaultHeapState(); + GCIdleTimeHeapState heap_state = DefaultHeapState(); int idle_time_ms = 10; heap_state.incremental_marking_stopped = true; heap_state.used_new_space_size = @@ -321,7 +321,7 @@ TEST_F(GCIdleTimeHandlerTest, ScavengeAndDone) { TEST_F(GCIdleTimeHandlerTest, DoNotStartIncrementalMarking) { - GCIdleTimeHandler::HeapState heap_state = DefaultHeapState(); + GCIdleTimeHeapState heap_state = DefaultHeapState(); heap_state.incremental_marking_stopped = true; double idle_time_ms = 10.0; GCIdleTimeAction action = handler()->Compute(idle_time_ms, heap_state); @@ -330,7 +330,7 @@ TEST_F(GCIdleTimeHandlerTest, DoNotStartIncrementalMarking) { TEST_F(GCIdleTimeHandlerTest, ContinueAfterStop) { - GCIdleTimeHandler::HeapState heap_state = DefaultHeapState(); + GCIdleTimeHeapState heap_state = DefaultHeapState(); heap_state.incremental_marking_stopped = true; double idle_time_ms = 10.0; GCIdleTimeAction action = handler()->Compute(idle_time_ms, heap_state); @@ -342,7 +342,7 @@ TEST_F(GCIdleTimeHandlerTest, ContinueAfterStop) { TEST_F(GCIdleTimeHandlerTest, ZeroIdleTimeNothingToDo) { - GCIdleTimeHandler::HeapState heap_state = DefaultHeapState(); + GCIdleTimeHeapState heap_state = DefaultHeapState(); for (int i = 0; i < kMaxNotifications; i++) { GCIdleTimeAction action = handler()->Compute(0, heap_state); EXPECT_EQ(DO_NOTHING, action.type); @@ -351,7 +351,7 @@ TEST_F(GCIdleTimeHandlerTest, ZeroIdleTimeNothingToDo) { TEST_F(GCIdleTimeHandlerTest, SmallIdleTimeNothingToDo) { - GCIdleTimeHandler::HeapState heap_state = DefaultHeapState(); + GCIdleTimeHeapState heap_state = DefaultHeapState(); heap_state.incremental_marking_stopped = true; for (int i = 0; i < kMaxNotifications; i++) { GCIdleTimeAction action = handler()->Compute(10, heap_state); @@ -362,7 +362,7 @@ TEST_F(GCIdleTimeHandlerTest, SmallIdleTimeNothingToDo) { TEST_F(GCIdleTimeHandlerTest, DoneIfNotMakingProgressOnIncrementalMarking) { // Regression test for crbug.com/489323. - GCIdleTimeHandler::HeapState heap_state = DefaultHeapState(); + GCIdleTimeHeapState heap_state = DefaultHeapState(); // Simulate incremental marking stopped and not eligible to start. heap_state.incremental_marking_stopped = true; -- 2.7.4