Add category handling to RasterWorkerPool

content/renderer/raster_worker_pool.cc

 // Copyright 2015 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 "content/renderer/raster_worker_pool.h"
 
-#include <stddef.h>
-#include <stdint.h>
-
+#include <string>
 #include <utility>
+#include <vector>
 
 #include "base/strings/stringprintf.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/trace_event/trace_event.h"
+#include "cc/base/math_util.h"
+#include "cc/raster/task_category.h"
 
 namespace content {
+namespace {
+
+// A thread which forwards to RasterWorkerPool::Run with the runnable
+// categories.
+class RasterWorkerPoolThread : public base::SimpleThread {
+ public:
+  explicit RasterWorkerPoolThread(const std::string& name_prefix,
+                                  const Options& options,
+                                  RasterWorkerPool* pool,
+                                  std::vector<cc::TaskCategory> categories)
+      : SimpleThread(name_prefix, options),
+        pool_(pool),
+        categories_(categories) {}
+
+  void Run() override { pool_->Run(categories_); }
+
+ private:
+  RasterWorkerPool* const pool_;
+  const std::vector<cc::TaskCategory> categories_;
+};
+
+}  // namespace
 
 // A sequenced task runner which posts tasks to a RasterWorkerPool.
 class RasterWorkerPool::RasterWorkerPoolSequencedTaskRunner
     : public base::SequencedTaskRunner {
  public:
   explicit RasterWorkerPoolSequencedTaskRunner(
       cc::TaskGraphRunner* task_graph_runner)
       : task_graph_runner_(task_graph_runner),
         namespace_token_(task_graph_runner->GetNamespaceToken()) {}
 
@@ skipping 18 matching lines @@
 
     tasks_.erase(tasks_.begin(), tasks_.begin() + completed_tasks_.size());
 
     tasks_.push_back(make_scoped_refptr(new ClosureTask(task)));
     graph_.Reset();
     for (const auto& graph_task : tasks_) {
       int dependencies = 0;
       if (!graph_.nodes.empty())
         dependencies = 1;
 
-      cc::TaskGraph::Node node(graph_task.get(), 0u /* category */,
+      // Treat any tasks that are enqueued through the SequencedTaskRunner as
+      // FOREGROUND priority. We don't have enough information to know the
+      // actual priority of such tasks, so we run them as soon as possible.
+      cc::TaskGraph::Node node(graph_task.get(), cc::TASK_CATEGORY_FOREGROUND,
                                0u /* priority */, dependencies);
       if (dependencies) {
         graph_.edges.push_back(
             cc::TaskGraph::Edge(graph_.nodes.back().task, node.task));
       }
       graph_.nodes.push_back(node);
     }
     task_graph_runner_->ScheduleTasks(namespace_token_, &graph_);
     completed_tasks_.clear();
     return true;
@@ skipping 23 matching lines @@
 };
 
 RasterWorkerPool::RasterWorkerPool()
     : namespace_token_(GetNamespaceToken()),
       has_ready_to_run_tasks_cv_(&lock_),
       has_namespaces_with_finished_running_tasks_cv_(&lock_),
       shutdown_(false) {}
 
 void RasterWorkerPool::Start(
     int num_threads,
+    bool use_single_thread_for_background_raster_tasks,
     const base::SimpleThread::Options& thread_options) {
   DCHECK(threads_.empty());
   while (threads_.size() < static_cast<size_t>(num_threads)) {
-    scoped_ptr<base::DelegateSimpleThread> thread(
-        new base::DelegateSimpleThread(
-            this, base::StringPrintf("CompositorTileWorker%u",
-                                     static_cast<unsigned>(threads_.size() + 1))
-                      .c_str(),
-            thread_options));
+    // Determine the categories that each thread can run.
+    std::vector<cc::TaskCategory> task_categories;
+
+    // The first thread can run nonconcurrent tasks.
+    if (threads_.size() == 0) {
+      task_categories.push_back(cc::TASK_CATEGORY_NONCONCURRENT_FOREGROUND);
+    }
+
+    // All threads can run foreground tasks.
+    task_categories.push_back(cc::TASK_CATEGORY_FOREGROUND);
+
+    // If we've enabled the use of a single thread for background tasks, only
+    // the first thread will run background tasks. Otherwise all threads may run
+    // them.
+    if (threads_.size() == 0 ||

[reveman, 2016/01/13 23:09:30]

hm, should we avoid using the same thread as NONCONCURRENT_FOREGROUND?
"threads_.size() == (num_threads - 1)" instead?

[ericrk, 2016/01/21 01:19:44]

with this setup, low priority tasks will never run until all nonconcurrent and
regular foreground tasks are started - if we moved it to the last thread, they
might run when there are still pending nonconcurrent foreground tasks...
although this might be fine, as nonconcurrent foreground tasks are, well,
nonconcurrent, so I don't expect as much contention...

I think your suggestion may make more sense (doesn't leave threads idle in more
scenarios), so changing this for now... WDYT?

[reveman, 2016/01/21 17:12:24]

Sgtm

+        !use_single_thread_for_background_raster_tasks) {
+      task_categories.push_back(cc::TASK_CATEGORY_BACKGROUND);
+    }
+
+    scoped_ptr<base::SimpleThread> thread(new RasterWorkerPoolThread(
+        base::StringPrintf("CompositorTileWorker%u",
+                           static_cast<unsigned>(threads_.size() + 1))
+            .c_str(),
+        thread_options, this, task_categories));
     thread->Start();
     threads_.push_back(std::move(thread));
   }
 }
 
 void RasterWorkerPool::Shutdown() {
   WaitForTasksToFinishRunning(namespace_token_);
   CollectCompletedTasks(namespace_token_, &completed_tasks_);
   // Shutdown raster threads.
   {
@@ skipping 28 matching lines @@
   cc::Task::Vector::iterator end = std::remove_if(
       tasks_.begin(), tasks_.end(), [this](const scoped_refptr<cc::Task>& e) {
         return std::find(this->completed_tasks_.begin(),
                          this->completed_tasks_.end(),
                          e) != this->completed_tasks_.end();
       });
   tasks_.erase(end, tasks_.end());
 
   tasks_.push_back(make_scoped_refptr(new ClosureTask(task)));
   graph_.Reset();
-  for (const auto& graph_task : tasks_)
+  for (const auto& graph_task : tasks_) {
+    // Delayed tasks are assigned FOREGROUND category, ensuring that they run as
+    // soon as possible once their delay has expired.
     graph_.nodes.push_back(
-        cc::TaskGraph::Node(graph_task.get(), 0u /* category */,
+        cc::TaskGraph::Node(graph_task.get(), cc::TASK_CATEGORY_FOREGROUND,
                             0u /* priority */, 0u /* dependencies */));
+  }
 
   ScheduleTasksWithLockAcquired(namespace_token_, &graph_);
   completed_tasks_.clear();
   return true;
 }
 
 bool RasterWorkerPool::RunsTasksOnCurrentThread() const {
   return true;
 }
 
-// Overridden from base::DelegateSimpleThread::Delegate:
-void RasterWorkerPool::Run() {
+void RasterWorkerPool::Run(const std::vector<cc::TaskCategory>& categories) {
   base::AutoLock lock(lock_);
 
   while (true) {
-    if (!RunTaskWithLockAcquired()) {
+    cc::TaskCategory category_to_run;
+    bool should_run = false;
+    for (const auto& category : categories) {
+      if (work_queue_.HasReadyToRunTasksForCategory(category)) {
+        should_run = true;
+        category_to_run = category;
+      }
+    }

[reveman, 2016/01/13 23:09:30]

nit: move this code into a new utility function that return false when no task
was ready to run. Maybe reuse the RunTaskWithLockAcquired name and rename the
RunTaskWithLockAcquired(cc::TaskCategory category) function below to
RunTaskInCategoryWithLockAcquired instead.

[ericrk, 2016/01/21 01:19:44]

Good suggestion.

+
+    if (!should_run) {
       // Exit when shutdown is set and no more tasks are pending.
       if (shutdown_)
         break;
 
       // Wait for more tasks.
       has_ready_to_run_tasks_cv_.Wait();
       continue;
     }
+    RunTaskWithLockAcquired(category_to_run);
   }
 }
 
 void RasterWorkerPool::FlushForTesting() {
   base::AutoLock lock(lock_);
 
   while (!work_queue_.HasFinishedRunningTasksInAllNamespaces()) {
     has_namespaces_with_finished_running_tasks_cv_.Wait();
   }
 }
@@ skipping 21 matching lines @@
 }
 
 void RasterWorkerPool::ScheduleTasksWithLockAcquired(cc::NamespaceToken token,
                                                      cc::TaskGraph* graph) {
   DCHECK(token.IsValid());
   DCHECK(!cc::TaskGraphWorkQueue::DependencyMismatch(graph));
   DCHECK(!shutdown_);
 
   work_queue_.ScheduleTasks(token, graph);
 
-  // If there is more work available, wake up worker thread.
+  // If there is more work available, wake up the other worker threads.
   if (work_queue_.HasReadyToRunTasks())
-    has_ready_to_run_tasks_cv_.Signal();
+    has_ready_to_run_tasks_cv_.Broadcast();
 }
 
 void RasterWorkerPool::WaitForTasksToFinishRunning(cc::NamespaceToken token) {
   TRACE_EVENT0("cc", "RasterWorkerPool::WaitForTasksToFinishRunning");
 
   DCHECK(token.IsValid());
 
   {
     base::AutoLock lock(lock_);
     base::ThreadRestrictions::ScopedAllowWait allow_wait;
@@ skipping 19 matching lines @@
   }
 }
 
 void RasterWorkerPool::CollectCompletedTasksWithLockAcquired(
     cc::NamespaceToken token,
     cc::Task::Vector* completed_tasks) {
   DCHECK(token.IsValid());
   work_queue_.CollectCompletedTasks(token, completed_tasks);
 }
 
-bool RasterWorkerPool::RunTaskWithLockAcquired() {
+void RasterWorkerPool::RunTaskWithLockAcquired(cc::TaskCategory category) {
   TRACE_EVENT0("toplevel", "TaskGraphRunner::RunTask");
 
   lock_.AssertAcquired();
 
-  // Find the first category with any tasks to run. This task graph runner
-  // treats categories as an additional priority.
-  // TODO(ericrk): Add more category/thread logic.
-  const auto& ready_to_run_namespaces = work_queue_.ready_to_run_namespaces();
-  auto found = std::find_if(
-      ready_to_run_namespaces.cbegin(), ready_to_run_namespaces.cend(),
-      [](const std::pair<uint16_t,
-                         cc::TaskGraphWorkQueue::TaskNamespace::Vector>& pair) {
-        return !pair.second.empty();
-      });
-
-  if (found == ready_to_run_namespaces.cend()) {
-    return false;
-  }
-
-  const uint16_t category = found->first;
   auto prioritized_task = work_queue_.GetNextTaskToRun(category);
   cc::Task* task = prioritized_task.task;
 
-  // There may be more work available, so wake up another worker thread.
-  if (work_queue_.HasReadyToRunTasks())
-    has_ready_to_run_tasks_cv_.Signal();
-
   // Call WillRun() before releasing |lock_| and running task.
   task->WillRun();
 
   {
     base::AutoUnlock unlock(lock_);
 
     task->RunOnWorkerThread();
   }
 
   // This will mark task as finished running.
   task->DidRun();
 
   work_queue_.CompleteTask(prioritized_task);
 
+  // We may have just dequeued more tasks, wake up the other worker threads.
+  if (work_queue_.HasReadyToRunTasks())
+    has_ready_to_run_tasks_cv_.Broadcast();
+
   // If namespace has finished running all tasks, wake up origin threads.
   if (work_queue_.HasFinishedRunningTasksInNamespace(
           prioritized_task.task_namespace))
     has_namespaces_with_finished_running_tasks_cv_.Broadcast();
-
-  return true;
 }
 
 RasterWorkerPool::ClosureTask::ClosureTask(const base::Closure& closure)
     : closure_(closure) {}
 
 // Overridden from cc::Task:
 void RasterWorkerPool::ClosureTask::RunOnWorkerThread() {
   closure_.Run();
   closure_.Reset();
 }
 
 RasterWorkerPool::ClosureTask::~ClosureTask() {}
 
 }  // namespace content