Move Task Scheduler Single Thread Task Runners to Dedicated Threads

base/task_scheduler/scheduler_worker_pool_impl.cc

Index: base/task_scheduler/scheduler_worker_pool_impl.cc
diff --git a/base/task_scheduler/scheduler_worker_pool_impl.cc b/base/task_scheduler/scheduler_worker_pool_impl.cc
index 93800eb33b754e11d261ef11b4e1046e1774d5da..59c8fef722e6c4cb8b7384dc85be901a0cd8622c 100644
--- a/base/task_scheduler/scheduler_worker_pool_impl.cc
+++ b/base/task_scheduler/scheduler_worker_pool_impl.cc
@@ -150,9 +150,12 @@ class SchedulerWorkerPoolImpl::SchedulerSingleThreadTaskRunner :
   // tasks so long as |worker_pool| and |worker| are alive.
   // TODO(robliao): Find a concrete way to manage the memory of |worker_pool|
   // and |worker|.
-  SchedulerSingleThreadTaskRunner(const TaskTraits& traits,
-                                  SchedulerWorkerPool* worker_pool,
-                                  SchedulerWorker* worker);
+  SchedulerSingleThreadTaskRunner(
+      const TaskTraits& traits,
+      SchedulerWorkerPool* worker_pool,
+      SchedulerWorker* worker,
+      UnregisterSingleThreadWorkerPoolCallback
+          unregister_single_thread_worker_pool_callback);
 
   // SingleThreadTaskRunner:
   bool PostDelayedTask(const tracked_objects::Location& from_here,
@@ -189,6 +192,8 @@ class SchedulerWorkerPoolImpl::SchedulerSingleThreadTaskRunner :
   const TaskTraits traits_;
   SchedulerWorkerPool* const worker_pool_;
   SchedulerWorker* const worker_;
+  const UnregisterSingleThreadWorkerPoolCallback

[fdoray, 2017/01/27 16:47:35]

Not needed. SchedulerSingleThreadTaskRunner can access
worker_pool_->unregister_single_thread_worker_pool_callback_ directly.

[robliao, 2017/01/27 21:25:41]

Indeed! Done.

+      unregister_single_thread_worker_pool_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(SchedulerSingleThreadTaskRunner);
 };
@@ -291,6 +296,24 @@ std::unique_ptr<SchedulerWorkerPoolImpl> SchedulerWorkerPoolImpl::Create(
   return nullptr;
 }
 
