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..98fe13f7a50b39053f37738ab6be9e7c1b04e8a5 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
+  // apply to this pool.
+  const char* name;
+
+  // Default values used to compute the maximum number of threads in the pool
+  // (used when not specified by variation params).
+  int default_min_threads;

[robliao, 2016/06/14 21:43:44]

uint32_t might be more appropriate.

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

Does this rule apply here?
"You should not use the unsigned integer types such as uint32_t, unless there is
a valid reason such as representing a bit pattern rather than a number, or you
need defined overflow modulo 2^N. In particular, do not use unsigned types to
say a number will never be negative. Instead, use assertions for this."
https://google.github.io/styleguide/cppguide.html#Integer_Types 

[robliao, 2016/06/15 17:32:37]

Hrm... interesting. I guess we'll stick with that then. We're pretty
inconsistent with regards to this around Chromium.

+  int default_max_threads;
+  int default_threads_percentage_num_processors;
+};
+
+constexpr ThreadPoolSettings kBackgroundPoolSettings{"Background", 1, 1, 0};

[robliao, 2016/06/14 21:43:44]

This shouldn't trigger a static init, right?

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

No (constexpr + POD)

+constexpr ThreadPoolSettings kBackgroundFileIOPoolSettings{"BackgroundFileIO",
+                                                           3, 3, 0};
+constexpr ThreadPoolSettings kForegroundPoolSettings{"Foreground", 4, 16, 100};
+constexpr ThreadPoolSettings kForegroundFileIOPoolSettings{"ForegroundFileIO",
+                                                           8, 32, 200};
+
+// Returns the integer associated with |key| in |map| or |default_value| there
+// is no such integer.
+int ReadIntegerFromMap(const std::string& key,
+                       int default_value,
+                       const std::map<std::string, std::string>& map) {

[robliao, 2016/06/14 21:43:44]

Consider having the map argument first. This would read more like a "this"
pointer for the map.

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

Done.

+  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);
+  const std::string pool_name(settings.name);
+  const int min_threads = ReadIntegerFromMap(
+      pool_name + "MinThreads", settings.default_min_threads, variation_params);
+  DCHECK_GT(min_threads, 0);
+  const int max_threads = ReadIntegerFromMap(
+      pool_name + "MaxThreads", settings.default_max_threads, variation_params);
+  const int threads_percentage_num_processors = ReadIntegerFromMap(
+      pool_name + "ThreadsPercentageNumProcessors",
+      settings.default_threads_percentage_num_processors, variation_params);
+
+  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_);