Virtualize The Existence of a Scheduler Worker Thread

base/task_scheduler/scheduler_worker_thread.h

Index: base/task_scheduler/scheduler_worker_thread.h
diff --git a/base/task_scheduler/scheduler_worker_thread.h b/base/task_scheduler/scheduler_worker_thread.h
index b6f0860da0ee83cc5f3d3c30ac4f908384185585..74b10b9f7ac9116207dfa6db262de219b62103fa 100644
--- a/base/task_scheduler/scheduler_worker_thread.h
+++ b/base/task_scheduler/scheduler_worker_thread.h
@@ -17,6 +17,7 @@
 #include "base/time/time.h"
 
 namespace base {
+
 namespace internal {
 
 class TaskTracker;
@@ -30,7 +31,7 @@ class TaskTracker;
 // has completed and exits when it has.
 //
 // This class is thread-safe.
-class BASE_EXPORT SchedulerWorkerThread : public PlatformThread::Delegate {
+class BASE_EXPORT SchedulerWorkerThread {
  public:
   // Delegate interface for SchedulerWorkerThread. The methods are always called
   // from the thread managed by the SchedulerWorkerThread instance.
@@ -53,8 +54,13 @@ class BASE_EXPORT SchedulerWorkerThread : public PlatformThread::Delegate {
     // call to GetWork(). GetWork() may be called before this timeout expires
     // if the thread's WakeUp() method is called.
     virtual TimeDelta GetSleepTimeout() = 0;
+
+    // Called by |worker_thread| to see if it is allowed to detach.

[fdoray, 2016/06/08 17:51:48]

s/CanDetach/CanHibernate/ ? I think those who implement this method should ask
themselves "can this worker enter a state in which it consumes less resources
but takes longer to respond to a WakeUp()?" rather "can the thread handle be
released?" (implementation detail).

Add comments about the side-effects of returning true:
- Future Tasks may run on a different thread than Tasks that already ran.
- The next WakeUp() will be more costly.
- The worker will free resources.

[robliao, 2016/06/08 19:00:08]

Hibernation in the natural sense doesn't actually release resources. It just
shuffles them around or consumes them more slowly.

We're actually releasing resources when we let go of the thread. 

Added the bullet points about detachment.

+    virtual bool CanDetach(SchedulerWorkerThread* worker_thread) = 0;

[fdoray, 2016/06/08 17:51:48]

This is called after GetWork() returns nullptr.

[robliao, 2016/06/08 19:00:09]

Updated loosing the ordering constraint.

   };
 
+  enum class InitialWorkerState { ALIVE, DETACHED };

[fdoray, 2016/06/08 17:51:49]

ALIVE, HIBERNATING

[robliao, 2016/06/08 19:00:09]

See discussion about hibernation vs detachment.

+
   // Creates a SchedulerWorkerThread with priority |thread_priority| that runs
   // Tasks from Sequences returned by |delegate|. |task_tracker| is used to
   // handle shutdown behavior of Tasks. Returns nullptr if creating the
@@ -62,12 +68,13 @@ class BASE_EXPORT SchedulerWorkerThread : public PlatformThread::Delegate {
   static std::unique_ptr<SchedulerWorkerThread> Create(
       ThreadPriority thread_priority,
       std::unique_ptr<Delegate> delegate,
-      TaskTracker* task_tracker);
+      TaskTracker* task_tracker,
+      InitialWorkerState worker_state);

[fdoray, 2016/06/08 17:51:49]

I would remove this argument and always start HIBERNATING.

Advantages:
- WakeUp() is the only method from which Worker is instantiated.
- Simpler and less confusing interface (If WakeUp() needs to be called before
the SchedulerWorkerThread runs tasks, what are the benefits of creating it
ALIVE?)

[robliao, 2016/06/08 19:00:08]

The main benefit here is that it allows the SchedulerThreadPool to ensure that
at least one thread is good to go during create. At the moment, a
SchedulerThreadPool does not have the ability to determine if a real thread is
backing the SchedulerWorkerThread or not.

Updated comment.

 
   // Destroying a SchedulerWorkerThread in production is not allowed; it is
   // always leaked. In tests, it can only be destroyed after JoinForTesting()
   // has returned.
-  ~SchedulerWorkerThread() override;
+  ~SchedulerWorkerThread();
 
   // Wakes up this SchedulerWorkerThread if it wasn't already awake. After this
   // is called, this SchedulerWorkerThread will run Tasks from Sequences
@@ -80,22 +87,33 @@ class BASE_EXPORT SchedulerWorkerThread : public PlatformThread::Delegate {
   // allowed to complete its execution. This can only be called once.
   void JoinForTesting();
 
+  // Whether is worker is alive.

[fdoray, 2016/06/08 17:51:48]

// Returns true if the worker is alive.
+ const

[robliao, 2016/06/08 19:00:09]

I think I was falling into java style habits with these names. Changed to return
true.

+  bool WorkerAliveForTesting();
+
  private:
+  class Worker;
+
   SchedulerWorkerThread(ThreadPriority thread_priority,
                         std::unique_ptr<Delegate> delegate,
                         TaskTracker* task_tracker);
 
-  // PlatformThread::Delegate:
-  void ThreadMain() override;
+  // Returns the worker for ownership by the worker thread if the detach was
+  // successful. If the detach is not possible, returns nullptr.
+  std::unique_ptr<SchedulerWorkerThread::Worker> Detach();
+
+  void CreateWorker();
+
+  void CreateWorkerAssertSynchronized();
 
   bool ShouldExitForTesting() const;
 
-  // Platform thread managed by this SchedulerWorkerThread.
-  PlatformThreadHandle thread_handle_;
+  // Synchronizes access to |worker_|
+  SchedulerLock worker_lock_;
 
-  // Event signaled to wake up this SchedulerWorkerThread.
-  WaitableEvent wake_up_event_;
+  // The underlying thread for this SchedulerWorkerThread.
+  std::unique_ptr<Worker> worker_;
 
+  const ThreadPriority thread_priority_;
   const std::unique_ptr<Delegate> delegate_;
   TaskTracker* const task_tracker_;