[heap] Reland "Tune incremental marking step size."

This reverts commit adf0fc8c.

Original change's description:
> [heap] Tune incremental marking step size.
>
> The main thread now can reduce marking step size if concurrent marking
> tasks are making progress and the bailout worklist is empty.
>
> Bug: chromium:694255

Change-Id: Ib2f04be258e14887059d88da301ddf17f6b453b4
Reviewed-on: https://chromium-review.googlesource.com/794135Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49685}

src/heap/concurrent-marking.cc

@@ -413,6 +413,7 @@ ConcurrentMarking::ConcurrentMarking(Heap* heap, MarkingWorklist* shared,
       bailout_(bailout),
       on_hold_(on_hold),
       weak_objects_(weak_objects),
+      total_marked_bytes_(0),
       pending_task_count_(0),
       task_count_(0) {
 // The runtime flag should be set only if the compile time flag was set.
@@ -421,6 +422,7 @@ ConcurrentMarking::ConcurrentMarking(Heap* heap, MarkingWorklist* shared,
 #endif
   for (int i = 0; i <= kMaxTasks; i++) {
     is_pending_[i] = false;
+    task_state_[i].marked_bytes = 0;
   }
 }
 
@@ -579,6 +581,7 @@ void ConcurrentMarking::FlushLiveBytes(
       }
     }
     live_bytes.clear();
+    task_state_[i].marked_bytes = 0;
   }
   total_marked_bytes_.SetValue(0);
 }

src/heap/incremental-marking.cc

