From 72ffc42d045d6f1bf4e3765f3ac73f6f115edde2 Mon Sep 17 00:00:00 2001 From: "hpayer@chromium.org" Date: Wed, 20 Aug 2014 15:37:43 +0000 Subject: [PATCH] Use size_t in GCIdleTimeHandler to fix undefined behaviour. BUG= R=bmeurer@chromium.org Review URL: https://codereview.chromium.org/490943002 git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@23248 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/heap/gc-idle-time-handler.cc | 22 ++++++++++------------ src/heap/gc-idle-time-handler.h | 9 ++++++--- src/heap/heap.cc | 6 ++++-- src/heap/spaces.cc | 3 +++ test/heap-unittests/heap-unittest.cc | 32 ++++++++++++++++++-------------- 5 files changed, 41 insertions(+), 31 deletions(-) diff --git a/src/heap/gc-idle-time-handler.cc b/src/heap/gc-idle-time-handler.cc index 59791e6..5ff4017 100644 --- a/src/heap/gc-idle-time-handler.cc +++ b/src/heap/gc-idle-time-handler.cc @@ -1,9 +1,6 @@ // 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" @@ -14,8 +11,8 @@ 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) { +size_t GCIdleTimeHandler::EstimateMarkingStepSize( + size_t idle_time_in_ms, size_t marking_speed_in_bytes_per_ms) { DCHECK(idle_time_in_ms > 0); if (marking_speed_in_bytes_per_ms == 0) { @@ -23,16 +20,17 @@ intptr_t GCIdleTimeHandler::EstimateMarkingStepSize( 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) { + size_t marking_step_size = marking_speed_in_bytes_per_ms * idle_time_in_ms; + if (marking_step_size / marking_speed_in_bytes_per_ms != idle_time_in_ms) { // In the case of an overflow we return maximum marking step size. - return INT_MAX; + return GCIdleTimeHandler::kMaximumMarkingStepSize; } - return Min(static_cast(marking_step_size * - GCIdleTimeHandler::kConservativeTimeRatio), - static_cast(INT_MAX)); + if (marking_step_size > kMaximumMarkingStepSize) + return kMaximumMarkingStepSize; + + 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 index cf43b9b..609fdb1 100644 --- a/src/heap/gc-idle-time-handler.h +++ b/src/heap/gc-idle-time-handler.h @@ -14,12 +14,15 @@ namespace internal { // 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); + static size_t EstimateMarkingStepSize(size_t idle_time_in_ms, + size_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; + static const size_t kInitialConservativeMarkingSpeed = 100 * KB; + + // Maximum marking step size returned by EstimateMarkingStepSize. + static const size_t kMaximumMarkingStepSize = 700 * MB; // We have to make sure that we finish the IdleNotification before // idle_time_in_ms. Hence, we conservatively prune our workload estimate. diff --git a/src/heap/heap.cc b/src/heap/heap.cc index 5bd8339..1c966b3 100644 --- a/src/heap/heap.cc +++ b/src/heap/heap.cc @@ -4265,8 +4265,10 @@ void Heap::MakeHeapIterable() { void Heap::AdvanceIdleIncrementalMarking(int idle_time_in_ms) { - intptr_t step_size = GCIdleTimeHandler::EstimateMarkingStepSize( - idle_time_in_ms, tracer_.IncrementalMarkingSpeedInBytesPerMillisecond()); + intptr_t step_size = + static_cast(GCIdleTimeHandler::EstimateMarkingStepSize( + idle_time_in_ms, + tracer_.IncrementalMarkingSpeedInBytesPerMillisecond())); incremental_marking()->Step(step_size, IncrementalMarking::NO_GC_VIA_STACK_GUARD, true); diff --git a/src/heap/spaces.cc b/src/heap/spaces.cc index 72f6b5e..76afef6 100644 --- a/src/heap/spaces.cc +++ b/src/heap/spaces.cc @@ -2738,6 +2738,9 @@ void PagedSpace::ReportStatistics() { Capacity(), Waste(), Available(), pct); if (!swept_precisely_) return; + if (heap()->mark_compact_collector()->sweeping_in_progress()) { + heap()->mark_compact_collector()->EnsureSweepingCompleted(); + } ClearHistograms(heap()->isolate()); HeapObjectIterator obj_it(this); for (HeapObject* obj = obj_it.Next(); obj != NULL; obj = obj_it.Next()) diff --git a/test/heap-unittests/heap-unittest.cc b/test/heap-unittests/heap-unittest.cc index 220d8f2..db331f9 100644 --- a/test/heap-unittests/heap-unittest.cc +++ b/test/heap-unittests/heap-unittest.cc @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include +#include #include "src/heap/gc-idle-time-handler.h" @@ -12,33 +12,37 @@ namespace v8 { namespace internal { TEST(EstimateMarkingStepSizeTest, EstimateMarkingStepSizeInitial) { - intptr_t step_size = GCIdleTimeHandler::EstimateMarkingStepSize(1, 0); - EXPECT_EQ(static_cast( - GCIdleTimeHandler::kInitialConservativeMarkingSpeed * - GCIdleTimeHandler::kConservativeTimeRatio), - step_size); + size_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( + size_t marking_speed_in_bytes_per_millisecond = 100; + size_t step_size = GCIdleTimeHandler::EstimateMarkingStepSize( 1, marking_speed_in_bytes_per_millisecond); - EXPECT_EQ(static_cast(marking_speed_in_bytes_per_millisecond * - GCIdleTimeHandler::kConservativeTimeRatio), + 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); + size_t step_size = GCIdleTimeHandler::EstimateMarkingStepSize( + 10, std::numeric_limits::max()); + EXPECT_EQ(static_cast(GCIdleTimeHandler::kMaximumMarkingStepSize), + step_size); } TEST(EstimateMarkingStepSizeTest, EstimateMarkingStepSizeOverflow2) { - intptr_t step_size = GCIdleTimeHandler::EstimateMarkingStepSize(INT_MAX, 10); - EXPECT_EQ(INT_MAX, step_size); + size_t step_size = GCIdleTimeHandler::EstimateMarkingStepSize( + std::numeric_limits::max(), 10); + EXPECT_EQ(static_cast(GCIdleTimeHandler::kMaximumMarkingStepSize), + step_size); } } // namespace internal -- 2.7.4