Implement Shared SingleThreadTaskRunners in the Task Scheduler

base/task_scheduler/scheduler_single_thread_task_runner_manager.h

 // Copyright 2017 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.
 
 #ifndef BASE_TASK_SCHEDULER_SCHEDULER_SINGLE_THREAD_TASK_RUNNER_MANAGER_H_
 #define BASE_TASK_SCHEDULER_SCHEDULER_SINGLE_THREAD_TASK_RUNNER_MANAGER_H_
 
 #include <memory>
 #include <string>
 #include <vector>
 
 #include "base/atomicops.h"
 #include "base/base_export.h"
 #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"
 
 namespace base {
 
 class TaskTraits;
 class SingleThreadTaskRunner;
 
 namespace internal {
 
 class DelayedTaskManager;
 class SchedulerWorker;
 class TaskTracker;
 
 namespace {
 
 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.
 //
 // This class is thread-safe.
 class BASE_EXPORT SchedulerSingleThreadTaskRunnerManager final {
  public:
   SchedulerSingleThreadTaskRunnerManager(
       TaskTracker* task_tracker,
       DelayedTaskManager* delayed_task_manager);
   ~SchedulerSingleThreadTaskRunnerManager();
 
   // Starts threads for existing SingleThreadTaskRunners and allows threads to
   // 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();
 
  private:
   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(
       const std::string& name,
       int id);
 
   template <typename DelegateType>
   SchedulerWorker* CreateAndRegisterSchedulerWorker(
       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_|.
-  SchedulerLock lock_;
-  std::vector<scoped_refptr<SchedulerWorker>> workers_;
-  int next_worker_id_ = 0;
-
-  // Set to true when Start() is called.
-  bool started_ = false;
-
 #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;
+
   DISALLOW_COPY_AND_ASSIGN(SchedulerSingleThreadTaskRunnerManager);
 };
 
 }  // namespace internal
 }  // namespace base
 
 #endif  // BASE_TASK_SCHEDULER_SCHEDULER_SINGLE_THREAD_TASK_RUNNER_MANAGER_H_