From 1605474d709c6aa47d8ee964fd9327797d9ecbbe Mon Sep 17 00:00:00 2001 From: "hpayer@chromium.org" Date: Wed, 20 Aug 2014 10:33:03 +0000 Subject: [PATCH] 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 | 2 ++ src/heap/gc-idle-time-handler.cc | 37 ++++++++++++++++++++++++++++++++++++ src/heap/gc-idle-time-handler.h | 35 ++++++++++++++++++++++++++++++++++ src/heap/heap.cc | 36 +++++++++++++++-------------------- src/heap/heap.h | 4 ++-- test/heap-unittests/heap-unittest.cc | 35 +++++++++++++++++++++++++++++++--- tools/gyp/v8.gyp | 2 ++ 7 files changed, 125 insertions(+), 26 deletions(-) create mode 100644 src/heap/gc-idle-time-handler.cc create mode 100644 src/heap/gc-idle-time-handler.h diff --git a/BUILD.gn b/BUILD.gn index ea18242..087f373 100644 --- a/BUILD.gn +++ b/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", diff --git a/src/heap/gc-idle-time-handler.cc b/src/heap/gc-idle-time-handler.cc new file mode 100644 index 0000000..83e95ac --- /dev/null +++ b/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 + +#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(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(marking_step_size * + GCIdleTimeHandler::kConservativeTimeRatio); +} +} +} diff --git a/src/heap/gc-idle-time-handler.h b/src/heap/gc-idle-time-handler.h new file mode 100644 index 0000000..cf43b9b --- /dev/null +++ b/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_ diff --git a/src/heap/heap.cc b/src/heap/heap.cc index 5d3bde4..5bd8339 100644 --- a/src/heap/heap.cc +++ b/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; - // The size factor is in range [5..250]. The numbers here are chosen from - // 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); + isolate()->counters()->gc_idle_time_allotted_in_ms()->AddSample( + idle_time_in_ms); 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() && - hint > kMinHintForIncrementalMarking) { - AdvanceIdleIncrementalMarking(step_size); + if (!incremental_marking()->IsStopped()) { + AdvanceIdleIncrementalMarking(idle_time_in_ms); } 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(); } diff --git a/src/heap/heap.h b/src/heap/heap.h index 093660a..ef947c9 100644 --- a/src/heap/heap.h +++ b/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); diff --git a/test/heap-unittests/heap-unittest.cc b/test/heap-unittests/heap-unittest.cc index ee1dcbc..220d8f2 100644 --- a/test/heap-unittests/heap-unittest.cc +++ b/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 + +#include "src/heap/gc-idle-time-handler.h" + #include "testing/gtest/include/gtest/gtest.h" namespace v8 { namespace internal { -TEST(HeapTest, Dummy) { - EXPECT_FALSE(false); - EXPECT_TRUE(true); +TEST(EstimateMarkingStepSizeTest, EstimateMarkingStepSizeInitial) { + intptr_t step_size = GCIdleTimeHandler::EstimateMarkingStepSize(1, 0); + EXPECT_EQ(static_cast( + 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(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 diff --git a/tools/gyp/v8.gyp b/tools/gyp/v8.gyp index 3b83285..01ca799 100644 --- a/tools/gyp/v8.gyp +++ b/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', -- 2.7.4