Implement Shared SingleThreadTaskRunners in the Task Scheduler

base/task_scheduler/scheduler_single_thread_task_runner_manager.h

Index: base/task_scheduler/scheduler_single_thread_task_runner_manager.h
diff --git a/base/task_scheduler/scheduler_single_thread_task_runner_manager.h b/base/task_scheduler/scheduler_single_thread_task_runner_manager.h
index c6d9d692b22e6e2b521cfc3890ddd8b3ea135b10..c6cd8ed3f4ab96e19fc8ca26c43d7d0825509c4b 100644
--- a/base/task_scheduler/scheduler_single_thread_task_runner_manager.h
+++ b/base/task_scheduler/scheduler_single_thread_task_runner_manager.h
@@ -14,7 +14,9 @@
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
+#include "base/task_scheduler/environment_config.h"
 #include "base/task_scheduler/scheduler_lock.h"
+#include "base/task_scheduler/single_thread_task_runner_thread_mode.h"
 #include "base/threading/platform_thread.h"
 #include "build/build_config.h"
 
@@ -35,8 +37,12 @@ class SchedulerWorkerDelegate;
 
 }  // namespace
 
-// Manages a pool of threads which are each associated with one
-// SingleThreadTaskRunner.
+// Manages a pool of threads which are each associated with one or more
+// SingleThreadTaskRunners.
+//
+// SingleThreadTaskRunners using SingleThreadTaskRunnerThreadMode::SHARED, are

[fdoray, 2017/05/25 17:02:59]

no comma

[robliao, 2017/05/30 20:01:06]

Removed.

+// backed by shared SchedulerWorkers for each COM+task environment combination.
+// These workers are only reclaimed during JoinForTesting().

[gab, 2017/05/25 18:47:00]

"These workers are lazily instantiated and then only reclaimed during
JoinForTesting()"

[robliao, 2017/05/30 20:01:06]

Done.

 //
 // No threads are created (and hence no tasks can run) before Start() is called.
 //
@@ -52,24 +58,28 @@ class BASE_EXPORT SchedulerSingleThreadTaskRunnerManager final {
   // be started when SingleThreadTaskRunners are created in the future.
   void Start();
 
-  // Creates a SingleThreadTaskRunner which runs tasks with |traits| on a
-  // dedicated thread named "TaskSchedulerSingleThread" + |name| +  index.
-  // |priority_hint| is the preferred thread priority; the actual thread
-  // priority depends on shutdown state and platform capabilities.
+  // Creates a SingleThreadTaskRunner which runs tasks with |traits| on a thread
+  // named "TaskSchedulerSingleThread[Shared]" + |name| +

[gab, 2017/05/25 18:47:00]

I'd say TaskScheduler[Shared]SingleThread so that sharing is more obvious in the
groupings (especially when the names are truncated)

[robliao, 2017/05/30 20:01:07]

This is an artifact of preserving TaskSchedulerSingleThread together. Piping
shared to the middle would complicate the worker creation path by forcing us to
move the prefix outside of the worker creation. Let's see how this goes first.

+  // kEnvironmentParams[GetEnvironmentIndexForTraits(traits)].name_suffix +
+  // index. "Shared" will be in the thread name when |thread_mode| is SHARED. If
+  // |thread_mode| is DEDICATED, a thread will be dedicated to the returned task
+  // runner, otherwise it could be shared with other SingleThreadTaskRunners.

[gab, 2017/05/25 18:47:00]

I think the rest of this comment is redundant starting from \"Shared\" on 64.

An enum is self-documenting, don't think we need to redefine the enum in
different words everywhere it's used (e.g. say we added a category, having to
update all comments would result in useless churn -- and even more likely in
outdated comments).

[robliao, 2017/05/30 20:01:06]

I certainly agree with this sentiment. I would prefer to omit it, but then we
wouldn't have a |thread_mode| comment other than point it to the enum. I'm going
to remove it in the update CL so we have a root for discussion instead of
branching the thread here.

   scoped_refptr<SingleThreadTaskRunner> CreateSingleThreadTaskRunnerWithTraits(
       const std::string& name,
-      ThreadPriority priority_hint,
-      const TaskTraits& traits);
+      const TaskTraits& traits,
+      SingleThreadTaskRunnerThreadMode thread_mode);
 
 #if defined(OS_WIN)
