TaskScheduler: Make the worker pools of TaskSchedulerImpl configurable

base/task_scheduler/task_scheduler_impl.cc

Index: base/task_scheduler/task_scheduler_impl.cc
diff --git a/base/task_scheduler/task_scheduler_impl.cc b/base/task_scheduler/task_scheduler_impl.cc
index e46a5b42977288fe6f9afb1c4c1b0d78c442473c..fccac14a6c492ef4d60764716273086919a94f13 100644
--- a/base/task_scheduler/task_scheduler_impl.cc
+++ b/base/task_scheduler/task_scheduler_impl.cc
@@ -4,11 +4,15 @@
 
 #include "base/task_scheduler/task_scheduler_impl.h"
 
+#include <algorithm>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
+#include "base/logging.h"
 #include "base/memory/ptr_util.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/sys_info.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"
@@ -18,10 +22,68 @@
 namespace base {
 namespace internal {
 
+namespace {
+
+constexpr char kTaskSchedulerThreadNamePrefix[] = "TaskScheduler";
+
+struct ThreadPoolSettings {
+  // Name of the pool. Used to name threads and to find variation params that

[gab, 2016/06/15 19:56:04]

s/name threads/name the pool's threads/

+  // apply to this pool.

[gab, 2016/06/15 19:56:04]

s/to this pool/to it/ (since I'm adding "the pool" to the beginning of the
sentence above..)

+  const char* name;

[gab, 2016/06/15 19:56:03]

const char[]

(i.e. to get const char* const, but [] is cleaner for string literals)

+
+  // Default values used to compute the maximum number of threads in the pool
+  // (used when not specified by variation params).
+  int default_min_threads;
+  int default_max_threads;
+  int default_threads_percentage_num_processors;

[gab, 2016/06/15 19:56:04]

default_threads_per_processor ?

+};
+
+constexpr ThreadPoolSettings kBackgroundPoolSettings{"Background", 1, 1, 0};
+constexpr ThreadPoolSettings kBackgroundFileIOPoolSettings{"BackgroundFileIO",
+                                                           3, 3, 0};
+constexpr ThreadPoolSettings kForegroundPoolSettings{"Foreground", 4, 16, 100};
+constexpr ThreadPoolSettings kForegroundFileIOPoolSettings{"ForegroundFileIO",
+                                                           8, 32, 200};

[gab, 2016/06/15 19:56:04]

Minimums 4/8 above sound arbitrary :-), why do we care about a minimum? Isn't
the scheduler inherently trying to keep thread count to a minimum unless
contention occurs anyways?

+
+// Returns the integer associated with |key| in |map| or |default_value| if
+// there is no such integer.
+int ReadIntegerFromMap(const std::map<std::string, std::string>& map,
+                       const std::string& key,
+                       int default_value) {
+  auto it = map.find(key);
+  int value = 0;
+  if (it == map.end() || !StringToInt(it->second, &value))
+    return default_value;
+  return value;
+}
+
+int GetMaxThreadsForPool(
+    const ThreadPoolSettings& settings,
+    const std::map<std::string, std::string>& variation_params) {
+  DCHECK_GT(settings.default_min_threads, 0);

[gab, 2016/06/15 19:56:04]

Overkill given other DCHECK below which encompasses the default if it matters?

+  const std::string pool_name(settings.name);
+  const int min_threads = ReadIntegerFromMap(
+      variation_params, pool_name + "MinThreads", settings.default_min_threads);
+  DCHECK_GT(min_threads, 0);
+  const int max_threads = ReadIntegerFromMap(
+      variation_params, pool_name + "MaxThreads", settings.default_max_threads);

[gab, 2016/06/15 19:56:04]

DCHECK_GE(min_threads, max_threads);

+  const int threads_percentage_num_processors = ReadIntegerFromMap(
+      variation_params, pool_name + "ThreadsPercentageNumProcessors",
+      settings.default_threads_percentage_num_processors);

[gab, 2016/06/15 19:56:04]

Split these with new lines between different variables, my eyes are torn from
too much text :-)

+
+  return std::max(
+      min_threads,
+      std::min(max_threads, threads_percentage_num_processors *
+                                SysInfo::NumberOfProcessors() / 100));
+}
+
+}  // namespace
+
 // static
-std::unique_ptr<TaskSchedulerImpl> TaskSchedulerImpl::Create() {
+std::unique_ptr<TaskSchedulerImpl> TaskSchedulerImpl::Create(
+    const std::map<std::string, std::string>& variation_params) {
   std::unique_ptr<TaskSchedulerImpl> scheduler(new TaskSchedulerImpl);
-  scheduler->Initialize();
+  scheduler->Initialize(variation_params);
   return scheduler;
 }
 
@@ -78,39 +140,46 @@ TaskSchedulerImpl::TaskSchedulerImpl()
 {
 }
 
-void TaskSchedulerImpl::Initialize() {
+void TaskSchedulerImpl::Initialize(
+    const std::map<std::string, std::string>& variation_params) {
   using IORestriction = SchedulerThreadPoolImpl::IORestriction;
 
+  const std::string thread_name_prefix(kTaskSchedulerThreadNamePrefix);
+
   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.
-
   // 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,
+      thread_name_prefix + kBackgroundPoolSettings.name,
+      ThreadPriority::BACKGROUND,
+      GetMaxThreadsForPool(kBackgroundPoolSettings, variation_params),
       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,
+      thread_name_prefix + kBackgroundFileIOPoolSettings.name,
+      ThreadPriority::BACKGROUND,
+      GetMaxThreadsForPool(kBackgroundFileIOPoolSettings, variation_params),
       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,
+      thread_name_prefix + kForegroundPoolSettings.name, ThreadPriority::NORMAL,
+      GetMaxThreadsForPool(kForegroundPoolSettings, variation_params),
       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,
+      thread_name_prefix + kForegroundFileIOPoolSettings.name,
+      ThreadPriority::NORMAL,
+      GetMaxThreadsForPool(kForegroundFileIOPoolSettings, variation_params),
       IORestriction::ALLOWED, re_enqueue_sequence_callback, &task_tracker_,
       &delayed_task_manager_);
   CHECK(normal_file_io_thread_pool_);