Virtualize The Existence of a Scheduler Worker Thread

base/task_scheduler/scheduler_worker_thread.h

 // Copyright 2016 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_WORKER_THREAD_H_
 #define BASE_TASK_SCHEDULER_SCHEDULER_WORKER_THREAD_H_
 
 #include <memory>
 
 #include "base/base_export.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/task_scheduler/scheduler_lock.h"
 #include "base/task_scheduler/sequence.h"
 #include "base/threading/platform_thread.h"
 #include "base/time/time.h"
 
 namespace base {
+

[gab, 2016/06/10 16:15:21]

rm empty line

[robliao, 2016/06/10 18:03:41]

Done.

 namespace internal {
 
 class TaskTracker;
 
-// A thread that runs Tasks from Sequences returned by a delegate.
+// A worker that manages at most one thread to run Tasks from Sequences returned
+// by a delegate.
 //
 // A SchedulerWorkerThread starts out sleeping. It is woken up by a call to
 // WakeUp(). After a wake-up, a SchedulerWorkerThread runs Tasks from Sequences
 // returned by the GetWork() method of its delegate as long as it doesn't return
 // nullptr. It also periodically checks with its TaskTracker whether shutdown
 // has completed and exits when it has.
 //
+// The worker is free to release and reallocate thread suggestions with guidance

[gab, 2016/06/10 16:15:20]

"thread suggestions"?

[robliao, 2016/06/10 18:03:41]

Fixed!

+// from the delegate
+//
 // 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.
   class Delegate {
    public:
     virtual ~Delegate() = default;
 
-    // Called by |worker_thread| when it enters its main function.
+    // Called when |worker_thread|'s worker enters the main function.
+    // If a thread is recreated after detachment, this call will occur again.
     virtual void OnMainEntry(SchedulerWorkerThread* worker_thread) = 0;
 
     // Called by |worker_thread| to get a Sequence from which to run a Task.
     virtual scoped_refptr<Sequence> GetWork(
         SchedulerWorkerThread* worker_thread) = 0;
 
     // Called when |sequence| isn't empty after the SchedulerWorkerThread pops a
     // Task from it. |sequence| is the last Sequence returned by GetWork().
     virtual void ReEnqueueSequence(scoped_refptr<Sequence> sequence) = 0;
 
     // Called by |worker_thread| to determine how long to sleep before the next
     // 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 if the last
+    // call to GetWork() returned nullptr.
+    // A SchedulerWorkerThread is free to ignore the request to detach.

[gab, 2016/06/10 16:15:21]

Which "request to detach"? Is a return true here a "request to detach"?

[robliao, 2016/06/10 18:03:41]

Clarified. Today, it is.

+    //
+    // It is the responsibility of the delegate to determine if detachment is
+    // safe. If |worker_thread| is responsible for thread-affine work,
+    // detachment is generally not safe.
+    //
+    // - Future tasks may run on a different PlatformThread after detachment.

[gab, 2016/06/10 16:15:21]

I don't think this is worth mentioning (i.e. it's implicit that shared work will
be picked up by other threads regardless of what this thread does)

[robliao, 2016/06/10 18:03:41]

sgtm, since threads are indeed virtualized now.

+    // - The next WakeUp() could be more costly due to new thread creation.
+    // - The worker will free resources if it can.
+    virtual bool CanDetach(SchedulerWorkerThread* worker_thread) = 0;
   };
 
+  enum class InitialWorkerState { ALIVE, DETACHED };

[gab, 2016/06/10 16:15:21]

"InitialState" is good enough IMO, "Worker" is redundant per this being nested
in "SchedulerWorkerThread"

[robliao, 2016/06/10 18:03:42]

sgtm. Done.

+
   // 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
-  // underlying platform thread fails.
+  // handle shutdown behavior of Tasks. If |worker_state| is DETACHED, the

[gab, 2016/06/10 16:15:21]

Is the idea that we would create |max_threads| SWTs with most in DETACHED
InitialState?

[robliao, 2016/06/10 18:03:41]

Yup. The new CL I have only creates one in ALIVE mode, the rest are in detached.

+  // worker will be created upon a WakeUp. Returns nullptr if creating the
+  // underlying platform thread fails during Create..

[gab, 2016/06/10 16:15:21]

s/.././

[robliao, 2016/06/10 18:03:41]

Done.

   static std::unique_ptr<SchedulerWorkerThread> Create(
       ThreadPriority thread_priority,
       std::unique_ptr<Delegate> delegate,
-      TaskTracker* task_tracker);
+      TaskTracker* task_tracker,
+      InitialWorkerState worker_state);
 
   // 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
   // returned by the GetWork() method of its delegate until it returns nullptr.
+  // May fail if the worker is detached.

[gab, 2016/06/10 16:15:21]

Document what failure means since this is a void return.

[robliao, 2016/06/10 18:03:41]

Done.

   void WakeUp();
 
   SchedulerWorkerThread::Delegate* delegate() { return delegate_.get(); }
 
   // Joins this SchedulerWorkerThread. If a Task is already running, it will be
   // allowed to complete its execution. This can only be called once.
   void JoinForTesting();

[gab, 2016/06/10 16:15:21]

Document that Detach() must always return false while this call is in progress.

[robliao, 2016/06/10 18:03:42]

Doc'ed in Delegate::CanDetach().

 
+  // Returns ture if the worker is alive.
+  bool WorkerAliveForTesting() const;
+
  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

[gab, 2016/06/10 16:15:21]

"for ownership by" => I don't understand this sentence

[robliao, 2016/06/10 18:03:41]

Clarified.

+  // 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_|
+  mutable 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_;
 
   // Synchronizes access to |should_exit_for_testing_|.
   mutable SchedulerLock should_exit_for_testing_lock_;
 
   // True once JoinForTesting() has been called.
   bool should_exit_for_testing_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(SchedulerWorkerThread);
 };
 
 }  // namespace internal
 }  // namespace base
 
 #endif  // BASE_TASK_SCHEDULER_SCHEDULER_WORKER_THREAD_H_