Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1149593002:
Move code around in IdleNotification. (Closed)
|
Created:
5 years, 7 months ago by Hannes Payer (out of office) Modified:
5 years, 7 months ago Reviewers:
ulan CC:
v8-dev Base URL:
https://chromium.googlesource.com/v8/v8.git@master Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
DescriptionMove code around in IdleNotification.
BUG=
Committed: https://crrev.com/292a4b04c93db0e221ed73b91a2758ba2b8e59d6
Cr-Commit-Position: refs/heads/master@{#28513}
Patch Set 1 #
Total comments: 1
Patch Set 2 : #Patch Set 3 : #Messages
Total messages: 16 (8 generated)
hpayer@chromium.org changed reviewers: + ulan@chromium.org
https://codereview.chromium.org/1149593002/diff/1/src/heap/heap.cc File src/heap/heap.cc (right): https://codereview.chromium.org/1149593002/diff/1/src/heap/heap.cc#newcode4708 src/heap/heap.cc:4708: if (is_long_idle_notification) { Diff: this got moved down to the Epilogue.
lgtm
The CQ bit was checked by hpayer@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from ulan@chromium.org Link to the patchset: https://codereview.chromium.org/1149593002/#ps20001 (title: " ")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1149593002/20001
The CQ bit was unchecked by hpayer@chromium.org
The CQ bit was checked by hpayer@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from ulan@chromium.org Link to the patchset: https://codereview.chromium.org/1149593002/#ps40001 (title: " ")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1149593002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Failed to apply patch for src/heap/heap.cc:
While running git apply --index -3 -p1;
error: patch failed: src/heap/heap.cc:4586
Falling back to three-way merge...
Applied patch to 'src/heap/heap.cc' with conflicts.
U src/heap/heap.cc
Patch: src/heap/heap.cc
Index: src/heap/heap.cc
diff --git a/src/heap/heap.cc b/src/heap/heap.cc
index
b5850271170967281dceddf308021fc936ec7598..07c76e68447a2951e1bcadf736ffd761f22fec95
100644
--- a/src/heap/heap.cc
+++ b/src/heap/heap.cc
@@ -4586,35 +4586,7 @@ bool Heap::TryFinalizeIdleIncrementalMarking(
}
-double Heap::MonotonicallyIncreasingTimeInMs() {
- return V8::GetCurrentPlatform()->MonotonicallyIncreasingTime() *
- static_cast<double>(base::Time::kMillisecondsPerSecond);
-}
-
-
-bool Heap::IdleNotification(int idle_time_in_ms) {
- return IdleNotification(
- V8::GetCurrentPlatform()->MonotonicallyIncreasingTime() +
- (static_cast<double>(idle_time_in_ms) /
- static_cast<double>(base::Time::kMillisecondsPerSecond)));
-}
-
-
-bool Heap::IdleNotification(double deadline_in_seconds) {
- CHECK(HasBeenSetUp()); // http://crbug.com/425035
- double deadline_in_ms =
- deadline_in_seconds *
- static_cast<double>(base::Time::kMillisecondsPerSecond);
- HistogramTimerScope idle_notification_scope(
- isolate_->counters()->gc_idle_notification());
- double start_ms = MonotonicallyIncreasingTimeInMs();
- double idle_time_in_ms = deadline_in_ms - start_ms;
- bool is_long_idle_notification =
- static_cast<size_t>(idle_time_in_ms) >
- GCIdleTimeHandler::kMaxFrameRenderingIdleTime;
-
- static const double kLastGCTimeTreshold = 1000;
-
+GCIdleTimeHandler::HeapState Heap::ComputeHeapState(bool reduce_memory) {
GCIdleTimeHandler::HeapState heap_state;
heap_state.contexts_disposed = contexts_disposed_;
heap_state.contexts_disposal_rate =
@@ -4623,8 +4595,7 @@ bool Heap::IdleNotification(double deadline_in_seconds) {
heap_state.incremental_marking_stopped = incremental_marking()->IsStopped();
// TODO(ulan): Start incremental marking only for large heaps.
intptr_t limit = old_generation_allocation_limit_;
- if (is_long_idle_notification &&
- (start_ms - last_gc_time_ > kLastGCTimeTreshold)) {
+ if (reduce_memory) {
limit = idle_old_generation_allocation_limit_;
}
@@ -4650,21 +4621,14 @@ bool Heap::IdleNotification(double deadline_in_seconds)
{
heap_state.new_space_allocation_throughput_in_bytes_per_ms =
static_cast<size_t>(
tracer()->NewSpaceAllocationThroughputInBytesPerMillisecond());
+ return heap_state;
+}
- GCIdleTimeAction action =
- gc_idle_time_handler_.Compute(idle_time_in_ms, heap_state);
-
- isolate()->counters()->gc_idle_time_allotted_in_ms()->AddSample(
- static_cast<int>(idle_time_in_ms));
- if (is_long_idle_notification) {
- int committed_memory = static_cast<int>(CommittedMemory() / KB);
- int used_memory = static_cast<int>(heap_state.size_of_objects / KB);
- isolate()->counters()->aggregated_memory_heap_committed()->AddSample(
- start_ms, committed_memory);
- isolate()->counters()->aggregated_memory_heap_used()->AddSample(
- start_ms, used_memory);
- }
+bool Heap::PerformIdleTimeAction(GCIdleTimeAction action,
+ GCIdleTimeHandler::HeapState heap_state,
+ double deadline_in_ms,
+ bool is_long_idle_notification) {
bool result = false;
switch (action.type) {
case DONE:
@@ -4723,11 +4687,33 @@ bool Heap::IdleNotification(double deadline_in_seconds)
{
new_space_.Shrink();
UncommitFromSpace();
}
+ return result;
+}
+
+void Heap::IdleNotificationEpilogue(GCIdleTimeAction action,
+ GCIdleTimeHandler::HeapState heap_state,
+ double start_ms, double deadline_in_ms,
+ bool is_long_idle_notification) {
+ double idle_time_in_ms = deadline_in_ms - start_ms;
double current_time = MonotonicallyIncreasingTimeInMs();
last_idle_notification_time_ = current_time;
double deadline_difference = deadline_in_ms - current_time;
+ contexts_disposed_ = 0;
+
+ isolate()->counters()->gc_idle_time_allotted_in_ms()->AddSample(
+ static_cast<int>(idle_time_in_ms));
+
+ if (is_long_idle_notification) {
+ int committed_memory = static_cast<int>(CommittedMemory() / KB);
+ int used_memory = static_cast<int>(heap_state.size_of_objects / KB);
+ isolate()->counters()->aggregated_memory_heap_committed()->AddSample(
+ start_ms, committed_memory);
+ isolate()->counters()->aggregated_memory_heap_used()->AddSample(
+ start_ms, used_memory);
+ }
+
if (deadline_difference >= 0) {
if (action.type != DONE && action.type != DO_NOTHING) {
isolate()->counters()->gc_idle_time_limit_undershot()->AddSample(
@@ -4755,8 +4741,49 @@ bool Heap::IdleNotification(double deadline_in_seconds) {
}
PrintF("\n");
}
+}
- contexts_disposed_ = 0;
+
+double Heap::MonotonicallyIncreasingTimeInMs() {
+ return V8::GetCurrentPlatform()->MonotonicallyIncreasingTime() *
+ static_cast<double>(base::Time::kMillisecondsPerSecond);
+}
+
+
+bool Heap::IdleNotification(int idle_time_in_ms) {
+ return IdleNotification(
+ V8::GetCurrentPlatform()->MonotonicallyIncreasingTime() +
+ (static_cast<double>(idle_time_in_ms) /
+ static_cast<double>(base::Time::kMillisecondsPerSecond)));
+}
+
+
+bool Heap::IdleNotification(double deadline_in_seconds) {
+ CHECK(HasBeenSetUp());
+ static const double kLastGCTimeTreshold = 1000;
+ double deadline_in_ms =
+ deadline_in_seconds *
+ static_cast<double>(base::Time::kMillisecondsPerSecond);
+ HistogramTimerScope idle_notification_scope(
+ isolate_->counters()->gc_idle_notification());
+ double start_ms = MonotonicallyIncreasingTimeInMs();
+ double idle_time_in_ms = deadline_in_ms - start_ms;
+ bool is_long_idle_notification =
+ static_cast<size_t>(idle_time_in_ms) >
+ GCIdleTimeHandler::kMaxFrameRenderingIdleTime;
+ bool has_low_gc_activity = (start_ms - last_gc_time_) > kLastGCTimeTreshold;
+
+ GCIdleTimeHandler::HeapState heap_state =
+ ComputeHeapState(is_long_idle_notification && has_low_gc_activity);
+
+ GCIdleTimeAction action =
+ gc_idle_time_handler_.Compute(idle_time_in_ms, heap_state);
+
+ bool result = PerformIdleTimeAction(action, heap_state, deadline_in_ms,
+ is_long_idle_notification);
+
+ IdleNotificationEpilogue(action, heap_state, start_ms, deadline_in_ms,
+ is_long_idle_notification);
return result;
}
The CQ bit was checked by hpayer@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1149593002/40001
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/292a4b04c93db0e221ed73b91a2758ba2b8e59d6 Cr-Commit-Position: refs/heads/master@{#28513} |
