Make DelayedFences an opt in feature because sampling now isn't free

Now you have to set SetDelayedFenceAllowed in the spec if you want to use
delayed fences on a particular TaskQueue.

Bug: 897751
Change-Id: I5ca9869ae6999fde264e2553e451c1b08fe90204
Reviewed-on: https://chromium-review.googlesource.com/c/1309776
Commit-Queue: Alex Clarke <alexclarke@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604651}

base/task/sequence_manager/sequence_manager_impl_unittest.cc

@@ -246,11 +246,32 @@ class TestCountUsesTimeSource : public TickClock {
   DISALLOW_COPY_AND_ASSIGN(TestCountUsesTimeSource);
 };
 
+TEST_P(SequenceManagerTestWithCustomInitialization, NowNotCalledIfUnneeded) {
+  message_loop_.reset(new MessageLoop());
+  TestCountUsesTimeSource test_count_uses_time_source;
+
+  manager_ = SequenceManagerForTest::Create(
+      nullptr, ThreadTaskRunnerHandle::Get(), &test_count_uses_time_source);
+  manager_->SetWorkBatchSize(6);
+
+  for (size_t i = 0; i < 3; i++)
+    runners_.push_back(CreateTaskQueue());
+
+  runners_[0]->PostTask(FROM_HERE, BindOnce(&NopTask));
+  runners_[0]->PostTask(FROM_HERE, BindOnce(&NopTask));
+  runners_[1]->PostTask(FROM_HERE, BindOnce(&NopTask));
+  runners_[1]->PostTask(FROM_HERE, BindOnce(&NopTask));
+  runners_[2]->PostTask(FROM_HERE, BindOnce(&NopTask));
+  runners_[2]->PostTask(FROM_HERE, BindOnce(&NopTask));
+
+  RunLoop().RunUntilIdle();
+
+  EXPECT_EQ(0, test_count_uses_time_source.now_calls_count());
+}
+
 TEST_P(SequenceManagerTestWithCustomInitialization,
        NowCalledMinimumNumberOfTimesToComputeTaskDurations) {
   message_loop_.reset(new MessageLoop());
-  // This memory is managed by the SequenceManager, but we need to hold a
-  // pointer to this object to read out how many times Now was called.
   TestCountUsesTimeSource test_count_uses_time_source;
 
   manager_ = SequenceManagerForTest::Create(
@@ -268,6 +289,34 @@ TEST_P(SequenceManagerTestWithCustomInitialization,
   runners_[2]->PostTask(FROM_HERE, BindOnce(&NopTask));
   runners_[2]->PostTask(FROM_HERE, BindOnce(&NopTask));
 
+  RunLoop().RunUntilIdle();
+  // Now is called when each task starts running and when its completed.
+  // 6 * 2 = 12 calls.
+  EXPECT_EQ(12, test_count_uses_time_source.now_calls_count());
+}
+
+TEST_P(SequenceManagerTestWithCustomInitialization,
+       NowCalledMinimumNumberOfTimesToComputeTaskDurationsDelayedFenceAllowed) {
+  message_loop_.reset(new MessageLoop());
+  TestCountUsesTimeSource test_count_uses_time_source;
+
+  manager_ = SequenceManagerForTest::Create(
+      nullptr, ThreadTaskRunnerHandle::Get(), &test_count_uses_time_source);
+  manager_->SetWorkBatchSize(6);
+  manager_->AddTaskTimeObserver(&test_task_time_observer_);
+
+  for (size_t i = 0; i < 3; i++) {
+    runners_.push_back(
+        CreateTaskQueue(TaskQueue::Spec("test").SetDelayedFencesAllowed(true)));
+  }
+
+  runners_[0]->PostTask(FROM_HERE, BindOnce(&NopTask));
+  runners_[0]->PostTask(FROM_HERE, BindOnce(&NopTask));
+  runners_[1]->PostTask(FROM_HERE, BindOnce(&NopTask));
+  runners_[1]->PostTask(FROM_HERE, BindOnce(&NopTask));
+  runners_[2]->PostTask(FROM_HERE, BindOnce(&NopTask));
+  runners_[2]->PostTask(FROM_HERE, BindOnce(&NopTask));
+
   RunLoop().RunUntilIdle();
   // Now is called each time a task is queued, when first task is started
   // running, and when a task is completed. 6 * 3 = 18 calls.
@@ -865,55 +914,57 @@ void RecordTimeAndQueueTask(
 }  // namespace
 
 TEST_P(SequenceManagerTest, DelayedFence_DelayedTasks) {
-  CreateTaskQueues(1u);
+  scoped_refptr<TestTaskQueue> queue =
+      CreateTaskQueue(TaskQueue::Spec("test").SetDelayedFencesAllowed(true));
 
   std::vector<TimeTicks> run_times;
-  runners_[0]->PostDelayedTask(
-      FROM_HERE, BindOnce(&RecordTimeTask, &run_times, GetTickClock()),
-      TimeDelta::FromMilliseconds(100));
-  runners_[0]->PostDelayedTask(
-      FROM_HERE, BindOnce(&RecordTimeTask, &run_times, GetTickClock()),
-      TimeDelta::FromMilliseconds(200));
-  runners_[0]->PostDelayedTask(
-      FROM_HERE, BindOnce(&RecordTimeTask, &run_times, GetTickClock()),
-      TimeDelta::FromMilliseconds(300));
-
-  runners_[0]->InsertFenceAt(GetTickClock()->NowTicks() +
-                             TimeDelta::FromMilliseconds(250));
-  EXPECT_FALSE(runners_[0]->HasActiveFence());
+  queue->PostDelayedTask(FROM_HERE,
+                         BindOnce(&RecordTimeTask, &run_times, GetTickClock()),
+                         TimeDelta::FromMilliseconds(100));
+  queue->PostDelayedTask(FROM_HERE,
+                         BindOnce(&RecordTimeTask, &run_times, GetTickClock()),
+                         TimeDelta::FromMilliseconds(200));
+  queue->PostDelayedTask(FROM_HERE,
+                         BindOnce(&RecordTimeTask, &run_times, GetTickClock()),
+                         TimeDelta::FromMilliseconds(300));
+
+  queue->InsertFenceAt(GetTickClock()->NowTicks() +
+                       TimeDelta::FromMilliseconds(250));
+  EXPECT_FALSE(queue->HasActiveFence());
 
   test_task_runner_->FastForwardUntilNoTasksRemain();
 
-  EXPECT_TRUE(runners_[0]->HasActiveFence());
+  EXPECT_TRUE(queue->HasActiveFence());
   EXPECT_THAT(run_times,
               ElementsAre(start_time_ + TimeDelta::FromMilliseconds(100),
                           start_time_ + TimeDelta::FromMilliseconds(200)));
   run_times.clear();
 
-  runners_[0]->RemoveFence();
+  queue->RemoveFence();
 
   test_task_runner_->FastForwardUntilNoTasksRemain();
 
-  EXPECT_FALSE(runners_[0]->HasActiveFence());
+  EXPECT_FALSE(queue->HasActiveFence());
   EXPECT_THAT(run_times,
               ElementsAre(start_time_ + TimeDelta::FromMilliseconds(300)));
 }
 
 TEST_P(SequenceManagerTest, DelayedFence_ImmediateTasks) {
-  CreateTaskQueues(1u);
+  scoped_refptr<TestTaskQueue> queue =
+      CreateTaskQueue(TaskQueue::Spec("test").SetDelayedFencesAllowed(true));
 
   std::vector<TimeTicks> run_times;
-  runners_[0]->InsertFenceAt(GetTickClock()->NowTicks() +
-                             TimeDelta::FromMilliseconds(250));
+  queue->InsertFenceAt(GetTickClock()->NowTicks() +
+                       TimeDelta::FromMilliseconds(250));
 
   for (int i = 0; i < 5; ++i) {
-    runners_[0]->PostTask(
-        FROM_HERE, BindOnce(&RecordTimeTask, &run_times, GetTickClock()));
+    queue->PostTask(FROM_HERE,
+                    BindOnce(&RecordTimeTask, &run_times, GetTickClock()));
     test_task_runner_->FastForwardBy(TimeDelta::FromMilliseconds(100));
     if (i < 2) {
-      EXPECT_FALSE(runners_[0]->HasActiveFence());
+      EXPECT_FALSE(queue->HasActiveFence());
     } else {
-      EXPECT_TRUE(runners_[0]->HasActiveFence());
+      EXPECT_TRUE(queue->HasActiveFence());
     }
   }
 
@@ -923,7 +974,7 @@ TEST_P(SequenceManagerTest, DelayedFence_ImmediateTasks) {
                   start_time_ + TimeDelta::FromMilliseconds(200)));
   run_times.clear();
 
-  runners_[0]->RemoveFence();
+  queue->RemoveFence();
   test_task_runner_->FastForwardUntilNoTasksRemain();
 
   EXPECT_THAT(run_times,
@@ -932,27 +983,28 @@ TEST_P(SequenceManagerTest, DelayedFence_ImmediateTasks) {
 }
 
 TEST_P(SequenceManagerTest, DelayedFence_RemovedFenceDoesNotActivate) {
-  CreateTaskQueues(1u);
+  scoped_refptr<TestTaskQueue> queue =
+      CreateTaskQueue(TaskQueue::Spec("test").SetDelayedFencesAllowed(true));
 
   std::vector<TimeTicks> run_times;
-  runners_[0]->InsertFenceAt(GetTickClock()->NowTicks() +
-                             TimeDelta::FromMilliseconds(250));
+  queue->InsertFenceAt(GetTickClock()->NowTicks() +
+                       TimeDelta::FromMilliseconds(250));
 
   for (int i = 0; i < 3; ++i) {
-    runners_[0]->PostTask(
-        FROM_HERE, BindOnce(&RecordTimeTask, &run_times, GetTickClock()));
-    EXPECT_FALSE(runners_[0]->HasActiveFence());
+    queue->PostTask(FROM_HERE,
+                    BindOnce(&RecordTimeTask, &run_times, GetTickClock()));
+    EXPECT_FALSE(queue->HasActiveFence());
     test_task_runner_->FastForwardBy(TimeDelta::FromMilliseconds(100));
   }
 
-  EXPECT_TRUE(runners_[0]->HasActiveFence());
-  runners_[0]->RemoveFence();
+  EXPECT_TRUE(queue->HasActiveFence());
+  queue->RemoveFence();
 
   for (int i = 0; i < 2; ++i) {
-    runners_[0]->PostTask(
-        FROM_HERE, BindOnce(&RecordTimeTask, &run_times, GetTickClock()));
+    queue->PostTask(FROM_HERE,
+                    BindOnce(&RecordTimeTask, &run_times, GetTickClock()));
     test_task_runner_->FastForwardBy(TimeDelta::FromMilliseconds(100));
-    EXPECT_FALSE(runners_[0]->HasActiveFence());
+    EXPECT_FALSE(queue->HasActiveFence());
   }
 
   EXPECT_THAT(
@@ -967,10 +1019,10 @@ TEST_P(SequenceManagerTest, DelayedFence_TakeIncomingImmediateQueue) {
   // This test checks that everything works correctly when a work queue
   // is swapped with an immediate incoming queue and a delayed fence
   // is activated, forcing a different queue to become active.
-  CreateTaskQueues(2u);
-
-  scoped_refptr<TestTaskQueue> queue1 = runners_[0];
-  scoped_refptr<TestTaskQueue> queue2 = runners_[1];
+  scoped_refptr<TestTaskQueue> queue1 =
+      CreateTaskQueue(TaskQueue::Spec("test").SetDelayedFencesAllowed(true));
+  scoped_refptr<TestTaskQueue> queue2 =
+      CreateTaskQueue(TaskQueue::Spec("test").SetDelayedFencesAllowed(true));
 
   std::vector<std::pair<scoped_refptr<TestTaskQueue>, TimeTicks>> run_times;
 

base/task/sequence_manager/task_queue.h

@@ -100,11 +100,7 @@ class BASE_EXPORT TaskQueue : public RefCountedThreadSafe<TaskQueue> {
 
   // Options for constructing a TaskQueue.
   struct Spec {
-    explicit Spec(const char* name)
-        : name(name),
-          should_monitor_quiescence(false),
-          time_domain(nullptr),
-          should_notify_observers(true) {}
+    explicit Spec(const char* name) : name(name) {}
 
     Spec SetShouldMonitorQuiescence(bool should_monitor) {
       should_monitor_quiescence = should_monitor;
@@ -116,15 +112,23 @@ class BASE_EXPORT TaskQueue : public RefCountedThreadSafe<TaskQueue> {
       return *this;
     }
 
+    // Delayed fences require Now() to be sampled when posting immediate tasks
+    // which is not free.
+    Spec SetDelayedFencesAllowed(bool allow_delayed_fences) {
+      delayed_fence_allowed = allow_delayed_fences;
+      return *this;
+    }
+
     Spec SetTimeDomain(TimeDomain* domain) {
       time_domain = domain;
       return *this;
     }
 
     const char* name;
-    bool should_monitor_quiescence;
-    TimeDomain* time_domain;
-    bool should_notify_observers;
+    bool should_monitor_quiescence = false;
+    TimeDomain* time_domain = nullptr;
+    bool should_notify_observers = true;
+    bool delayed_fence_allowed = false;
   };
 
   // Information about task execution.
@@ -275,6 +279,10 @@ class BASE_EXPORT TaskQueue : public RefCountedThreadSafe<TaskQueue> {
   // Only one fence can be scheduled at a time. Inserting a new fence
   // will automatically remove the previous one, regardless of fence type.
   void InsertFence(InsertFencePosition position);
+
+  // Delayed fences are only allowed for queues created with
+  // SetDelayedFencesAllowed(true) because this feature implies sampling Now()
+  // (which isn't free) for every PostTask, even those with zero delay.
   void InsertFenceAt(TimeTicks time);
 
   // Removes any previously added fence and unblocks execution of any tasks

base/task/sequence_manager/task_queue_impl.cc

@@ -54,7 +54,8 @@ TaskQueueImpl::TaskQueueImpl(SequenceManagerImpl* sequence_manager,
       main_thread_only_(sequence_manager, this, time_domain),
       proxy_(MakeRefCounted<TaskQueueProxy>(this, associated_thread_)),
       should_monitor_quiescence_(spec.should_monitor_quiescence),
-      should_notify_observers_(spec.should_notify_observers) {
+      should_notify_observers_(spec.should_notify_observers),
+      delayed_fence_allowed_(spec.delayed_fence_allowed) {
   DCHECK(time_domain);
   // SequenceManager can't be set later, so we need to prevent task runners
   // from posting any tasks.
@@ -184,9 +185,10 @@ void TaskQueueImpl::PostImmediateTaskImpl(PostedTask task) {
   EnqueueOrder sequence_number =
       any_thread().sequence_manager->GetNextSequenceNumber();
 
-  PushOntoImmediateIncomingQueueLocked(Task(std::move(task),
-                                            any_thread().time_domain->Now(),
-                                            sequence_number, sequence_number));
+  PushOntoImmediateIncomingQueueLocked(Task(
+      std::move(task),
+      delayed_fence_allowed_ ? any_thread().time_domain->Now() : TimeTicks(),
+      sequence_number, sequence_number));
 }
 
 void TaskQueueImpl::PostDelayedTaskImpl(PostedTask task) {
@@ -659,6 +661,10 @@ void TaskQueueImpl::InsertFence(TaskQueue::InsertFencePosition position) {
 }
 
 void TaskQueueImpl::InsertFenceAt(TimeTicks time) {
+  DCHECK(delayed_fence_allowed_)
+      << "Delayed fences are not supported for this queue. Enable them "
+         "explicitly in TaskQueue::Spec when creating the queue";
+
   // Task queue can have only one fence, delayed or not.
   RemoveFence();
   main_thread_only().delayed_fence = time;

base/task/sequence_manager/task_queue_impl.h

@@ -424,6 +424,7 @@ class BASE_EXPORT TaskQueueImpl {
 
   const bool should_monitor_quiescence_;
   const bool should_notify_observers_;
+  const bool delayed_fence_allowed_;
 
   DISALLOW_COPY_AND_ASSIGN(TaskQueueImpl);
 };

third_party/blink/renderer/platform/scheduler/main_thread/main_thread_task_queue.h

@@ -177,31 +177,37 @@ class PLATFORM_EXPORT MainThreadTaskQueue
 
     QueueCreationParams SetCanBeDeferred(bool value) {
       queue_traits = queue_traits.SetCanBeDeferred(value);
+      ApplyQueueTraitsToSpec();
       return *this;
     }
 
     QueueCreationParams SetCanBeThrottled(bool value) {
       queue_traits = queue_traits.SetCanBeThrottled(value);
+      ApplyQueueTraitsToSpec();
       return *this;
     }
 
     QueueCreationParams SetCanBePaused(bool value) {
       queue_traits = queue_traits.SetCanBePaused(value);
+      ApplyQueueTraitsToSpec();
       return *this;
     }
 
     QueueCreationParams SetCanBeFrozen(bool value) {
       queue_traits = queue_traits.SetCanBeFrozen(value);
+      ApplyQueueTraitsToSpec();
       return *this;
     }
 
     QueueCreationParams SetCanRunInBackground(bool value) {
       queue_traits = queue_traits.SetCanRunInBackground(value);
+      ApplyQueueTraitsToSpec();
       return *this;
     }
 
     QueueCreationParams SetQueueTraits(QueueTraits value) {
       queue_traits = value;
+      ApplyQueueTraitsToSpec();
       return *this;
     }
 
@@ -234,6 +240,11 @@ class PLATFORM_EXPORT MainThreadTaskQueue
     FrameSchedulerImpl* frame_scheduler;
     QueueTraits queue_traits;
     bool freeze_when_keep_active;
+
+   private:
+    void ApplyQueueTraitsToSpec() {
+      spec = spec.SetDelayedFencesAllowed(queue_traits.can_be_throttled);
+    }
   };
 
   ~MainThreadTaskQueue() override;