heap: Raise concurrent marking priority if no progress is made

With the new jobs api, concurrent marking tasks get a lower priority
than before. We've seen cases where that means no concurrent marking
tasks are executed, which leads to long GC times.

I've seen a trace that contains a GC from 10.8s to 13.2s (2.4s in
total). During this GC there are no concurrent marking tasks because
wasm compilation tasks are given a higher priority.

However, as long as GC is running, the mutator thread pays the cost of
write barriers. Thus it is in our interest to finish marking as soon as
possible.

This CL raises the priority of concurrent marking tasks for the current
gc cycle if there was no concurrent progress being made for a
significant portion of the expected marking duration.

Bug: 1119552
Change-Id: I1934e72df171b63292513b63be1ddee925806718
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2378305Reviewed-by: Hannes Payer <hpayer@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Omer Katz <omerkatz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#803267}

third_party/blink/renderer/platform/heap/marking_scheduling_oracle.cc

@@ -35,9 +35,12 @@ void MarkingSchedulingOracle::AddConcurrentlyMarkedBytes(size_t marked_bytes) {
   concurrently_marked_bytes_.fetch_add(marked_bytes, std::memory_order_relaxed);
 }
 
+size_t MarkingSchedulingOracle::GetConcurrentlyMarkedBytes() {
+  return concurrently_marked_bytes_.load(std::memory_order_relaxed);
+}
+
 size_t MarkingSchedulingOracle::GetOverallMarkedBytes() {
-  return incrementally_marked_bytes_ +
-         concurrently_marked_bytes_.load(std::memory_order_relaxed);
+  return incrementally_marked_bytes_ + GetConcurrentlyMarkedBytes();
 }
 
 double MarkingSchedulingOracle::GetElapsedTimeInMs(base::TimeTicks start_time) {

third_party/blink/renderer/platform/heap/marking_scheduling_oracle.h

@@ -36,6 +36,7 @@ class PLATFORM_EXPORT MarkingSchedulingOracle {
   void UpdateIncrementalMarkingStats(size_t, base::TimeDelta, base::TimeDelta);
   void AddConcurrentlyMarkedBytes(size_t);
 
+  size_t GetConcurrentlyMarkedBytes();
   size_t GetOverallMarkedBytes();
 
   base::TimeDelta GetNextIncrementalStepDurationForTask(size_t);

third_party/blink/renderer/platform/heap/thread_state.cc

@@ -99,6 +99,8 @@ uint8_t ThreadState::main_thread_state_storage_[sizeof(ThreadState)];
 
 namespace {
 
+constexpr double kMarkingScheduleRatioBeforeConcurrentPriorityIncrease = 0.5;
+
 constexpr size_t kMaxTerminationGCLoops = 20;
 
 // Helper function to convert a byte count to a KB count, capping at
@@ -1119,6 +1121,9 @@ void ThreadState::IncrementalMarkingStart(BlinkGC::GCReason reason) {
           MarkingWorklist::kNumTasks == WriteBarrierWorklist::kNumTasks,
           "Marking worklist and write-barrier worklist should be the "
           "same size");
+      last_concurrently_marked_bytes_ = 0;
+      last_concurrently_marked_bytes_update_ = base::TimeTicks::Now();
+      concurrent_marking_priority_increased_ = false;
       ScheduleConcurrentMarking();
     }
     SetGCState(kIncrementalMarkingStepScheduled);
@@ -1181,6 +1186,32 @@ bool ThreadState::ConcurrentMarkingStep() {
     // Notifies the scheduler that max concurrency might have increased.
     // This will adjust the number of markers if necessary.
     marker_handle_.NotifyConcurrencyIncrease();
+    if (!concurrent_marking_priority_increased_) {
+      // If concurrent tasks aren't executed, it might delay GC finalization.
+      // As long as GC is active so is the write barrier, which incurs a
+      // performance cost. Marking is estimated to take overall
+      // |MarkingSchedulingOracle::kEstimatedMarkingTimeMs| (500ms). If
+      // concurrent marking tasks have not reported any progress (i.e. the
+      // concurrently marked bytes count as not changed) in over
+      // |kMarkingScheduleRatioBeforeConcurrentPriorityIncrease| (50%) of
+      // that expected duration, we increase the concurrent task priority
+      // for the duration of the current GC. This is meant to prevent the
+      // GC from exceeding it's expected end time.
+      size_t current_concurrently_marked_bytes_ =
+          marking_scheduling_->GetConcurrentlyMarkedBytes();
+      if (current_concurrently_marked_bytes_ >
+          last_concurrently_marked_bytes_) {
+        last_concurrently_marked_bytes_ = current_concurrently_marked_bytes_;
+        last_concurrently_marked_bytes_update_ = base::TimeTicks::Now();
+      } else if ((base::TimeTicks::Now() -
+                  last_concurrently_marked_bytes_update_)
+                     .InMilliseconds() >
+                 kMarkingScheduleRatioBeforeConcurrentPriorityIncrease *
+                     MarkingSchedulingOracle::kEstimatedMarkingTimeMs) {
+        marker_handle_.UpdatePriority(base::TaskPriority::USER_BLOCKING);
+        concurrent_marking_priority_increased_ = true;
+      }
+    }
     return false;
   }
   return marker_handle_.IsCompleted();
@@ -1674,7 +1705,6 @@ void ThreadState::ScheduleConcurrentMarking() {
   DCHECK(base::FeatureList::IsEnabled(
       blink::features::kBlinkHeapConcurrentMarking));
 
-  // |USER_BLOCKING| is used to minimize marking on foreground thread.
   marker_handle_ = base::PostJob(
       FROM_HERE, {base::ThreadPool(), base::TaskPriority::USER_VISIBLE},
       ConvertToBaseRepeatingCallback(

third_party/blink/renderer/platform/heap/thread_state.h

@@ -674,6 +674,10 @@ class PLATFORM_EXPORT ThreadState final {
 
   bool skip_incremental_marking_for_testing_ = false;
 
+  size_t last_concurrently_marked_bytes_ = 0;
+  base::TimeTicks last_concurrently_marked_bytes_update_;
+  bool concurrent_marking_priority_increased_ = false;
+
   friend class BlinkGCObserver;
   friend class incremental_marking_test::IncrementalMarkingScope;
   friend class IncrementalMarkingTestDriver;