Revert "[Blink Heap]: Marking uses jobs API."

This reverts commit b9031921.

Reason for revert: Suspected to cause browser_tests failures on mac:
https://bugs.chromium.org/p/chromium/issues/detail?id=1097000

Original change's description:
> [Blink Heap]: Marking uses jobs API.
>
> Manual reland:
> Original CL: https://chromium-review.googlesource.com/c/chromium/src/+/2029609
> Revert: https://chromium-review.googlesource.com/c/chromium/src/+/2109970
> Reset for revert: flaky DCHECK in AssertExpectedConcurrency
> Fix: Avoid calling AssertExpectedConcurrency from ShouldYield.
> https://chromium-review.googlesource.com/c/chromium/src/+/2112235
>
> Original Description:
> Note: This CL enables feature BlinkHeapConcurrentMarking to get
> the desired behavior. However, we should land enabling
> BlinkHeapConcurrentMarking on its own first.
>
> active_markers + GlobalPoolSize() is used to determine
> the desired number of workers.
>
> NotifyConcurrencyIncrease() is called periodically from
> ConcurrentMarkingStep() if GlobalPoolSize() > 0,
> to make sure enough workers are scheduled.
>
> Bug: 1046343
> Change-Id: I4c36044686128b23fc25f45b99334c219a08cffa
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2214908
> Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
> Reviewed-by: Omer Katz <omerkatz@chromium.org>
> Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#779969}

TBR=mlippautz@chromium.org,etiennep@chromium.org,omerkatz@chromium.org

Change-Id: I4384cf531428b09d02f97f41368069dd9d056e9f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1046343, 1097000
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2254470Reviewed-by: Etienne Pierre-Doray <etiennep@chromium.org>
Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#780387}

third_party/blink/renderer/platform/heap/BUILD.gn

