Reland "Make sure that memory reducer makes progress in incremental marking""

This reverts commit b06a6a891cc762835577bb856f8c5e8f0bf8ab31.

BUG=chromium:519319,chromium:515873
LOG=NO

Review URL: https://codereview.chromium.org/1311993003

Cr-Commit-Position: refs/heads/master@{#30569}

diff --git a/src/heap/heap.cc b/src/heap/heap.cc
index 0c47d8c..3d8f449 100644 (file)
--- a/src/heap/heap.cc
+++ b/src/heap/heap.cc
@@ -4622,10 +4622,23 @@ void Heap::ReduceNewSpaceSize() {
 }
 
 
+void Heap::FinalizeIncrementalMarkingIfComplete(const char* comment) {
+  if (FLAG_overapproximate_weak_closure && incremental_marking()->IsMarking() &&
+      (incremental_marking()->IsReadyToOverApproximateWeakClosure() ||
+       (!incremental_marking()->weak_closure_was_overapproximated() &&
+        mark_compact_collector_.marking_deque()->IsEmpty()))) {
+    OverApproximateWeakClosure(comment);
+  } else if (incremental_marking()->IsComplete() ||
+             (mark_compact_collector_.marking_deque()->IsEmpty())) {
+    CollectAllGarbage(current_gc_flags_, comment);
+  }
+}
+
+
 bool Heap::TryFinalizeIdleIncrementalMarking(
     double idle_time_in_ms, size_t size_of_objects,
     size_t final_incremental_mark_compact_speed_in_bytes_per_ms) {
-  if (FLAG_overapproximate_weak_closure &&
+  if (FLAG_overapproximate_weak_closure && incremental_marking()->IsMarking() &&
       (incremental_marking()->IsReadyToOverApproximateWeakClosure() ||
        (!incremental_marking()->weak_closure_was_overapproximated() &&
         mark_compact_collector_.marking_deque()->IsEmpty() &&

diff --git a/src/heap/heap.h b/src/heap/heap.h
index 6e3b4cf..2f9bf0c 100644 (file)
--- a/src/heap/heap.h
+++ b/src/heap/heap.h
@@ -1034,6 +1034,8 @@ class Heap {
   bool HasHighFragmentation();
   bool HasHighFragmentation(intptr_t used, intptr_t committed);
 
+  bool ShouldOptimizeForMemoryUsage() { return optimize_for_memory_usage_; }
+
   // ===========================================================================
   // Initialization. ===========================================================
   // ===========================================================================
@@ -1292,6 +1294,8 @@ class Heap {
       intptr_t step_size_in_bytes, double deadline_in_ms,
       IncrementalMarking::StepActions step_actions);
 
+  void FinalizeIncrementalMarkingIfComplete(const char* comment);
+
   IncrementalMarking* incremental_marking() { return &incremental_marking_; }
 
   // ===========================================================================

diff --git a/src/heap/incremental-marking.cc b/src/heap/incremental-marking.cc
index 009f4ca..b420cb7 100644 (file)
--- a/src/heap/incremental-marking.cc
+++ b/src/heap/incremental-marking.cc
@@ -541,6 +541,7 @@ void IncrementalMarking::StartMarking() {
 void IncrementalMarking::MarkObjectGroups() {
   DCHECK(FLAG_overapproximate_weak_closure);
   DCHECK(!weak_closure_was_overapproximated_);
+  DCHECK(IsMarking());
 
   int old_marking_deque_top =
       heap_->mark_compact_collector()->marking_deque()->top();

diff --git a/src/heap/memory-reducer.cc b/src/heap/memory-reducer.cc
index 44088f2..115e376 100644 (file)
--- a/src/heap/memory-reducer.cc
+++ b/src/heap/memory-reducer.cc
@@ -46,12 +46,35 @@ void MemoryReducer::NotifyTimer(const Event& event) {
   if (state_.action == kRun) {
     DCHECK(heap()->incremental_marking()->IsStopped());
     DCHECK(FLAG_incremental_marking);
-    heap()->StartIdleIncrementalMarking();
     if (FLAG_trace_gc_verbose) {
       PrintIsolate(heap()->isolate(), "Memory reducer: started GC #%d\n",
                    state_.started_gcs);
     }
+    if (heap()->ShouldOptimizeForMemoryUsage()) {
+      // Do full GC if memory usage has higher priority than latency. This is
+      // important for background tabs that do not send idle notifications.
+      heap()->CollectAllGarbage(Heap::kReduceMemoryFootprintMask,
+                                "memory reducer");
+    } else {
+      heap()->StartIdleIncrementalMarking();
+    }
   } else if (state_.action == kWait) {
+    if (!heap()->incremental_marking()->IsStopped() &&
+        heap()->ShouldOptimizeForMemoryUsage()) {
+      // Make progress with pending incremental marking if memory usage has
+      // higher priority than latency. This is important for background tabs
+      // that do not send idle notifications.
+      const int kIncrementalMarkingDelayMs = 500;
+      double deadline = heap()->MonotonicallyIncreasingTimeInMs() +
+                        kIncrementalMarkingDelayMs;
+      heap()->AdvanceIncrementalMarking(
+          0, deadline, i::IncrementalMarking::StepActions(
+                           i::IncrementalMarking::NO_GC_VIA_STACK_GUARD,
+                           i::IncrementalMarking::FORCE_MARKING,
+                           i::IncrementalMarking::FORCE_COMPLETION));
+      heap()->FinalizeIncrementalMarkingIfComplete(
+          "Memory reducer: finalize incremental marking");
+    }
     // Re-schedule the timer.
     ScheduleTimer(state_.next_gc_start_ms - event.time_ms);
     if (FLAG_trace_gc_verbose) {

diff --git a/test/cctest/test-heap.cc b/test/cctest/test-heap.cc
index 2c85596..66e4f2a 100644 (file)
--- a/test/cctest/test-heap.cc
+++ b/test/cctest/test-heap.cc
@@ -6663,5 +6663,40 @@ TEST(SharedFunctionInfoIterator) {
   CHECK_EQ(0, sfi_count);
 }
 
+
+template <typename T>
+static UniqueId MakeUniqueId(const Persistent<T>& p) {
+  return UniqueId(reinterpret_cast<uintptr_t>(*v8::Utils::OpenPersistent(p)));
+}
+
+
+TEST(Regress519319) {
+  CcTest::InitializeVM();
+  v8::Isolate* isolate = CcTest::isolate();
+  v8::HandleScope scope(isolate);
+  Heap* heap = CcTest::heap();
+  LocalContext context;
+
+  v8::Persistent<Value> parent;
+  v8::Persistent<Value> child;
+
+  parent.Reset(isolate, v8::Object::New(isolate));
+  child.Reset(isolate, v8::Object::New(isolate));
+
+  SimulateFullSpace(heap->old_space());
+  heap->CollectGarbage(OLD_SPACE);
+  {
+    UniqueId id = MakeUniqueId(parent);
+    isolate->SetObjectGroupId(parent, id);
+    isolate->SetReferenceFromGroup(id, child);
+  }
+  // The CollectGarbage call above starts sweeper threads.
+  // The crash will happen if the following two functions
+  // are called before sweeping finishes.
+  heap->StartIncrementalMarking();
+  heap->FinalizeIncrementalMarkingIfComplete("test");
+}
+
+
 }  // namespace internal
 }  // namespace v8