TaskScheduler [7] SchedulerThreadPool

base/task_scheduler/utils.cc

+// 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.
+
+#include "base/task_scheduler/utils.h"
+
+#include <utility>
+
+#include "base/logging.h"
+#include "base/memory/ptr_util.h"
+#include "base/task_scheduler/priority_queue.h"
+#include "base/task_scheduler/sequence_sort_key.h"
+#include "base/task_scheduler/task_tracker.h"
+
+namespace base {
+namespace internal {
+
+bool PostTaskHelper(std::unique_ptr<Task> task,

[danakj, 2016/04/07 23:08:44]

Can this be a method on SchedulerThreadPool?

That feels like it's advantageous in a few ways:
- We don't leave scheduler_thread_pool.cc when we're posting tasks, easier to
follow.
- We don't have to pass around queue pointers and stuff, we just use the
members.
- That makes SchedulerThreadPool the only caller of BeginTransaction()->Push()
right? Then maybe it can just call WakeUpOneThread on itself instead of going
(SchedulerThreadPool or *TaskRunner) -> PriorityQueue::Transaction ->
SchedulerThreadPool to wake up a thread.

[fdoray, 2016/04/08 14:53:03]

This method will also be used with single-threaded PriorityQueues. If I move it
to SchedulerThreadPool:
- |priority_queue| will still be an argument.
- single-threaded task runners declared in scheduler_worker_thread.cc will have
to know about SchedulerThreadPool.

[danakj, 2016/04/08 18:05:32]

Why aren't single-threaded queues a SchedulerThreadPool with 1 thread? Maybe you
can elaborate.
These single-thread ones wouldn't be waking up a thread I imagine, so maybe they
could also separately be a place that calls Transaction::Push?

I'm mostly trying to remove the callback for pushing and just do what work is in
that callback at the callsite to Push(), since it's coming from the same class
as the callback's destination.

[fdoray, 2016/04/08 19:00:05]

Each thread in a pool will get work from both the pool's shared PQ and from its
single-threaded PQ. PostTaskHelper can be used to post a task to either kind of
PQ.

Currently, you see PostTaskHelper being used in scheduler_thread_pool.cc, but it
will also be used in scheduler_worker_thread.cc (by
SchedulerSingleThreadTaskRunner) and delayed_task_manager.cc (to insert a task
in its sequence/PQ when its delay expires).

Because PQ has a callback to wake up a thread when a Sequence is inserted into
it, delayed_task_manager.cc will be able to use shared/single-thread PQs without
knowing the difference between the two. Without the callback,
delayed_task_manager.cc will have to know that it has to call
SchedulerThreadPool::WakeUpOneThread when it inserts a Sequence in a shared PQ
and SchedulerWorkerThread::WakeUp when it inserts a Sequence in a
single-threaded PQ.

[fdoray, 2016/04/11 14:25:46]

Usage of this helper in DelayedTaskManager:
https://codereview.chromium.org/1806473002/diff/160001/base/task_scheduler/de...

+                    scoped_refptr<Sequence> sequence,
+                    PriorityQueue* priority_queue,
+                    TaskTracker* task_tracker) {
+  DCHECK(task);
+  DCHECK(sequence);
+  DCHECK(priority_queue);
+  DCHECK(task_tracker);
+
+  if (!task_tracker->WillPostTask(task.get()))
+    return false;
+
+  const bool sequence_was_empty = sequence->PushTask(std::move(task));
+  if (sequence_was_empty) {
+    // Insert |sequence| in |priority_queue| if it was empty before |task| was
+    // inserted into it. When that's not the case, one of these must be true:
+    // - |sequence| is already in a PriorityQueue, or,
+    // - A worker thread is running a Task from |sequence|. It will insert
+    //   |sequence| in a PriorityQueue once it's done running the Task.
+    const SequenceSortKey sequence_sort_key = sequence->GetSortKey();
+    priority_queue->BeginTransaction()->Push(
+        WrapUnique(new PriorityQueue::SequenceAndSortKey(std::move(sequence),
+                                                         sequence_sort_key)));
+  }
+
+  return true;
+}
+
+}  // namespace internal
+}  // namespace base