@@ -49,6 +49,8 @@ blink_platform_sources("heap") {
     "blink_gc.h",
     "blink_gc_memory_dump_provider.cc",
     "blink_gc_memory_dump_provider.h",
+    "cancelable_task_scheduler.cc",
+    "cancelable_task_scheduler.h",
     "collection_support/heap_hash_table_backing.h",
     "collection_support/heap_linked_stack.h",
     "collection_support/heap_vector_backing.h",
@@ -151,6 +153,7 @@ jumbo_source_set("blink_heap_unittests_sources") {
   sources = [
     "../testing/run_all_tests.cc",
     "test/blink_gc_memory_dump_provider_test.cc",
+    "test/cancelable_task_scheduler_test.cc",
     "test/card_table_test.cc",
     "test/concurrent_marking_test.cc",
     "test/gc_info_test.cc",

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

+// Copyright 2019 The Chromium 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 "third_party/blink/renderer/platform/heap/cancelable_task_scheduler.h"
+
+#include "base/check.h"
+#include "base/macros.h"
+#include "base/task_runner.h"
+#include "third_party/blink/renderer/platform/wtf/allocator/allocator.h"
+#include "third_party/blink/renderer/platform/wtf/vector.h"
+
+namespace blink {
+
+class CancelableTaskScheduler::TaskData {
+  USING_FAST_MALLOC(TaskData);
+  DISALLOW_COPY_AND_ASSIGN(TaskData);
+
+ public:
+  TaskData(Task task, CancelableTaskScheduler* scheduler)
+      : task_(std::move(task)), scheduler_(scheduler), status_(kWaiting) {}
+
+  ~TaskData() {
+    // The task runner is responsible for unregistering the task in case the
+    // task hasn't been cancelled.
+    if (TryCancel()) {
+      scheduler_->UnregisterAndSignal(this);
+    }
+  }
+
+  void Run() {
+    if (TryRun()) {
+      std::move(task_).Run();
+      scheduler_->UnregisterAndSignal(this);
+    }
+  }
+
+  bool TryCancel() {
+    Status expected = kWaiting;
+    return status_.compare_exchange_strong(expected, kCancelled,
+                                           std::memory_order_acq_rel,
+                                           std::memory_order_acquire);
+  }
+
+ private:
+  // Identifies the state a cancelable task is in:
+  // |kWaiting|: The task is scheduled and waiting to be executed. {TryRun} will
+  // succeed.
+  // |kCancelled|: The task has been cancelled. {TryRun} will fail.
+  // |kRunning|: The task is currently running and cannot be canceled anymore.
+  enum Status : uint8_t { kWaiting, kCancelled, kRunning };
+
+  bool TryRun() {
+    Status expected = kWaiting;
+    return status_.compare_exchange_strong(expected, kRunning,
+                                           std::memory_order_acq_rel,
+                                           std::memory_order_acquire);
+  }
+
+  Task task_;
+  CancelableTaskScheduler* const scheduler_;
+  std::atomic<Status> status_;
+};
+
+CancelableTaskScheduler::CancelableTaskScheduler(
+    scoped_refptr<base::TaskRunner> task_runner)
+    : cond_var_(&lock_), task_runner_(std::move(task_runner)) {}
+
+CancelableTaskScheduler::~CancelableTaskScheduler() {
+  base::AutoLock lock(lock_);
+  CHECK(tasks_.IsEmpty());
+}
+
+void CancelableTaskScheduler::ScheduleTask(Task task) {
+  std::unique_ptr<TaskData> task_data = Register(std::move(task));
+  task_runner_->PostTask(FROM_HERE,
+                         base::BindOnce(&TaskData::Run, std::move(task_data)));
+}
+
+size_t CancelableTaskScheduler::CancelAndWait() {
+  size_t result = 0;
+  base::AutoLock lock(lock_);
+  while (!tasks_.IsEmpty()) {
+    result += RemoveCancelledTasks();
+    if (!tasks_.IsEmpty()) {
+      cond_var_.Wait();
+    }
+  }
+  return result;
+}
+
+std::unique_ptr<CancelableTaskScheduler::TaskData>
+CancelableTaskScheduler::Register(Task task) {
+  auto task_data = std::make_unique<TaskData>(std::move(task), this);
+  base::AutoLock lock(lock_);
+  tasks_.insert(task_data.get());
+  return task_data;
+}
+
+void CancelableTaskScheduler::UnregisterAndSignal(TaskData* task_data) {
+  base::AutoLock lock(lock_);
+  CHECK(tasks_.Contains(task_data));
+  tasks_.erase(task_data);
+  cond_var_.Signal();
+}
+
+// This function is needed because WTF::HashSet::erase function invalidates
+// all iterators. Returns number of removed tasks.
+size_t CancelableTaskScheduler::RemoveCancelledTasks() {
+  WTF::Vector<TaskData*> to_be_removed;
+  // Assume worst case.
+  to_be_removed.ReserveCapacity(tasks_.size());
+  for (TaskData* task : tasks_) {
+    if (task->TryCancel()) {
+      to_be_removed.push_back(task);
+    }
+  }
+  tasks_.RemoveAll(to_be_removed);
+  return to_be_removed.size();
+}
+
+size_t CancelableTaskScheduler::NumberOfTasksForTesting() const {
+  base::AutoLock lock(lock_);
+  return tasks_.size();
+}
+
+}  // namespace blink

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

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef THIRD_PARTY_BLINK_RENDERER_PLATFORM_HEAP_CANCELABLE_TASK_SCHEDULER_H_
+#define THIRD_PARTY_BLINK_RENDERER_PLATFORM_HEAP_CANCELABLE_TASK_SCHEDULER_H_
+
+#include <memory>
+
+#include "base/memory/scoped_refptr.h"
+#include "base/synchronization/condition_variable.h"
+#include "base/synchronization/lock.h"
+#include "third_party/blink/renderer/platform/platform_export.h"
+#include "third_party/blink/renderer/platform/wtf/allocator/allocator.h"
+#include "third_party/blink/renderer/platform/wtf/functional.h"
+#include "third_party/blink/renderer/platform/wtf/hash_set.h"
+
+namespace base {
+class TaskRunner;
+}
+
+namespace blink {
+
+// CancelableTaskScheduler allows for scheduling tasks that can be cancelled
+// before they are invoked. User is responsible for synchronizing completion of
+// tasks and destruction of CancelableTaskScheduler.
+class PLATFORM_EXPORT CancelableTaskScheduler final {
+  USING_FAST_MALLOC(CancelableTaskScheduler);
+
+ public:
+  using Task = WTF::CrossThreadOnceFunction<void()>;
+
+  explicit CancelableTaskScheduler(scoped_refptr<base::TaskRunner>);
+  ~CancelableTaskScheduler();
+
+  // Schedules task to run on TaskRunner.
+  void ScheduleTask(Task);
+  // Cancels all not yet started tasks and waits for running ones to complete.
+  // Returns number of cancelled (not executed) tasks.
+  size_t CancelAndWait();
+
+ private:
+  class TaskData;
+  template <class T>
+  friend class CancelableTaskSchedulerTest;
+
+  std::unique_ptr<TaskData> Register(Task);
+  void UnregisterAndSignal(TaskData*);
+
+  size_t RemoveCancelledTasks();
+
+  size_t NumberOfTasksForTesting() const;
+
+  WTF::HashSet<TaskData*> tasks_;
+  mutable base::Lock lock_;
+  base::ConditionVariable cond_var_;
+  scoped_refptr<base::TaskRunner> task_runner_;
+};
+
+}  // namespace blink
+
+#endif  // THIRD_PARTY_BLINK_RENDERER_PLATFORM_HEAP_CANCELABLE_TASK_SCHEDULER_H_

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

@@ -265,11 +265,11 @@ void ThreadHeap::MarkNotFullyConstructedObjects(MarkingVisitor* visitor) {
 
 namespace {
 
-template <typename Worklist, typename Callback, typename YieldPredicate>
-bool DrainWorklist(Worklist* worklist,
-                   Callback callback,
-                   YieldPredicate should_yield,
-                   int task_id) {
+template <typename Worklist, typename Callback>
+bool DrainWorklistWithDeadline(base::TimeTicks deadline,
+                               Worklist* worklist,
+                               Callback callback,
+                               int task_id) {
   const size_t kDeadlineCheckInterval = 1250;
 
   size_t processed_callback_count = 0;
@@ -277,34 +277,15 @@ bool DrainWorklist(Worklist* worklist,
   while (worklist->Pop(task_id, &item)) {
     callback(item);
     if (++processed_callback_count == kDeadlineCheckInterval) {
-      if (should_yield())
+      if (deadline <= base::TimeTicks::Now()) {
         return false;
+      }
       processed_callback_count = 0;
     }
   }
   return true;
 }
 
-template <typename Worklist, typename Callback>
-bool DrainWorklistWithDeadline(base::TimeTicks deadline,
-                               Worklist* worklist,
-                               Callback callback,
-                               int task_id) {
-  return DrainWorklist(
-      worklist, std::move(callback),
-      [deadline]() { return deadline <= base::TimeTicks::Now(); }, task_id);
-}
-
-template <typename Worklist, typename Callback>
-bool DrainWorklistWithYielding(base::JobDelegate* delegate,
-                               Worklist* worklist,
-                               Callback callback,
-                               int task_id) {
-  return DrainWorklist(
-      worklist, std::move(callback),
-      [delegate]() { return delegate->ShouldYield(); }, task_id);
-}
-
 }  // namespace
 
 bool ThreadHeap::InvokeEphemeronCallbacks(MarkingVisitor* visitor,
@@ -433,19 +414,14 @@ bool ThreadHeap::HasWorkForConcurrentMarking() const {
          !write_barrier_worklist_->IsGlobalPoolEmpty();
 }
 
-size_t ThreadHeap::ConcurrentMarkingGlobalWorkSize() const {
-  return marking_worklist_->GlobalPoolSize() +
-         write_barrier_worklist_->GlobalPoolSize();
-}
-
 bool ThreadHeap::AdvanceConcurrentMarking(ConcurrentMarkingVisitor* visitor,
-                                          base::JobDelegate* delegate) {
+                                          base::TimeTicks deadline) {
   bool finished;
   do {
     // Iteratively mark all objects that are reachable from the objects
     // currently pushed onto the marking worklist.
-    finished = DrainWorklistWithYielding(
-        delegate, marking_worklist_.get(),
+    finished = DrainWorklistWithDeadline(
+        deadline, marking_worklist_.get(),
         [visitor](const MarkingItem& item) {
           HeapObjectHeader* header =
               HeapObjectHeader::FromPayload(item.base_object_payload);
@@ -458,8 +434,8 @@ bool ThreadHeap::AdvanceConcurrentMarking(ConcurrentMarkingVisitor* visitor,
     if (!finished)
       break;
 
-    finished = DrainWorklistWithYielding(
-        delegate, write_barrier_worklist_.get(),
+    finished = DrainWorklistWithDeadline(
+        deadline, write_barrier_worklist_.get(),
         [visitor](HeapObjectHeader* header) {
           PageFromObject(header)->SynchronizedLoad();
           DCHECK(!ConcurrentMarkingVisitor::IsInConstruction(header));

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

@@ -279,11 +279,8 @@ class PLATFORM_EXPORT ThreadHeap {
 
   // Returns true if concurrent markers will have work to steal
   bool HasWorkForConcurrentMarking() const;
-  // Returns the amount of work currently available for stealing (there could be
-  // work remaining even if this is 0).
-  size_t ConcurrentMarkingGlobalWorkSize() const;
   // Returns true if marker is done
-  bool AdvanceConcurrentMarking(ConcurrentMarkingVisitor*, base::JobDelegate*);
+  bool AdvanceConcurrentMarking(ConcurrentMarkingVisitor*, base::TimeTicks);
 
   // Conservatively checks whether an address is a pointer in any of the
   // thread heaps.  If so marks the object pointed to as live.

third_party/blink/renderer/platform/heap/test/cancelable_task_scheduler_test.cc

+// Copyright 2019 The Chromium 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 "third_party/blink/renderer/platform/heap/cancelable_task_scheduler.h"
+
+#include <atomic>
+
+#include "base/memory/scoped_refptr.h"
+#include "base/task_runner.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/blink/renderer/platform/heap/heap_test_utilities.h"
+#include "third_party/blink/renderer/platform/scheduler/public/worker_pool.h"
+#include "third_party/blink/renderer/platform/scheduler/test/fake_task_runner.h"
+#include "third_party/blink/renderer/platform/wtf/cross_thread_functional.h"
+
+namespace blink {
+
+class ParallelTaskRunner : public base::TaskRunner {
+ public:
+  bool PostDelayedTask(const base::Location& location,
+                       base::OnceClosure task,
+                       base::TimeDelta) override {
+    worker_pool::PostTask(location, WTF::CrossThreadBindOnce(std::move(task)));
+    return true;
+  }
+
+  void RunUntilIdle() {}
+};
+
+template <class Runner>
+class CancelableTaskSchedulerTest : public TestSupportingGC {
+ public:
+  using Task = CancelableTaskScheduler::Task;
+
+  void ScheduleTask(Task callback) {
+    scheduler_.ScheduleTask(std::move(callback));
+  }
+
+  void RunTaskRunner() { task_runner_->RunUntilIdle(); }
+  size_t CancelAndWait() { return scheduler_.CancelAndWait(); }
+
+  size_t NumberOfRegisteredTasks() const {
+    return scheduler_.NumberOfTasksForTesting();
+  }
+
+ private:
+  scoped_refptr<Runner> task_runner_ = base::MakeRefCounted<Runner>();
+  CancelableTaskScheduler scheduler_{task_runner_};
+};
+
+using RunnerTypes =
+    ::testing::Types<scheduler::FakeTaskRunner, ParallelTaskRunner>;
+TYPED_TEST_SUITE(CancelableTaskSchedulerTest, RunnerTypes);
+
+TYPED_TEST(CancelableTaskSchedulerTest, EmptyCancelTasks) {
+  const size_t cancelled = this->CancelAndWait();
+  EXPECT_EQ(0u, cancelled);
+  EXPECT_EQ(0u, this->NumberOfRegisteredTasks());
+}
+
+TYPED_TEST(CancelableTaskSchedulerTest, RunAndCancelTasks) {
+  static constexpr size_t kNumberOfTasks = 10u;
+
+  const auto callback = [](std::atomic<int>* i) { ++(*i); };
+  std::atomic<int> var{0};
+
+  for (size_t i = 0; i < kNumberOfTasks; ++i) {
+    this->ScheduleTask(
+        WTF::CrossThreadBindOnce(callback, WTF::CrossThreadUnretained(&var)));
+    EXPECT_GE(i + 1, this->NumberOfRegisteredTasks());
+  }
+
+  this->RunTaskRunner();
+  // Tasks will remove themselves after running
+  EXPECT_LE(0u, this->NumberOfRegisteredTasks());
+
+  const size_t cancelled = this->CancelAndWait();
+  EXPECT_EQ(0u, this->NumberOfRegisteredTasks());
+  EXPECT_EQ(kNumberOfTasks, var + cancelled);
+}
+
+TEST(CancelableTaskSchedulerTest, RemoveTasksFromQueue) {
+  auto task_runner = base::MakeRefCounted<scheduler::FakeTaskRunner>();
+  CancelableTaskScheduler scheduler{task_runner};
+  int var = 0;
+  scheduler.ScheduleTask(WTF::CrossThreadBindOnce(
+      [](int* var) { ++(*var); }, WTF::CrossThreadUnretained(&var)));
+  auto tasks = task_runner->TakePendingTasksForTesting();
+  // Clearing the task queue should destroy all cancelable closures, which in
+  // turn will notify CancelableTaskScheduler to remove corresponding tasks.
+  tasks.clear();
+  EXPECT_EQ(0, var);
+}
+
+}  // namespace blink

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

@@ -50,6 +50,7 @@
 #include "third_party/blink/renderer/platform/bindings/v8_per_isolate_data.h"
 #include "third_party/blink/renderer/platform/heap/blink_gc.h"
 #include "third_party/blink/renderer/platform/heap/blink_gc_memory_dump_provider.h"
+#include "third_party/blink/renderer/platform/heap/cancelable_task_scheduler.h"
 #include "third_party/blink/renderer/platform/heap/handle.h"
 #include "third_party/blink/renderer/platform/heap/heap.h"
 #include "third_party/blink/renderer/platform/heap/heap_buildflags.h"
@@ -98,6 +99,19 @@ uint8_t ThreadState::main_thread_state_storage_[sizeof(ThreadState)];
 
 namespace {
 
+// Concurrent marking should stop every once in a while to flush private
+// segments to v8 marking worklist. It should also stop to avoid priority
+// inversion.
+//
+// TODO(omerkatz): What is a good value to set here?
+constexpr base::TimeDelta kConcurrentMarkingStepDuration =
+    base::TimeDelta::FromMilliseconds(2);
+// Number of concurrent marking tasks to use.
+//
+// TODO(omerkatz): kNumberOfMarkingTasks should be set heuristically
+// instead of a constant.
+constexpr uint8_t kNumberOfConcurrentMarkingTasks = 3u;
+
 constexpr size_t kMaxTerminationGCLoops = 20;
 
 // Helper function to convert a byte count to a KB count, capping at
@@ -208,7 +222,9 @@ ThreadState::ThreadState()
       asan_fake_stack_(__asan_get_current_fake_stack()),
 #endif
       incremental_marking_scheduler_(
-          std::make_unique<IncrementalMarkingScheduler>(this)) {
+          std::make_unique<IncrementalMarkingScheduler>(this)),
+      marker_scheduler_(std::make_unique<CancelableTaskScheduler>(
+          base::MakeRefCounted<WorkerPoolTaskRunner>())) {
   DCHECK(CheckThread());
   DCHECK(!**thread_specific_);
   **thread_specific_ = this;
@@ -706,10 +722,9 @@ void ThreadState::AtomicPauseMarkPrologue(
     SetGCState(kNoGCScheduled);
     if (base::FeatureList::IsEnabled(
             blink::features::kBlinkHeapConcurrentMarking)) {
-      // Stop concurrent markers and wait synchronously until they have all
-      // returned.
-      marker_handle_.Cancel();
-      DCHECK_EQ(0U, active_markers_);
+      // Stop concurrent markers
+      marker_scheduler_->CancelAndWait();
+      active_markers_ = 0;
       available_concurrent_marking_task_ids_.clear();
     }
 #if DCHECK_IS_ON()
@@ -1142,11 +1157,27 @@ void ThreadState::IncrementalMarkingStart(BlinkGC::GCReason reason) {
     EnableIncrementalMarkingBarrier();
     if (base::FeatureList::IsEnabled(
             blink::features::kBlinkHeapConcurrentMarking)) {
+      // No active concurrent markers yet, so it is safe to write to
+      // concurrently_marked_bytes_ without a lock.
+      concurrently_marked_bytes_ = 0;
       current_gc_data_.visitor->FlushMarkingWorklists();
+      // Check that the marking worklist has enough private segments for all
+      // concurrent marking tasks.
+      const uint8_t max_concurrent_task_id =
+          WorklistTaskId::ConcurrentThreadBase +
+          kNumberOfConcurrentMarkingTasks;
       static_assert(
           MarkingWorklist::kNumTasks == WriteBarrierWorklist::kNumTasks,
           "Marking worklist and write-barrier worklist should be the "
           "same size");
+      static_assert(max_concurrent_task_id <= MarkingWorklist::kNumTasks,
+                    "Number of concurrent marking tasks should not exceed "
+                    "number of tasks in worlkist");
+      // Initialize concurrent marking task ids.
+      for (uint8_t i = WorklistTaskId::ConcurrentThreadBase;
+           i < max_concurrent_task_id; ++i) {
+        available_concurrent_marking_task_ids_.push_back(i);
+      }
       ScheduleConcurrentMarking();
     }
     SetGCState(kIncrementalMarkingStepScheduled);
@@ -1203,19 +1234,11 @@ void ThreadState::IncrementalMarkingStep(BlinkGC::StackState stack_state,
 bool ThreadState::ConcurrentMarkingStep() {
   current_gc_data_.visitor->FlushMarkingWorklists();
   if (Heap().HasWorkForConcurrentMarking()) {
-    // Notifies the scheduler that max concurrency might have increased.
-    // This will adjust the number of markers if necessary.
-    marker_handle_.NotifyConcurrencyIncrease();
+    ScheduleConcurrentMarking();
     return false;
   }
-  base::AutoLock lock(concurrent_marker_lock_);
-  // !HasWorkForConcurrentMarking() is checked again under
-  // |concurrent_marker_lock_| and after |active_markers_| == 0 because active
-  // markers might flush work and return meanwhile. NotifyConcurrencyIncrease()
-  // is not necessary since returning workers will be re-scheduled if there's
-  // any remaining work.
-  return active_markers_.load(std::memory_order_relaxed) == 0 &&
-         !Heap().HasWorkForConcurrentMarking();
+  base::AutoLock lock(concurrent_marker_bootstrapping_lock_);
+  return active_markers_ == 0;
 }
 
 void ThreadState::IncrementalMarkingFinalize() {
@@ -1674,57 +1697,32 @@ void ThreadState::EnableCompactionForNextGCForTesting() {
 }
 
 void ThreadState::ScheduleConcurrentMarking() {
+  base::AutoLock lock(concurrent_marker_bootstrapping_lock_);
+
   DCHECK(base::FeatureList::IsEnabled(
       blink::features::kBlinkHeapConcurrentMarking));
-  DCHECK_EQ(0U, active_markers_);
-
-  // No active concurrent markers yet, so it is safe to write to
-  // concurrently_marked_bytes_ without a lock.
-  concurrently_marked_bytes_ = 0;
 
-  const uint8_t max_concurrent_task_id = MarkingWorklist::kNumTasks;
-  // Initialize concurrent marking task ids.
-  for (uint8_t i = WorklistTaskId::ConcurrentThreadBase;
-       i < max_concurrent_task_id; ++i) {
-    available_concurrent_marking_task_ids_.push_back(i);
+  for (uint8_t i = active_markers_; i < kNumberOfConcurrentMarkingTasks; ++i) {
+    marker_scheduler_->ScheduleTask(WTF::CrossThreadBindOnce(
+        &ThreadState::PerformConcurrentMark, WTF::CrossThreadUnretained(this)));
   }
 
-  // |USER_VISIBLE| is used to minimize marking on foreground thread.
-  marker_handle_ = base::PostJob(
-      FROM_HERE, {base::ThreadPool(), base::TaskPriority::USER_VISIBLE},
-      ConvertToBaseRepeatingCallback(
-          WTF::CrossThreadBindRepeating(&ThreadState::PerformConcurrentMark,
-                                        WTF::CrossThreadUnretained(this))),
-      ConvertToBaseRepeatingCallback(WTF::CrossThreadBindRepeating(
-          [](ThreadState* state) -> size_t {
-            // We need to account for local segments in addition to
-            // ConcurrentMarkingGlobalWorkSize().
-            return std::min<size_t>(
-                state->Heap().ConcurrentMarkingGlobalWorkSize() +
-                    state->active_markers_.load(std::memory_order_relaxed),
-                MarkingWorklist::kNumTasks -
-                    WorklistTaskId::ConcurrentThreadBase);
-          },
-          WTF::CrossThreadUnretained(this))));
+  active_markers_ = kNumberOfConcurrentMarkingTasks;
 }
 
-void ThreadState::PerformConcurrentMark(base::JobDelegate* job) {
+void ThreadState::PerformConcurrentMark() {
   VLOG(2) << "[state:" << this << "] [threadid:" << CurrentThread() << "] "
           << "ConcurrentMark";
   ThreadHeapStatsCollector::EnabledConcurrentScope stats_scope(
       Heap().stats_collector(),
       ThreadHeapStatsCollector::kConcurrentMarkingStep);
 
-  if (!Heap().HasWorkForConcurrentMarking())
-    return;
-
   uint8_t task_id;
   {
-    base::AutoLock lock(concurrent_marker_lock_);
+    base::AutoLock lock(concurrent_marker_bootstrapping_lock_);
     DCHECK(!available_concurrent_marking_task_ids_.IsEmpty());
     task_id = available_concurrent_marking_task_ids_.back();
     available_concurrent_marking_task_ids_.pop_back();
-    active_markers_.fetch_add(1, std::memory_order_relaxed);
   }
 
   std::unique_ptr<ConcurrentMarkingVisitor> concurrent_visitor =
@@ -1736,15 +1734,26 @@ void ThreadState::PerformConcurrentMark(base::JobDelegate* job) {
                 this, GetMarkingMode(Heap().Compaction()->IsCompacting()),
                 task_id);
 
-  Heap().AdvanceConcurrentMarking(concurrent_visitor.get(), job);
-  concurrent_visitor->FlushWorklists();
+  const bool finished = Heap().AdvanceConcurrentMarking(
+      concurrent_visitor.get(),
+      base::TimeTicks::Now() + kConcurrentMarkingStepDuration);
 
+  concurrent_visitor->FlushWorklists();
   {
-    base::AutoLock lock(concurrent_marker_lock_);
-    active_markers_.fetch_sub(1, std::memory_order_relaxed);
+    base::AutoLock lock(concurrent_marker_bootstrapping_lock_);
+    // When marking is done, flush visitor worklists and decrement number of
+    // active markers so we know how many markers are left
     concurrently_marked_bytes_ += concurrent_visitor->marked_bytes();
     available_concurrent_marking_task_ids_.push_back(task_id);
+    if (finished) {
+      --active_markers_;
+      return;
+    }
   }
+
+  // Reschedule this marker
+  marker_scheduler_->ScheduleTask(WTF::CrossThreadBindOnce(
+      &ThreadState::PerformConcurrentMark, WTF::CrossThreadUnretained(this)));
 }
 
 }  // namespace blink

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

@@ -63,6 +63,7 @@ namespace incremental_marking_test {
 class IncrementalMarkingScope;
 }  // namespace incremental_marking_test
 
+class CancelableTaskScheduler;
 class MarkingVisitor;
 class PersistentNode;
 class PersistentRegion;
@@ -550,7 +551,7 @@ class PLATFORM_EXPORT ThreadState final {
   // terminated and the worklist is empty)
   bool ConcurrentMarkingStep();
   void ScheduleConcurrentMarking();
-  void PerformConcurrentMark(base::JobDelegate* job);
+  void PerformConcurrentMark();
 
   // Schedule helpers.
   void ScheduleIdleLazySweep();
@@ -657,11 +658,11 @@ class PLATFORM_EXPORT ThreadState final {
 
   std::unique_ptr<IncrementalMarkingScheduler> incremental_marking_scheduler_;
 
-  base::Lock concurrent_marker_lock_;
+  std::unique_ptr<CancelableTaskScheduler> marker_scheduler_;
   Vector<uint8_t> available_concurrent_marking_task_ids_;
-  std::atomic<size_t> active_markers_{0};
+  uint8_t active_markers_ = 0;
+  base::Lock concurrent_marker_bootstrapping_lock_;
   size_t concurrently_marked_bytes_ = 0;
-  base::JobHandle marker_handle_;
 
   base::JobHandle sweeper_handle_;
   std::atomic_bool has_unswept_pages_{false};