DESIGN DISCUSSION ONLY Task Scheduler Single Thread Task Runner Manager for Dedicated Threads per S…

base/task_scheduler/scheduler_single_thread_task_runner_manager_unittest.cc

Index: base/task_scheduler/scheduler_single_thread_task_runner_manager_unittest.cc
diff --git a/base/task_scheduler/scheduler_single_thread_task_runner_manager_unittest.cc b/base/task_scheduler/scheduler_single_thread_task_runner_manager_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..e871530e91589749ec138f3752bdff044d73533c
--- /dev/null
+++ b/base/task_scheduler/scheduler_single_thread_task_runner_manager_unittest.cc
@@ -0,0 +1,210 @@
+// Copyright 2017 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 "base/task_scheduler/scheduler_single_thread_task_runner_manager.h"
+
+#include "base/bind.h"
+#include "base/memory/ptr_util.h"
+#include "base/synchronization/lock.h"
+#include "base/synchronization/waitable_event.h"
+#include "base/task_scheduler/delayed_task_manager.h"
+#include "base/task_scheduler/post_task.h"
+#include "base/task_scheduler/scheduler_worker_pool_params.h"
+#include "base/task_scheduler/task_tracker.h"
+#include "base/task_scheduler/task_traits.h"
+#include "base/test/scoped_task_scheduler.h"
+#include "base/test/test_timeouts.h"
+#include "base/threading/thread.h"
+#include "testing/gtest/include/gtest/gtest.h"
+namespace base {
+namespace internal {
+
+namespace {
+
+enum WorkerPoolType {
+  BACKGROUND_WORKER_POOL = 0,
+  FOREGROUND_WORKER_POOL,
+};
+
+static size_t GetThreadPoolIndexForTraits(const TaskTraits& traits) {
+  return traits.priority() == TaskPriority::BACKGROUND ? BACKGROUND_WORKER_POOL
+                                                       : FOREGROUND_WORKER_POOL;
+}
+
+class TaskSchedulerSingleThreadTaskRunnerManagerTest : public testing::Test {
+ public:
+  TaskSchedulerSingleThreadTaskRunnerManagerTest()
+      : service_thread_("TaskSchedulerServiceThread") {}
+
+  void SetUp() override {
+    ASSERT_FALSE(delayed_task_manager_);
+    service_thread_.Start();
+
+    using StandbyThreadPolicy = SchedulerWorkerPoolParams::StandbyThreadPolicy;
+
+    std::vector<SchedulerWorkerPoolParams> params_vector;
+
+    ASSERT_EQ(BACKGROUND_WORKER_POOL, params_vector.size());
+    params_vector.emplace_back("Background", ThreadPriority::BACKGROUND,
+                               StandbyThreadPolicy::LAZY, 1U, TimeDelta::Max());
+
+    ASSERT_EQ(FOREGROUND_WORKER_POOL, params_vector.size());
+    params_vector.emplace_back("Foreground", ThreadPriority::NORMAL,
+                               StandbyThreadPolicy::LAZY, 1U, TimeDelta::Max());
+
+    delayed_task_manager_ =
+        MakeUnique<DelayedTaskManager>(service_thread_.task_runner());
+    single_thread_task_runner_manager_ =
+        MakeUnique<SchedulerSingleThreadTaskRunnerManager>(
+            params_vector, Bind(&GetThreadPoolIndexForTraits), &task_tracker_,
+            delayed_task_manager_.get());
+  }
+
+  void TearDown() override {
+    single_thread_task_runner_manager_->JoinForTesting();
+    single_thread_task_runner_manager_.reset();
+    delayed_task_manager_.reset();
+    service_thread_.Stop();
+  }
+
+ protected:
+  std::unique_ptr<SchedulerSingleThreadTaskRunnerManager>
+      single_thread_task_runner_manager_;
+  TaskTracker task_tracker_;
+
+ private:
+  Thread service_thread_;
+  std::unique_ptr<DelayedTaskManager> delayed_task_manager_;
+
+  DISALLOW_COPY_AND_ASSIGN(TaskSchedulerSingleThreadTaskRunnerManagerTest);
+};
+
+void CaptureThreadId(PlatformThreadId* thread_id) {
+  ASSERT_TRUE(thread_id);
+  *thread_id = PlatformThread::CurrentId();
+}
+
+void CaptureThreadPriority(ThreadPriority* thread_priority) {
+  ASSERT_TRUE(thread_priority);
+  *thread_priority = PlatformThread::GetCurrentThreadPriority();
+}
+
+void ShouldNotRunCallback() {
+  ADD_FAILURE() << "Ran a task that shouldn't run.";
+}
+
+}  // namespace
+
+TEST_F(TaskSchedulerSingleThreadTaskRunnerManagerTest, DifferentThreadsUsed) {
+  scoped_refptr<SingleThreadTaskRunner> task_runner_1 =
+      single_thread_task_runner_manager_
+          ->CreateSingleThreadTaskRunnerWithTraits(TaskTraits());
+  scoped_refptr<SingleThreadTaskRunner> task_runner_2 =
+      single_thread_task_runner_manager_
+          ->CreateSingleThreadTaskRunnerWithTraits(TaskTraits());
+
+  PlatformThreadId thread_id_1 = kInvalidThreadId;
+  task_runner_1->PostTask(FROM_HERE, Bind(&CaptureThreadId, &thread_id_1));
+  PlatformThreadId thread_id_2 = kInvalidThreadId;
+  task_runner_2->PostTask(FROM_HERE, Bind(&CaptureThreadId, &thread_id_2));
+  single_thread_task_runner_manager_->WaitForAllWorkersIdleForTesting();

[fdoray, 2017/02/08 17:59:01]

Could replace CaptureThreadId -> CaptureThreadIdAndSignalEvent and get rid of
this WaitForAllWorkersIdleForTesting() call.

[robliao, 2017/02/14 21:30:07]

Actually, I think we can just use the task tracker everywhere.

+
+  ASSERT_NE(kInvalidThreadId, thread_id_1);
+  ASSERT_NE(kInvalidThreadId, thread_id_2);
+  EXPECT_NE(thread_id_1, thread_id_2);
+}
+
+TEST_F(TaskSchedulerSingleThreadTaskRunnerManagerTest, PrioritySetCorrectly) {
+  scoped_refptr<SingleThreadTaskRunner> task_runner_background =
+      single_thread_task_runner_manager_
+          ->CreateSingleThreadTaskRunnerWithTraits(
+              TaskTraits().WithPriority(TaskPriority::BACKGROUND));

[fdoray, 2017/02/08 17:59:01]

.WithShutdownBehavior(base::TaskShutdownBehavior::BLOCK_SHUTDOWN)

[robliao, 2017/02/14 21:30:07]

Used everywhere now.

+  scoped_refptr<SingleThreadTaskRunner> task_runner_user_visible =
+      single_thread_task_runner_manager_
+          ->CreateSingleThreadTaskRunnerWithTraits(
+              TaskTraits().WithPriority(TaskPriority::USER_VISIBLE));
+  scoped_refptr<SingleThreadTaskRunner> task_runner_user_blocking =
+      single_thread_task_runner_manager_
+          ->CreateSingleThreadTaskRunnerWithTraits(
+              TaskTraits().WithPriority(TaskPriority::USER_BLOCKING));
+
+  ThreadPriority thread_priority_background;
+  task_runner_background->PostTask(
+      FROM_HERE, Bind(&CaptureThreadPriority, &thread_priority_background));

[fdoray, 2017/02/08 17:59:01]

Bind([] () {
  if (Lock::HandlesMultipleThreadPriorities()) {
    EXPECT_EQ(ThreadPriority::BACKGROUND,
              PlatformThread::GetCurrentThreadPriority());
  } else {
    EXPECT_EQ(ThreadPriority::NORMAL,
              PlatformThread::GetCurrentThreadPriority());
  }
})

[robliao, 2017/02/14 21:30:07]

Covered below.

+  ThreadPriority thread_priority_user_visible;
+  task_runner_user_visible->PostTask(
+      FROM_HERE, Bind(&CaptureThreadPriority, &thread_priority_user_visible));
+  ThreadPriority thread_priority_user_blocking;
+  task_runner_user_blocking->PostTask(
+      FROM_HERE, Bind(&CaptureThreadPriority, &thread_priority_user_blocking));
+  single_thread_task_runner_manager_->WaitForAllWorkersIdleForTesting();

[fdoray, 2017/02/08 17:59:01]

Replace WaitForAllWorkersIdleForTesting() with task_tracker_.Shutdown(). If
tasks are BLOCK_SHUTDOWN, it is guaranteed that they will run.

You can now get rid of WaitForAllWorkersIdleForTesting :)

[robliao, 2017/02/14 21:30:07]

Yep! Done.

+
+  if (Lock::HandlesMultipleThreadPriorities())
+    EXPECT_EQ(ThreadPriority::BACKGROUND, thread_priority_background);
+  else
+    EXPECT_EQ(ThreadPriority::NORMAL, thread_priority_background);
+  EXPECT_EQ(ThreadPriority::NORMAL, thread_priority_user_visible);
+  EXPECT_EQ(ThreadPriority::NORMAL, thread_priority_user_blocking);
+}
+
+TEST_F(TaskSchedulerSingleThreadTaskRunnerManagerTest, PostTaskAfterShutdown) {
+  auto task_runner = single_thread_task_runner_manager_
+                         ->CreateSingleThreadTaskRunnerWithTraits(TaskTraits());
+  task_tracker_.Shutdown();
+  EXPECT_FALSE(task_runner->PostTask(FROM_HERE, Bind(&ShouldNotRunCallback)));
+}
+
+// Verify that a Task runs shortly after its delay expires.
+TEST_F(TaskSchedulerSingleThreadTaskRunnerManagerTest, PostDelayedTask) {
+  TimeTicks start_time = TimeTicks::Now();
+
+  // Post a task with a short delay.
+  WaitableEvent task_ran(WaitableEvent::ResetPolicy::MANUAL,
+                         WaitableEvent::InitialState::NOT_SIGNALED);
+  auto task_runner = single_thread_task_runner_manager_
+                         ->CreateSingleThreadTaskRunnerWithTraits(TaskTraits());
+  EXPECT_TRUE(task_runner->PostDelayedTask(
+      FROM_HERE, Bind(&WaitableEvent::Signal, Unretained(&task_ran)),
+      TestTimeouts::tiny_timeout()));
+
+  // Wait until the task runs.
+  task_ran.Wait();
+
+  // Expect the task to run after its delay expires, but not more than 250 ms
+  // after that.
+  const TimeDelta actual_delay = TimeTicks::Now() - start_time;
+  EXPECT_GE(actual_delay, TestTimeouts::tiny_timeout());
+  EXPECT_LT(actual_delay,
+            TimeDelta::FromMilliseconds(250) + TestTimeouts::tiny_timeout());
+}
+
+// Verify that the RunsTasksOnCurrentThread() method of a SEQUENCED TaskRunner
+// returns false when called from a task that isn't part of the sequence. Note:
+// Tests that use TestTaskFactory already verify that RunsTasksOnCurrentThread()
+// returns true when appropriate so this method complements it to get full
+// coverage of that method.

[fdoray, 2017/02/08 17:59:01]

This tests interactions between SchedulerSequencedTaskRunner and
SchedulerSingleThreadTaskRunnerManager. I think it would be more in its place in
task_scheduler_impl_unittest.cc.

[robliao, 2017/02/14 21:30:07]

Done. Moved the test there instead. Unfortunately, that means we'll only catch
this issue at integration time, which is probably okay.

+TEST_F(TaskSchedulerSingleThreadTaskRunnerManagerTest,
+       SequencedRunsTasksOnCurrentThread) {
+  test::ScopedTaskScheduler scoped_task_scheduler;
+  auto task_runner = single_thread_task_runner_manager_
+                         ->CreateSingleThreadTaskRunnerWithTraits(TaskTraits());
+  scoped_refptr<SequencedTaskRunner> sequenced_task_runner(
+      CreateSequencedTaskRunnerWithTraits(TaskTraits()));
+
+  WaitableEvent task_ran(WaitableEvent::ResetPolicy::MANUAL,
+                         WaitableEvent::InitialState::NOT_SIGNALED);
+  task_runner->PostTask(
+      FROM_HERE,
+      Bind(
+          [](scoped_refptr<TaskRunner> sequenced_task_runner,
+             WaitableEvent* task_ran) {
+            EXPECT_FALSE(sequenced_task_runner->RunsTasksOnCurrentThread());
+            task_ran->Signal();
+          },
+          sequenced_task_runner, Unretained(&task_ran)));
+  task_ran.Wait();
+}
+
+}  // namespace internal
+}  // namespace base