+// static
+std::unique_ptr<SchedulerWorkerPoolImpl>
+SchedulerWorkerPoolImpl::CreateSingleThreadWorkerPool(

[fdoray, 2017/01/27 16:47:35]

Forward Create() and CreateSingleThreadWorkerPool() to CreateInternal()?

[robliao, 2017/01/27 21:25:41]

I wasn't convinced that it would make things clearer since CreateInternal would
have to take the union of all arguments, but I've done it here as an experiment.
It does make one path for construction, which is nice, but at the same time,
construction is already simple.

+    const SchedulerWorkerPoolParams& params,
+    UnregisterSingleThreadWorkerPoolCallback
+        unregister_single_thread_worker_pool_callback,
+    TaskTracker* task_tracker,
+    DelayedTaskManager* delayed_task_manager) {
+  DCHECK_EQ(params.max_threads(), 1U);
+  auto worker_pool = WrapUnique(new SchedulerWorkerPoolImpl(
+      params, task_tracker, delayed_task_manager,
+      unregister_single_thread_worker_pool_callback));
+  if (worker_pool->Initialize(params, ReEnqueueSequenceCallback())) {

[fdoray, 2017/01/27 16:47:35]

no braces

[robliao, 2017/01/27 21:25:41]

Done.

+    return worker_pool;
+  }
+  return nullptr;
+}
+
 scoped_refptr<TaskRunner> SchedulerWorkerPoolImpl::CreateTaskRunnerWithTraits(
     const TaskTraits& traits) {
   return make_scoped_refptr(new SchedulerParallelTaskRunner(traits, this));
@@ -305,6 +328,7 @@ SchedulerWorkerPoolImpl::CreateSequencedTaskRunnerWithTraits(
 scoped_refptr<SingleThreadTaskRunner>
 SchedulerWorkerPoolImpl::CreateSingleThreadTaskRunnerWithTraits(
     const TaskTraits& traits) {
+  DCHECK(!unregister_single_thread_worker_pool_callback_.is_null());

[fdoray, 2017/01/27 16:47:35]

Tip: You can write
DCHECK(unregister_single_thread_worker_pool_callback_);
https://cs.chromium.org/chromium/src/base/callback_internal.h?l=87

[robliao, 2017/01/27 21:25:41]

Nice! Done.

   // TODO(fdoray): Find a way to take load into account when assigning a
   // SchedulerWorker to a SingleThreadTaskRunner.
   size_t worker_index;
@@ -314,7 +338,8 @@ SchedulerWorkerPoolImpl::CreateSingleThreadTaskRunnerWithTraits(
     next_worker_index_ = (next_worker_index_ + 1) % workers_.size();
   }
   return make_scoped_refptr(new SchedulerSingleThreadTaskRunner(
-      traits, this, workers_[worker_index].get()));
+      traits, this, workers_[worker_index].get(),
+      unregister_single_thread_worker_pool_callback_));
 }
 
 void SchedulerWorkerPoolImpl::ReEnqueueSequence(
@@ -433,14 +458,20 @@ size_t SchedulerWorkerPoolImpl::NumberOfAliveWorkersForTesting() {
 }
 
 SchedulerWorkerPoolImpl::SchedulerSingleThreadTaskRunner::
-    SchedulerSingleThreadTaskRunner(const TaskTraits& traits,
-                                    SchedulerWorkerPool* worker_pool,
-                                    SchedulerWorker* worker)
+    SchedulerSingleThreadTaskRunner(
+        const TaskTraits& traits,
+        SchedulerWorkerPool* worker_pool,
+        SchedulerWorker* worker,
+        UnregisterSingleThreadWorkerPoolCallback
+            unregister_single_thread_worker_pool_callback)
     : traits_(traits),
       worker_pool_(worker_pool),
-      worker_(worker) {
+      worker_(worker),
+      unregister_single_thread_worker_pool_callback_(
+          unregister_single_thread_worker_pool_callback) {
   DCHECK(worker_pool_);
   DCHECK(worker_);
+  DCHECK(!unregister_single_thread_worker_pool_callback.is_null());
   static_cast<SchedulerWorkerDelegateImpl*>(worker_->delegate())->
       RegisterSingleThreadTaskRunner();
 }
@@ -449,6 +480,8 @@ SchedulerWorkerPoolImpl::SchedulerSingleThreadTaskRunner::
     ~SchedulerSingleThreadTaskRunner() {
   static_cast<SchedulerWorkerDelegateImpl*>(worker_->delegate())->
       UnregisterSingleThreadTaskRunner();
+  unregister_single_thread_worker_pool_callback_.Run(
+      static_cast<SchedulerWorkerPoolImpl*>(worker_pool_));
 }
 
 SchedulerWorkerPoolImpl::SchedulerWorkerDelegateImpl::
@@ -606,7 +639,8 @@ bool SchedulerWorkerPoolImpl::SchedulerWorkerDelegateImpl::CanDetach(
   const bool can_detach =
       !idle_start_time_.is_null() &&
       (TimeTicks::Now() - idle_start_time_) > outer_->suggested_reclaim_time_ &&
-      worker != outer_->PeekAtIdleWorkersStack() &&
+      (worker != outer_->PeekAtIdleWorkersStack() ||
+       outer_->workers_.size() == 1) &&  // Single Thread Worker Pools

[fdoray, 2017/01/27 16:47:35]

Should be

...
// In a pool which runs sequenced and parallel tasks, always keep
// on idle thread alive. In a pool which runs single-threaded tasks,
// keep the thread alive only when there are active TaskRunners.
(worker != outer_->PeekAtIdleWorkersStack() ||
 (unregister_single_thread_worker_pool_callback_ &&
  !subtle::NoBarrier_Load(&num_single_threaded_runners_))) &&
outer_->CanWorkerDetachForTesting();

Note: (outer_->workers_.size() == 1) is true in pools with a single thread that
runs parallel/sequenced tasks
https://cs.chromium.org/chromium/src/content/renderer/render_process_impl.cc?...
I think we want to keep the thread active in these pools.

[robliao, 2017/01/27 21:25:41]

Good point. Done.

       !subtle::NoBarrier_Load(&num_single_threaded_runners_) &&
       outer_->CanWorkerDetachForTesting();
   return can_detach;
@@ -624,6 +658,17 @@ SchedulerWorkerPoolImpl::SchedulerWorkerPoolImpl(
     const SchedulerWorkerPoolParams& params,
     TaskTracker* task_tracker,
     DelayedTaskManager* delayed_task_manager)
+    : SchedulerWorkerPoolImpl(params,
+                              task_tracker,
+                              delayed_task_manager,
+                              UnregisterSingleThreadWorkerPoolCallback()) {}
+
+SchedulerWorkerPoolImpl::SchedulerWorkerPoolImpl(
+    const SchedulerWorkerPoolParams& params,
+    TaskTracker* task_tracker,
+    DelayedTaskManager* delayed_task_manager,
+    UnregisterSingleThreadWorkerPoolCallback
+        unregister_single_thread_worker_pool_callback)
     : name_(params.name()),
       suggested_reclaim_time_(params.suggested_reclaim_time()),
       idle_workers_stack_lock_(shared_priority_queue_.container_lock()),
@@ -662,7 +707,9 @@ SchedulerWorkerPoolImpl::SchedulerWorkerPoolImpl(
           50,
           HistogramBase::kUmaTargetedHistogramFlag)),
       task_tracker_(task_tracker),
-      delayed_task_manager_(delayed_task_manager) {
+      delayed_task_manager_(delayed_task_manager),
+      unregister_single_thread_worker_pool_callback_(
+          unregister_single_thread_worker_pool_callback) {
   DCHECK(task_tracker_);
   DCHECK(delayed_task_manager_);
 }