Reviewers: Benedikt Meurer,
Description:
Use size_t in GCIdleTimeHandler to fix undefined behaviour.
BUG=
Please review this at https://codereview.chromium.org/490943002/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+39, -30 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/spaces.cc
M test/heap-unittests/heap-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
83e95acc0526f5c46740b1e8ddc4f32619577a14..5ff4017c233f071b742974b9fcf242609de03de3
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 <climits>
-
-#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,15 +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<intptr_t>(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 static_cast<intptr_t>(marking_step_size *
- GCIdleTimeHandler::kConservativeTimeRatio);
+ if (marking_step_size > kMaximumMarkingStepSize)
+ return kMaximumMarkingStepSize;
+
+ return static_cast<size_t>(marking_step_size *
+ GCIdleTimeHandler::kConservativeTimeRatio);
}
}
}
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
cf43b9b0700c0340d855616f0cd8be3ef6b0cf3a..609fdb18ef71e25f0157b931a33d789c32214e61
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.
Index: src/heap/heap.cc
diff --git a/src/heap/heap.cc b/src/heap/heap.cc
index
5bd833960be97a26f44f34c4b7652b91372496b2..1c966b350d2aee49a480e56c4f1c53b8619f25bf
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<size_t>(GCIdleTimeHandler::EstimateMarkingStepSize(
+ idle_time_in_ms,
+ tracer_.IncrementalMarkingSpeedInBytesPerMillisecond()));
incremental_marking()->Step(step_size,
IncrementalMarking::NO_GC_VIA_STACK_GUARD,
true);
Index: src/heap/spaces.cc
diff --git a/src/heap/spaces.cc b/src/heap/spaces.cc
index
9be53e03f284018c8b8cafa6462f82712992fba4..e9a9842b9a27a418957f1ac20915c344fc815412
100644
--- a/src/heap/spaces.cc
+++ b/src/heap/spaces.cc
@@ -2735,6 +2735,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())
Index: test/heap-unittests/heap-unittest.cc
diff --git a/test/heap-unittests/heap-unittest.cc
b/test/heap-unittests/heap-unittest.cc
index
220d8f2dffb7350579e9d489780eb58cc81f53d5..d04811dfa25e25a9925a371b43bd2ceecedc979b
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 <climits>
+#include <limits>
#include "src/heap/gc-idle-time-handler.h"
@@ -12,33 +12,35 @@ namespace v8 {
namespace internal {
TEST(EstimateMarkingStepSizeTest, EstimateMarkingStepSizeInitial) {
- intptr_t step_size = GCIdleTimeHandler::EstimateMarkingStepSize(1, 0);
- EXPECT_EQ(static_cast<intptr_t>(
- GCIdleTimeHandler::kInitialConservativeMarkingSpeed *
- GCIdleTimeHandler::kConservativeTimeRatio),
- step_size);
+ size_t step_size = GCIdleTimeHandler::EstimateMarkingStepSize(1, 0);
+ EXPECT_EQ(
+
static_cast<size_t>(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<intptr_t>(marking_speed_in_bytes_per_millisecond *
-
GCIdleTimeHandler::kConservativeTimeRatio),
+ EXPECT_EQ(static_cast<size_t>(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<size_t>::max());
+ EXPECT_EQ(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<size_t>::max(), 10);
+ EXPECT_EQ(GCIdleTimeHandler::kMaximumMarkingStepSize, step_size);
}
} // namespace internal
--
--
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.
