TaskScheduler [6] SchedulerWorkerThread

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
new file mode 100644
index 0000000000000000000000000000000000000000..908eaed93a9ea4c9a4aec9d93f08f41e45815a9b
--- /dev/null
+++ b/base/task_scheduler/scheduler_worker_thread.h
@@ -0,0 +1,109 @@
+// 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 "base/base_export.h"
+#include "base/callback.h"
+#include "base/macros.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.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"
+
+namespace base {
+namespace internal {
+
+class TaskTracker;
+
+// A thread that runs Tasks from Sequences returned by a callback.
+//
+// A SchedulerWorkerThread is woken up when its WakeUp() method is called. After
+// a wake- up, a SchedulerWorkerThread runs Tasks from Sequences returned by its
+// "get work" callback as long as it doesn't return nullptr. It also
+// periodically checks with its TaskTracker whether shutdown has completed and
+// exits when it has.
+//
+// This class is thread-safe.
+class BASE_EXPORT SchedulerWorkerThread : public PlatformThread::Delegate {
+ public:
+  // Callback invoked to get a Sequence from which to run a Task on
+  // |worker_thread|.
+  using GetWorkCallback =
+      Callback<scoped_refptr<Sequence>(SchedulerWorkerThread* worker_thread)>;
+
+  // Callback invoked after |worker_thread| has tried to run a Task from
+  // |sequence| (a TaskTracker might have prevented the Task from running).
+  using RanTaskFromSequenceCallback =
+      Callback<void(const SchedulerWorkerThread* worker_thread,
+                    scoped_refptr<Sequence> sequence)>;
+
+  // Creates a SchedulerWorkerThread with priority |thread_priority| that runs
+  // Tasks from Sequences returned by |get_work_callback|. |main_entry_callback|
+  // is invoked when the main function of the SchedulerWorkerThread is entered.
+  // |ran_task_from_sequence_callback| is invoked after the
+  // SchedulerWorkerThread has tried to run a Task from a Sequence returned by
+  // |get_work_callback|. |task_tracker| is used to handle shutdown behavior of
+  // Tasks. Returns nullptr if creating the underlying platform thread fails.
+  static scoped_ptr<SchedulerWorkerThread> CreateSchedulerWorkerThread(
+      ThreadPriority thread_priority,
+      const Closure& main_entry_callback,
+      const GetWorkCallback& get_work_callback,
+      const RanTaskFromSequenceCallback& ran_task_from_sequence_callback,
+      TaskTracker* task_tracker);
+
+  // 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;
+
+  // Wakes up this SchedulerWorkerThread. After this is called, this
+  // SchedulerWorkerThread will run Tasks from Sequences returned by
+  // |get_work_callback_| until it returns nullptr.
+  void WakeUp();
+
+  // 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();
+
+ private:
+  SchedulerWorkerThread(
+      ThreadPriority thread_priority,
+      const Closure& main_entry_callback,
+      const GetWorkCallback& get_work_callback,
+      const RanTaskFromSequenceCallback& ran_task_from_sequence_callback,

[gab, 2016/04/05 23:35:20]

1 callback is essentially a mini Delegate, 2 callbacks is still okay, but 3
Callbacks is when my review bell really starts thinking we should be merging
them into a single Delegate here, especially if in practice all callbacks are
all bound to dedicated methods on the caller's class.

[fdoray, 2016/04/07 16:07:43]

Done here https://codereview.chromium.org/1864333002/

+      TaskTracker* task_tracker);
+
+  // PlatformThread::Delegate:
+  void ThreadMain() override;
+
+  bool ShouldExitForTesting() const;
+
+  // Platform thread managed by this SchedulerWorkerThread.
+  PlatformThreadHandle thread_handle_;
+
+  // Event signaled to wake up this SchedulerWorkerThread.
+  WaitableEvent wake_up_event_;
+
+  const Closure main_entry_callback_;
+  const GetWorkCallback get_work_callback_;
+  const RanTaskFromSequenceCallback ran_task_from_sequence_callback_;
+  TaskTracker* const task_tracker_;
+
+  // Synchronizes access to |should_exit_for_testing_|.
+  mutable SchedulerLock should_exit_for_testing_lock_;

[gab, 2016/04/05 23:35:20]

Instead of using a lock just for this here we could make
|should_exit_for_testing_| volatile, also makes the impl cleaner IMO since it
doesn't have to have locking scopes :-).

Note: if I understand the C++ standard correctly I think this means you'll have
to make the methods using |should_exit_for_testing_| volatile as well (same
semantics as "const"). Never used volatile in C++ but curious whether it'd make
things cleaner here :-)?

[fdoray, 2016/04/07 16:07:43]

If should_exit_for_testing_ is volatile:

Thread 1:
  should_exit_for_testing_ = true;
  wake_up_event_.Signal();

Thread 2:
  wake_up_event_.Wait();
  if (should_exit_for_testing_) {}

There is no guarantee that the condition is true on thread 2 unless we know that
WaitableEvent::Signal/Wait both have visible side effects. I'm pretty sure that
these methods have visible side effects (pending input from etienneb@), but I
don't like relying on that fact.

"within a single thread of execution, volatile accesses cannot be optimized out
or reordered with another visible side effect that is sequenced-before or
sequenced-after the volatile access. This makes volatile objects suitable for
communication with a signal handler, but not with another thread of execution,
see std::memory_order" http://en.cppreference.com/w/cpp/language/cv
  

[gab, 2016/04/07 16:49:07]

Ah interesting, so it's not like Java's volatile which, IIRC, is essentially
like wrapping all access to |foo| into a |synchronized(foo){}| block. I guess
that's because Java natively supports multi-threading whereas C++ doesn't.

+
+  // True once JoinForTesting() has been called.
+  bool should_exit_for_testing_ = false;
+
+  DISALLOW_COPY_AND_ASSIGN(SchedulerWorkerThread);
+};

[gab, 2016/04/05 23:35:20]

Where did the single threaded PQ go?

[fdoray, 2016/04/07 16:07:43]

I will add it in another CL. It will be probably be owned by
SchedulerWorkerThread, but could also be owned by SchedulerThreadPool.

+
+}  // namespace internal
+}  // namespace base
+
+#endif  // BASE_TASK_SCHEDULER_SCHEDULER_WORKER_THREAD_H_