@@ -57,6 +57,7 @@ IncrementalMarking::IncrementalMarking(
       marking_worklist_(marking_worklist),
       initial_old_generation_size_(0),
       bytes_marked_ahead_of_schedule_(0),
+      bytes_marked_concurrently_(0),
       unscanned_bytes_of_large_object_(0),
       is_compacting_(false),
       should_hurry_(false),
@@ -348,6 +349,7 @@ void IncrementalMarking::Start(GarbageCollectionReason gc_reason) {
   old_generation_allocation_counter_ = heap_->OldGenerationAllocationCounter();
   bytes_allocated_ = 0;
   bytes_marked_ahead_of_schedule_ = 0;
+  bytes_marked_concurrently_ = 0;
   should_hurry_ = false;
   was_activated_ = true;
 
@@ -688,11 +690,17 @@ void IncrementalMarking::RevisitObject(HeapObject* obj) {
   visitor.Visit(map, obj);
 }
 
+template <WorklistToProcess worklist_to_process>
 intptr_t IncrementalMarking::ProcessMarkingWorklist(
     intptr_t bytes_to_process, ForceCompletionAction completion) {
   intptr_t bytes_processed = 0;
   while (bytes_processed < bytes_to_process || completion == FORCE_COMPLETION) {
-    HeapObject* obj = marking_worklist()->Pop();
+    HeapObject* obj;
+    if (worklist_to_process == WorklistToProcess::kBailout) {
+      obj = marking_worklist()->PopBailout();
+    } else {
+      obj = marking_worklist()->Pop();
+    }
     if (obj == nullptr) break;
     // Left trimming may result in white, grey, or black filler objects on the
     // marking deque. Ignore these objects.
@@ -920,42 +928,50 @@ void IncrementalMarking::AdvanceIncrementalMarkingOnAllocation() {
       StepSizeToKeepUpWithAllocations() + StepSizeToMakeProgress();
 
   if (bytes_to_process >= IncrementalMarking::kMinStepSizeInBytes) {
+    HistogramTimerScope incremental_marking_scope(
+        heap_->isolate()->counters()->gc_incremental_marking());
+    TRACE_EVENT0("v8", "V8.GCIncrementalMarking");
+    TRACE_GC(heap_->tracer(), GCTracer::Scope::MC_INCREMENTAL);
     // The first step after Scavenge will see many allocated bytes.
     // Cap the step size to distribute the marking work more uniformly.
     size_t max_step_size = GCIdleTimeHandler::EstimateMarkingStepSize(
         kMaxStepSizeInMs,
         heap()->tracer()->IncrementalMarkingSpeedInBytesPerMillisecond());
     bytes_to_process = Min(bytes_to_process, max_step_size);
-
-    if (FLAG_concurrent_marking && marking_worklist()->IsBailoutEmpty()) {
-      // The number of background tasks + the main thread.
-      size_t tasks = heap()->concurrent_marking()->TaskCount() + 1;
-      bytes_to_process = Max(IncrementalMarking::kMinStepSizeInBytes,
-                             bytes_to_process / tasks);
-    }
-
     size_t bytes_processed = 0;
+    if (FLAG_concurrent_marking) {
+      bytes_processed = Step(bytes_to_process, GC_VIA_STACK_GUARD,
+                             StepOrigin::kV8, WorklistToProcess::kBailout);
+      bytes_to_process = (bytes_processed >= bytes_to_process)
+                             ? 0
+                             : bytes_to_process - bytes_processed;
+      size_t current_bytes_marked_concurrently =
+          heap()->concurrent_marking()->TotalMarkedBytes();
+      // The concurrent_marking()->TotalMarkedBytes() is not monothonic for a
+      // short period of time when a concurrent marking task is finishing.
+      if (current_bytes_marked_concurrently > bytes_marked_concurrently_) {
+        bytes_marked_ahead_of_schedule_ +=
+            current_bytes_marked_concurrently - bytes_marked_concurrently_;
+        bytes_marked_concurrently_ = current_bytes_marked_concurrently;
+      }
+    }
     if (bytes_marked_ahead_of_schedule_ >= bytes_to_process) {
-      // Steps performed in tasks have put us ahead of schedule.
-      // We skip processing of marking dequeue here and thus
-      // shift marking time from inside V8 to standalone tasks.
+      // Steps performed in tasks and concurrently have put us ahead of
+      // schedule. We skip processing of marking dequeue here and thus shift
+      // marking time from inside V8 to standalone tasks.
       bytes_marked_ahead_of_schedule_ -= bytes_to_process;
-      bytes_processed = bytes_to_process;
-    } else {
-      HistogramTimerScope incremental_marking_scope(
-          heap_->isolate()->counters()->gc_incremental_marking());
-      TRACE_EVENT0("v8", "V8.GCIncrementalMarking");
-      TRACE_GC(heap_->tracer(), GCTracer::Scope::MC_INCREMENTAL);
-      bytes_processed =
-          Step(bytes_to_process, GC_VIA_STACK_GUARD, StepOrigin::kV8);
+      bytes_processed += bytes_to_process;
+      bytes_to_process = IncrementalMarking::kMinStepSizeInBytes;
     }
+    bytes_processed += Step(bytes_to_process, GC_VIA_STACK_GUARD,
+                            StepOrigin::kV8, WorklistToProcess::kAll);
     bytes_allocated_ -= Min(bytes_allocated_, bytes_processed);
   }
 }
 
 size_t IncrementalMarking::Step(size_t bytes_to_process,
-                                CompletionAction action,
-                                StepOrigin step_origin) {
+                                CompletionAction action, StepOrigin step_origin,
+                                WorklistToProcess worklist_to_process) {
   double start = heap_->MonotonicallyIncreasingTimeInMs();
 
   if (state_ == SWEEPING) {
@@ -977,7 +993,15 @@ size_t IncrementalMarking::Step(size_t bytes_to_process,
         FLAG_trace_gc_verbose) {
       marking_worklist()->Print();
     }
-    bytes_processed = ProcessMarkingWorklist(bytes_to_process);
+
+    if (worklist_to_process == WorklistToProcess::kBailout) {
+      bytes_processed =
+          ProcessMarkingWorklist<WorklistToProcess::kBailout>(bytes_to_process);
+    } else {
+      bytes_processed =
+          ProcessMarkingWorklist<WorklistToProcess::kAll>(bytes_to_process);
+    }
+
     if (step_origin == StepOrigin::kTask) {
       bytes_marked_ahead_of_schedule_ += bytes_processed;
     }

src/heap/incremental-marking.h

@@ -20,6 +20,7 @@ class Object;
 class PagedSpace;
 
 enum class StepOrigin { kV8, kTask };
+enum class WorklistToProcess { kAll, kBailout };
 
 class V8_EXPORT_PRIVATE IncrementalMarking {
  public:
@@ -187,7 +188,8 @@ class V8_EXPORT_PRIVATE IncrementalMarking {
   void FinalizeSweeping();
 
   size_t Step(size_t bytes_to_process, CompletionAction action,
-              StepOrigin step_origin);
+              StepOrigin step_origin,
+              WorklistToProcess worklist_to_process = WorklistToProcess::kAll);
 
   inline void RestartIfNotMarking();
 
@@ -295,6 +297,7 @@ class V8_EXPORT_PRIVATE IncrementalMarking {
   void DeactivateIncrementalWriteBarrierForSpace(NewSpace* space);
   void DeactivateIncrementalWriteBarrier();
 
+  template <WorklistToProcess worklist_to_process = WorklistToProcess::kAll>
   V8_INLINE intptr_t ProcessMarkingWorklist(
       intptr_t bytes_to_process,
       ForceCompletionAction completion = DO_NOT_FORCE_COMPLETION);
@@ -326,6 +329,10 @@ class V8_EXPORT_PRIVATE IncrementalMarking {
   size_t old_generation_allocation_counter_;
   size_t bytes_allocated_;
   size_t bytes_marked_ahead_of_schedule_;
+  // A sample of concurrent_marking()->TotalMarkedBytes() at the last
+  // incremental marking step. It is used for updating
+  // bytes_marked_ahead_of_schedule_ with contribution of concurrent marking.
+  size_t bytes_marked_concurrently_;
   size_t unscanned_bytes_of_large_object_;
 
   // Must use SetState() above to update state_

src/heap/mark-compact.h

@@ -541,6 +541,14 @@ class MarkCompactCollector final : public MarkCompactCollectorBase {
       return nullptr;
     }
 
+    HeapObject* PopBailout() {
+      HeapObject* result;
+#ifdef V8_CONCURRENT_MARKING
+      if (bailout_.Pop(kMainThread, &result)) return result;
+#endif
+      return nullptr;
+    }
+
     void Clear() {
       bailout_.Clear();
       shared_.Clear();