TaskScheduler: Make the worker pools of TaskSchedulerImpl configurable

base/task_scheduler/task_scheduler_impl.cc

 // Copyright 2016 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/task_scheduler_impl.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/memory/ptr_util.h"
 #include "base/task_scheduler/scheduler_service_thread.h"
-#include "base/task_scheduler/scheduler_thread_pool_impl.h"
 #include "base/task_scheduler/sequence_sort_key.h"
 #include "base/task_scheduler/task.h"
 #include "base/time/time.h"
 
 namespace base {
 namespace internal {
 
 // static
-std::unique_ptr<TaskSchedulerImpl> TaskSchedulerImpl::Create() {
-  std::unique_ptr<TaskSchedulerImpl> scheduler(new TaskSchedulerImpl);
+std::unique_ptr<TaskSchedulerImpl> TaskSchedulerImpl::Create(
+    std::unique_ptr<Delegate> delegate) {
+  std::unique_ptr<TaskSchedulerImpl> scheduler(
+      new TaskSchedulerImpl(std::move(delegate)));
   scheduler->Initialize();
   return scheduler;
 }
 
 TaskSchedulerImpl::~TaskSchedulerImpl() {
 #if DCHECK_IS_ON()
   DCHECK(join_for_testing_returned_.IsSignaled());
 #endif
 }
 
@@ skipping 16 matching lines @@
 
 void TaskSchedulerImpl::Shutdown() {
   // TODO(fdoray): Increase the priority of BACKGROUND tasks blocking shutdown.
   task_tracker_.Shutdown();
 }
 
 void TaskSchedulerImpl::JoinForTesting() {
 #if DCHECK_IS_ON()
   DCHECK(!join_for_testing_returned_.IsSignaled());
 #endif
-  background_thread_pool_->JoinForTesting();
-  background_file_io_thread_pool_->JoinForTesting();
-  normal_thread_pool_->JoinForTesting();
-  normal_file_io_thread_pool_->JoinForTesting();
+  for (const auto& thread_pool : thread_pools_)
+    thread_pool->JoinForTesting();
   service_thread_->JoinForTesting();
 #if DCHECK_IS_ON()
   join_for_testing_returned_.Signal();
 #endif
 }
 
-TaskSchedulerImpl::TaskSchedulerImpl()
-    : delayed_task_manager_(
+TaskSchedulerImpl::TaskSchedulerImpl(std::unique_ptr<Delegate> delegate)
+    : delegate_(std::move(delegate)),
+      delayed_task_manager_(
           Bind(&TaskSchedulerImpl::OnDelayedRunTimeUpdated, Unretained(this)))
 #if DCHECK_IS_ON()
       ,
       join_for_testing_returned_(WaitableEvent::ResetPolicy::MANUAL,
                                  WaitableEvent::InitialState::NOT_SIGNALED)
 #endif
 {
+  DCHECK(delegate_);
 }
 
 void TaskSchedulerImpl::Initialize() {
-  using IORestriction = SchedulerThreadPoolImpl::IORestriction;
+  constexpr char kTaskSchedulerThreadNamePrefix[] = "TaskScheduler";
 
   const SchedulerThreadPoolImpl::ReEnqueueSequenceCallback
       re_enqueue_sequence_callback =
           Bind(&TaskSchedulerImpl::ReEnqueueSequenceCallback, Unretained(this));
 
-  // TODO(fdoray): Derive the number of threads per pool from hardware
-  // characteristics rather than using hard-coded constants.
+  const size_t num_thread_pools = delegate_->GetNumThreadPools();

[robliao, 2016/06/16 20:51:15]

I'm not sure it's necessary to make the TaskScheduler this generic. 

Do we need the ability to reconfigure the threadpool hierarchy with finch?

If the answer for reconfiguration is no, then it may be more straightforward to
pursue a Factory pattern instead of a delegate pattern.

I anticipate that we could have two concrete implementations of
TaskSchedulerImpl instead of needing to do data directed configuration  for both
scenarios.

[fdoray, 2016/06/17 15:14:34]

We don't plan to reconfigure the thread pool hierarchy with finch. However,
having this capability makes it easier to use TaskSchedulerImpl outside the
Chrome browser process (e.g. a file i/o thread pool doesn't make sense in a
renderer).

TaskSchedulerImpl needs variation params put cannot call
variations::GetVariationParams directly (because it's in
components/variations/*). That means that code that instantiates
TaskSchedulerImpl is responsible for providing these variation params. Most
strategies to do that involve that TaskSchedulerImpl exposes the fact that it
manages 4 thread pools in its interface. With a delegate, TaskSchedulerImpl
doesn't have to commit itself to managing 4 thread pools in its interface.

A Factory that TaskSchedulerImpl would use to create its ThreadPools (e.g. "give
me a background file i/o thread pool"?) That still involves TaskSchedulerImpl
exposing the fact that it manages 4 thread pools to its users?

"two concrete implementations" = test and prod?

+  for (size_t i = 0; i < num_thread_pools; ++i) {
+    const Delegate::ThreadPoolCreationArgs creation_args =
+        delegate_->GetCreationArgsForThreadPool(i);
 
-  // Passing pointers to objects owned by |this| to
-  // SchedulerThreadPoolImpl::Create() is safe because a TaskSchedulerImpl can't
-  // be deleted before all its thread pools have been joined.
-  background_thread_pool_ = SchedulerThreadPoolImpl::Create(
-      "TaskSchedulerBackground", ThreadPriority::BACKGROUND, 1U,
-      IORestriction::DISALLOWED, re_enqueue_sequence_callback, &task_tracker_,
-      &delayed_task_manager_);
-  CHECK(background_thread_pool_);
-
-  background_file_io_thread_pool_ = SchedulerThreadPoolImpl::Create(
-      "TaskSchedulerBackgroundFileIO", ThreadPriority::BACKGROUND, 1U,
-      IORestriction::ALLOWED, re_enqueue_sequence_callback, &task_tracker_,
-      &delayed_task_manager_);
-  CHECK(background_file_io_thread_pool_);
-
-  normal_thread_pool_ = SchedulerThreadPoolImpl::Create(
-      "TaskSchedulerForeground", ThreadPriority::NORMAL, 4U,
-      IORestriction::DISALLOWED, re_enqueue_sequence_callback, &task_tracker_,
-      &delayed_task_manager_);
-  CHECK(normal_thread_pool_);
-
-  normal_file_io_thread_pool_ = SchedulerThreadPoolImpl::Create(
-      "TaskSchedulerForegroundFileIO", ThreadPriority::NORMAL, 12U,
-      IORestriction::ALLOWED, re_enqueue_sequence_callback, &task_tracker_,
-      &delayed_task_manager_);
-  CHECK(normal_file_io_thread_pool_);
+    // Passing pointers to objects owned by |this| to
+    // SchedulerThreadPoolImpl::Create() is safe because a TaskSchedulerImpl
+    // can't be deleted before all its thread pools have been joined.
+    thread_pools_.push_back(SchedulerThreadPoolImpl::Create(
+        kTaskSchedulerThreadNamePrefix + creation_args.name,
+        creation_args.thread_priority, creation_args.max_threads,
+        creation_args.io_restriction, re_enqueue_sequence_callback,
+        &task_tracker_, &delayed_task_manager_));
+    CHECK(thread_pools_.back());
+  }
 
   service_thread_ = SchedulerServiceThread::Create(&task_tracker_,
                                                    &delayed_task_manager_);
   CHECK(service_thread_);
 }
 
 SchedulerThreadPool* TaskSchedulerImpl::GetThreadPoolForTraits(
     const TaskTraits& traits) {
-  if (traits.with_file_io()) {
-    if (traits.priority() == TaskPriority::BACKGROUND)
-      return background_file_io_thread_pool_.get();
-    return normal_file_io_thread_pool_.get();
-  }
-
-  if (traits.priority() == TaskPriority::BACKGROUND)
-    return background_thread_pool_.get();
-  return normal_thread_pool_.get();
+  const size_t index = delegate_->GetThreadPoolIndexForTraits(traits);
+  DCHECK_LT(index, thread_pools_.size());
+  return thread_pools_[index].get();
 }
 
 void TaskSchedulerImpl::ReEnqueueSequenceCallback(
     scoped_refptr<Sequence> sequence) {
   DCHECK(sequence);
 
   const SequenceSortKey sort_key = sequence->GetSortKey();
   TaskTraits traits(sequence->PeekTask()->traits);
 
   // Update the priority of |traits| so that the next task in |sequence| runs
   // with the highest priority in |sequence| as opposed to the next task's
   // specific priority.
   traits.WithPriority(sort_key.priority());
 
   GetThreadPoolForTraits(traits)->ReEnqueueSequence(std::move(sequence),
                                                     sort_key);
 }
 
 void TaskSchedulerImpl::OnDelayedRunTimeUpdated() {
   service_thread_->WakeUp();
 }
 
 }  // namespace internal
 }  // namespace base