-  // Creates a SingleThreadTaskRunner which runs tasks with |traits| on a
-  // dedicated COM STA thread named "TaskSchedulerSingleThreadCOMSTA" + |name| +
-  // index. |priority_hint| is the preferred thread priority; the actual thread
-  // priority depends on shutdown state and platform capabilities.
+  // Creates a SingleThreadTaskRunner which runs tasks with |traits| on a COM
+  // STA thread named "TaskSchedulerSingleThreadCOMSTA[Shared]" + |name| +
+  // kEnvironmentParams[GetEnvironmentIndexForTraits(traits)].name_suffix +
+  // index. "Shared" will be in the thread name when |thread_mode| is SHARED. If
+  // |thread_mode| is DEDICATED, a thread will be dedicated to the returned task
+  // runner, otherwise it could be shared with other SingleThreadTaskRunners.

[gab, 2017/05/25 18:47:00]

ditto from \"Shared\"

[robliao, 2017/05/30 20:01:06]

Agreed. See above.

   scoped_refptr<SingleThreadTaskRunner> CreateCOMSTATaskRunnerWithTraits(
       const std::string& name,
-      ThreadPriority priority_hint,
-      const TaskTraits& traits);
+      const TaskTraits& traits,
+      SingleThreadTaskRunnerThreadMode thread_mode);
 #endif  // defined(OS_WIN)
 
   void JoinForTesting();
@@ -78,10 +88,10 @@ class BASE_EXPORT SchedulerSingleThreadTaskRunnerManager final {
   class SchedulerSingleThreadTaskRunner;
 
   template <typename DelegateType>
-  scoped_refptr<SingleThreadTaskRunner>
-  CreateSingleThreadTaskRunnerWithDelegate(const std::string& name,
-                                           ThreadPriority priority_hint,
-                                           const TaskTraits& traits);
+  scoped_refptr<SchedulerSingleThreadTaskRunner> CreateTaskRunnerWithTraitsImpl(
+      const std::string& name,
+      const TaskTraits& traits,
+      SingleThreadTaskRunnerThreadMode thread_mode);
 
   template <typename DelegateType>
   std::unique_ptr<SchedulerWorkerDelegate> CreateSchedulerWorkerDelegate(
@@ -93,23 +103,34 @@ class BASE_EXPORT SchedulerSingleThreadTaskRunnerManager final {
       const std::string& name,
       ThreadPriority priority_hint);
 
+  template <typename DelegateType>
+  SchedulerWorker*& GetSharedSchedulerWorkerForTraits(const TaskTraits& traits);

[gab, 2017/05/25 18:47:00]

** is more common and less surprising than *& IMO, especially when non-const (or
maybe it should be const? didn't check usage yet)

[robliao, 2017/05/30 20:01:06]

I wanted to enforce the non-nullable property for
SchedulerWorker*& worker =
      thread_mode == SingleThreadTaskRunnerThreadMode::DEDICATED
          ? dedicated_worker
          : GetSharedSchedulerWorkerForTraits<DelegateType>(traits);

This was the main way to make it work with a template.

+
   void UnregisterSchedulerWorker(SchedulerWorker* worker);
 
+  void ReleaseSharedSchedulerWorkers();
+
   TaskTracker* const task_tracker_;
   DelayedTaskManager* const delayed_task_manager_;
 
-  // Synchronizes access to |workers_|, |next_worker_id_| and |started_|.
+#if DCHECK_IS_ON()
+  subtle::Atomic32 workers_unregistered_during_join_ = 0;
+#endif
+
+  // Synchronizes access to all members below.
   SchedulerLock lock_;
   std::vector<scoped_refptr<SchedulerWorker>> workers_;
   int next_worker_id_ = 0;
 
+  SchedulerWorker* shared_scheduler_workers_[4];

[fdoray, 2017/05/25 17:02:59]

s/4/ENVIRONMENT_COUNT/

[gab, 2017/05/25 18:47:00]

= {};

to zero-initialize inline

[robliao, 2017/05/30 20:01:06]

Done.

+
+#if defined(OS_WIN)
+  SchedulerWorker* shared_com_scheduler_workers_[4];

[fdoray, 2017/05/25 17:02:59]

s/4/ENVIRONMENT_COUNT/

[gab, 2017/05/25 18:47:00]

ditto

[robliao, 2017/05/30 20:01:06]

Done.

+#endif  // defined(OS_WIN)
+
   // Set to true when Start() is called.
   bool started_ = false;
 
-#if DCHECK_IS_ON()
-  subtle::Atomic32 workers_unregistered_during_join_ = 0;
-#endif
-
   DISALLOW_COPY_AND_ASSIGN(SchedulerSingleThreadTaskRunnerManager);
 };