TaskScheduler: Make the worker pools of TaskSchedulerImpl configurable

base/task_scheduler/task_scheduler.h

Index: base/task_scheduler/task_scheduler.h
diff --git a/base/task_scheduler/task_scheduler.h b/base/task_scheduler/task_scheduler.h
index b8da64b891a952a4cc629ccae962c32c765c9645..e344e6f9f43fbfd2c9398fffec31200384d9746c 100644
--- a/base/task_scheduler/task_scheduler.h
+++ b/base/task_scheduler/task_scheduler.h
@@ -5,7 +5,9 @@
 #ifndef BASE_TASK_SCHEDULER_TASK_SCHEDULER_H_
 #define BASE_TASK_SCHEDULER_TASK_SCHEDULER_H_
 
+#include <map>
 #include <memory>
+#include <string>
 
 #include "base/base_export.h"
 #include "base/callback_forward.h"
@@ -60,8 +62,10 @@ class BASE_EXPORT TaskScheduler {
   // API for this process.
   static void SetInstance(std::unique_ptr<TaskScheduler> task_scheduler);
 
-  // Initializes the default task scheduler for this process.
-  static void InitializeDefaultTaskScheduler();
+  // Initializes the default task scheduler for this process. |variation_params|
+  // is used to configure the scheduler (can be empty).
+  static void InitializeDefaultTaskScheduler(

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

Can TaskSchedulerImpl pick these up without the caller needing to supply them?

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

No because variations::GetVariationParams() is in
components/variations/variations_associated_data.h

I don't want to describe supported keys in the header file because the map
shouldn't be filled manually (it should come directly from
variations::GetVariationParams()).

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

Looking a bit more broadly, it's conceivable that folks who consume the task
scheduler may not have access to variation params.

Maybe we should have our own strongly structured initialization parameters and
leave the finch unwrapping to the higher architectural level?

[fdoray, 2016/06/15 19:54:53]

What do you think of this?

class SchedulerThreadPoolImpl {
 public:
  struct Config {
    std::string name;
    ThreadPriority thread_priority;
    int max_num_threads;
  };

  static std::unique_ptr<SchedulerThreadPoolImpl> Create(
    const Config& config,
    ...
  );
};

class TaskSchedulerImpl {
 public:
  class Delegate {
   public:
    // Returns the number of thread pools to create.
    int GetNumThreadPools();

    // Returns arguments used to create the pool at index
    // |pool_index|.
    Config GetThreadPoolConfig(int pool_index);

    // Returns the index of the pool in which a task with
    // |traits| should run.
    int GetThreadPoolForTraits(const TaskTraits& traits);
  };

  static std::unique_ptr<TaskSchedulerImpl> Create(
    std::unique_ptr<Delegate> delegate,
    ...
  );
};

- The formula to compute the max number of threads isn't in TaskSchedulerImpl
(less complexity).
- The number of pools and the algorithm to choose in which pool a task runs is
customizable.

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

I think it's fine to match our variations to finch's (i.e. as-is done now).
Perhaps we want to document what goes in the map though for users that don't
have Finch to generate the map for them.

[fdoray, 2016/06/16 13:02:05]

gab@: WDYT of my new idea? (2016/06/15 19:54:53
https://codereview.chromium.org/2064073003/diff/1/base/task_scheduler/task_sc...)

[gab, 2016/06/16 15:07:07]

As discussed in person, this SGTM as a customizable delegate will also be a
requirement to support non-browser processes.

I'd define Delegate on TaskScheduler:: and Config on TaskScheduler::Delegate::,
not in the impls I think though.

[fdoray, 2016/06/16 18:10:53]

Our design doc says that we'll have a TestTaskScheduler that "runs tasks in FIFO
order on a single thread". If that's still true, Delegate shouldn't be on
TaskScheduler because not all TaskScheduler impls suport a delegate. WDYT?

+      const std::map<std::string, std::string>& variation_params);
 
   // Retrieve the TaskScheduler set via SetInstance() or
   // InitializeDefaultTaskScheduler(). This should be used very rarely; most