Move TaskGraph creation to TileManager

cc/tiles/tile_manager.cc

Index: cc/tiles/tile_manager.cc
diff --git a/cc/tiles/tile_manager.cc b/cc/tiles/tile_manager.cc
index 477025f3f80f9da5f0eb9304eafe787d7c398c64..b1e0c72cf213021c9acf82272f28ae8a5811717d 100644
--- a/cc/tiles/tile_manager.cc
+++ b/cc/tiles/tile_manager.cc
@@ -5,6 +5,7 @@
 #include "cc/tiles/tile_manager.h"
 
 #include <algorithm>
+#include <array>
 #include <limits>
 #include <string>
 
@@ -153,7 +154,7 @@ class RasterTaskImpl : public RasterTask {
   DISALLOW_COPY_AND_ASSIGN(RasterTaskImpl);
 };
 
-const char* TaskSetName(TaskSet task_set) {
+const char* TaskSetName(TileManager::TaskSet task_set) {
   switch (task_set) {
     case TileManager::ALL:
       return "ALL";
@@ -167,6 +168,98 @@ const char* TaskSetName(TaskSet task_set) {
   return "Invalid TaskSet";
 }
 
+// Task priorities that make sure task set finished tasks run before any
+// other remaining tasks. This is combined with the task set type to ensure
+// proper prioritization ordering between task set types.
+size_t kTaskSetFinishedTaskPriorityBase = 1u;
+// For correctness, |kTileTaskPriorityBase| must be greater than
+// |kTaskSetFinishedTaskPriorityBase + kNumberOfTaskSets|.
+size_t kTileTaskPriorityBase = 10u;
+
+void InsertNodeForTask(TaskGraph* graph,
+                       TileTask* task,
+                       size_t priority,
+                       size_t dependencies) {
+  DCHECK(std::find_if(graph->nodes.begin(), graph->nodes.end(),
+                      [task](const TaskGraph::Node& node) {
+                        return node.task == task;
+                      }) == graph->nodes.end());
+  graph->nodes.push_back(TaskGraph::Node(task, priority, dependencies));
+}
+
+void InsertNodesForRasterTask(TaskGraph* graph,
+                              RasterTask* raster_task,
+                              const ImageDecodeTask::Vector& decode_tasks,
+                              size_t priority) {
+  size_t dependencies = 0u;
+
+  // Insert image decode tasks.
+  for (ImageDecodeTask::Vector::const_iterator it = decode_tasks.begin();
+       it != decode_tasks.end(); ++it) {
+    ImageDecodeTask* decode_task = it->get();
+
+    // Skip if already decoded.
+    if (decode_task->HasCompleted())
+      continue;
+
+    dependencies++;
+
+    // Add decode task if it doesn't already exists in graph.
+    TaskGraph::Node::Vector::iterator decode_it =
+        std::find_if(graph->nodes.begin(), graph->nodes.end(),
+                     [decode_task](const TaskGraph::Node& node) {
+                       return node.task == decode_task;
+                     });
+    if (decode_it == graph->nodes.end())
+      InsertNodeForTask(graph, decode_task, priority, 0u);
+
+    graph->edges.push_back(TaskGraph::Edge(decode_task, raster_task));
+  }
+
+  InsertNodeForTask(graph, raster_task, priority, dependencies);
+}
+
+class TaskSetFinishedTaskImpl : public TileTask {
+ public:
+  explicit TaskSetFinishedTaskImpl(
+      base::SequencedTaskRunner* task_runner,
+      const base::Closure& on_task_set_finished_callback)
+      : task_runner_(task_runner),
+        on_task_set_finished_callback_(on_task_set_finished_callback) {}
+
+  // Overridden from Task:
+  void RunOnWorkerThread() override {
+    TRACE_EVENT0("cc", "TaskSetFinishedTaskImpl::RunOnWorkerThread");
+    TaskSetFinished();
+  }
+
+  // Overridden from TileTask:
+  void ScheduleOnOriginThread(TileTaskClient* client) override {}
+  void CompleteOnOriginThread(TileTaskClient* client) override {}
+
+ protected:
+  ~TaskSetFinishedTaskImpl() override {}
+
+  void TaskSetFinished() {
+    task_runner_->PostTask(FROM_HERE, on_task_set_finished_callback_);
+  }
+
+ private:
+  scoped_refptr<base::SequencedTaskRunner> task_runner_;
+  const base::Closure on_task_set_finished_callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(TaskSetFinishedTaskImpl);
+};
+
+// Utility function that can be used to create a "Task set finished" task that
+// posts |callback| to |task_runner| when run.
+scoped_refptr<TileTask> CreateTaskSetFinishedTask(

[reveman, 2015/12/02 05:22:55]

I'm generally not a fan of factory functions that does nothing more than add a
make_scoped_ptr/refptr. Is this function really useful?

[ericrk, 2015/12/02 21:28:08]

restructured things a bit - the function still exists, but is a bit more useful.

+    base::SequencedTaskRunner* task_runner,
+    const base::Closure& on_task_set_finished_callback) {
+  return make_scoped_refptr(
+      new TaskSetFinishedTaskImpl(task_runner, on_task_set_finished_callback));
+}
+
 }  // namespace
 
 RasterTaskCompletionStats::RasterTaskCompletionStats()
@@ -216,7 +309,8 @@ TileManager::TileManager(
                                          base::Unretained(this))),
       has_scheduled_tile_tasks_(false),
       prepare_tiles_count_(0u),
-      next_tile_id_(0u) {}
+      next_tile_id_(0u),
+      task_set_finished_weak_ptr_factory_(this) {}
 
 TileManager::~TileManager() {
   FinishTasksAndCleanUp();
@@ -228,13 +322,14 @@ void TileManager::FinishTasksAndCleanUp() {
 
   global_state_ = GlobalStateThatImpactsTilePriority();
 
-  TileTaskQueue empty;
-  tile_task_runner_->ScheduleTasks(&empty);
-  orphan_raster_tasks_.clear();
-
-  // This should finish all pending tasks and release any uninitialized
-  // resources.
+  // This cancels tasks if possible, finishes pending tasks, and release any
+  // uninitialized resources.
   tile_task_runner_->Shutdown();
+
+  // Now that all tasks have been finished, we can clear any
+  // |orphan_tasks_|.
+  orphan_tasks_.clear();
+
   tile_task_runner_->CheckForCompletedTasks();
 
   FreeResourcesForReleasedTiles();
@@ -244,6 +339,7 @@ void TileManager::FinishTasksAndCleanUp() {
   resource_pool_ = nullptr;
   more_tiles_need_prepare_check_notifier_.Cancel();
   signals_check_notifier_.Cancel();
+  task_set_finished_weak_ptr_factory_.InvalidateWeakPtrs();
 }
 
 void TileManager::SetResources(ResourcePool* resource_pool,
@@ -255,7 +351,6 @@ void TileManager::SetResources(ResourcePool* resource_pool,
   scheduled_raster_task_limit_ = scheduled_raster_task_limit;
   resource_pool_ = resource_pool;
   tile_task_runner_ = tile_task_runner;
-  tile_task_runner_->SetClient(this);
 }
 
 void TileManager::Release(Tile* tile) {
@@ -291,6 +386,16 @@ void TileManager::DidFinishRunningTileTasks(TaskSet task_set) {
   DCHECK(resource_pool_);
   DCHECK(tile_task_runner_);
 
+  DCHECK(tasks_pending_[task_set]);
+  tasks_pending_[task_set] = false;
+
+  if (tasks_pending_.any()) {
+    TRACE_EVENT_ASYNC_STEP_INTO1("cc", "ScheduledTasks", this, "running",
+                                 "state", ScheduledTasksStateAsValue());
+  } else {
+    TRACE_EVENT_ASYNC_END0("cc", "ScheduledTasks", this);
+  }
+
   switch (task_set) {
     case ALL: {
       has_scheduled_tile_tasks_ = false;
@@ -605,40 +710,29 @@ void TileManager::FreeResourcesForTileAndNotifyClientIfTileWasReadyToDraw(
 
 void TileManager::ScheduleTasks(
     const PrioritizedTileVector& tiles_that_need_to_be_rasterized) {
-  TRACE_EVENT1("cc",
-               "TileManager::ScheduleTasks",
-               "count",
+  TRACE_EVENT1("cc", "TileManager::ScheduleTasks", "count",
                tiles_that_need_to_be_rasterized.size());
 
   DCHECK(did_check_for_completed_tasks_since_last_schedule_tasks_);
 
-  raster_queue_.Reset();
+  if (tasks_pending_.none()) {
+    TRACE_EVENT_ASYNC_BEGIN0("cc", "ScheduledTasks", this);
+  }
+
+  // Cancel existing OnTaskSetFinished callbacks.
+  task_set_finished_weak_ptr_factory_.InvalidateWeakPtrs();
+  tasks_pending_.set();
 
   // Even when scheduling an empty set of tiles, the TTWP does some work, and
   // will always trigger a DidFinishRunningTileTasks notification. Because of
   // this we unconditionally set |has_scheduled_tile_tasks_| to true.
   has_scheduled_tile_tasks_ = true;
 
-  // Build a new task queue containing all task currently needed. Tasks
-  // are added in order of priority, highest priority task first.
-  for (auto& prioritized_tile : tiles_that_need_to_be_rasterized) {
-    Tile* tile = prioritized_tile.tile();
-
-    DCHECK(tile->draw_info().requires_resource());
-    DCHECK(!tile->draw_info().resource_);
-
-    if (!tile->raster_task_.get())
-      tile->raster_task_ = CreateRasterTask(prioritized_tile);
-
-    TaskSetCollection task_sets;
-    if (tile->required_for_activation())
-      task_sets.set(REQUIRED_FOR_ACTIVATION);
-    if (tile->required_for_draw())
-      task_sets.set(REQUIRED_FOR_DRAW);
-    task_sets.set(ALL);
-    raster_queue_.items.push_back(
-        TileTaskQueue::Item(tile->raster_task_.get(), task_sets));
-  }
+  scoped_ptr<TaskGraph> graph(new TaskGraph);

[reveman, 2015/12/02 05:22:54]

Before this we've swapped between two task graphs which we allowed to grow as
needed to minimize heap allocations each time we schedule a new graph. Those
heap allocations used to take ~50% of the time when scheduling a graph so it's
probably a good idea to keep doing that.

To be consistent with the TaskGraphRunner interface I suggest that
TileTaskRunner also takes raw pointer to a TaskGraph instead of a scoped_ptr and
ownership so that you can do this swap-between-two-graphs optimization here in
TM.

[ericrk, 2015/12/02 21:28:08]

from looking at the old code, it seems that we only need one graph, and to call
the Reset fn? Updated this to match the previous behavior (no new graphs).

+  std::array<scoped_refptr<TileTask>, kNumberOfTaskSets>

[reveman, 2015/12/02 05:22:54]

are we allowed to use std::array now?

[ericrk, 2015/12/02 21:28:08]

yup, went through the process of getting that approved, it's now in the style
guide... not useful anymore though :P

+      new_task_set_finished_tasks;
+  BuildTaskGraph(tiles_that_need_to_be_rasterized, graph.get(),
+                 &new_task_set_finished_tasks);
 
   // We must reduce the amount of unused resoruces before calling
   // ScheduleTasks to prevent usage from rising above limits.
@@ -647,14 +741,23 @@ void TileManager::ScheduleTasks(
   // Schedule running of |raster_queue_|. This replaces any previously
   // scheduled tasks and effectively cancels all tasks not present
   // in |raster_queue_|.
-  tile_task_runner_->ScheduleTasks(&raster_queue_);
+  tile_task_runner_->ScheduleTasks(graph.Pass());
 
   // It's now safe to clean up orphan tasks as raster worker pool is not
   // allowed to keep around unreferenced raster tasks after ScheduleTasks() has
   // been called.
-  orphan_raster_tasks_.clear();
+  orphan_tasks_.clear();
+
+  // Our |new_task_set_finished_tasks| need to be kept alive while the
+  // TaskGraphRunner may be using them.

[reveman, 2015/12/02 05:22:54]

They only need to be kept alive while scheduled. I think I would be prefer
explicit required_for_activation_finished_task_,
required_for_draw_finished_task_... members for this that we reset here. Wdyt?

[ericrk, 2015/12/02 21:28:08]

re-structured things along these lines.

+  for (int task_set = 0; task_set < kNumberOfTaskSets; ++task_set) {
+    orphan_tasks_.push_back(std::move(new_task_set_finished_tasks[task_set]));
+  }
 
   did_check_for_completed_tasks_since_last_schedule_tasks_ = false;
+
+  TRACE_EVENT_ASYNC_STEP_INTO1("cc", "ScheduledTasks", this, "running", "state",
+                               ScheduledTasksStateAsValue());
 }
 
 scoped_refptr<RasterTask> TileManager::CreateRasterTask(
@@ -709,7 +812,7 @@ void TileManager::OnRasterTaskCompleted(
 
   Tile* tile = tiles_[tile_id];
   DCHECK(tile->raster_task_.get());
-  orphan_raster_tasks_.push_back(tile->raster_task_);
+  orphan_tasks_.push_back(tile->raster_task_);
   tile->raster_task_ = nullptr;
 
   if (was_canceled) {
@@ -773,7 +876,6 @@ ScopedTilePtr TileManager::CreateTile(const Tile::CreateInfo& info,
 void TileManager::SetTileTaskRunnerForTesting(
     TileTaskRunner* tile_task_runner) {
   tile_task_runner_ = tile_task_runner;
-  tile_task_runner_->SetClient(this);
 }
 
 bool TileManager::AreRequiredTilesReadyToDraw(
@@ -947,9 +1049,80 @@ bool TileManager::DetermineResourceRequiresSwizzle(const Tile* tile) const {
   return tile_task_runner_->GetResourceRequiresSwizzle(!tile->is_opaque());
 }
 
-TileManager::MemoryUsage::MemoryUsage() : memory_bytes_(0), resource_count_(0) {
+scoped_refptr<base::trace_event::ConvertableToTraceFormat>
+TileManager::ScheduledTasksStateAsValue() const {
+  scoped_refptr<base::trace_event::TracedValue> state =
+      new base::trace_event::TracedValue();
+
+  state->BeginArray("tasks_pending");
+  for (int task_set = 0; task_set < kNumberOfTaskSets; ++task_set) {
+    state->AppendBoolean(tasks_pending_[task_set]);
+  }
+  state->EndArray();
+  return state;
 }
 
+void TileManager::BuildTaskGraph(
+    const PrioritizedTileVector& tiles_that_need_to_be_rasterized,
+    TaskGraph* graph,
+    std::array<scoped_refptr<TileTask>, kNumberOfTaskSets>*
+        new_task_set_finished_tasks) {
+  size_t task_count[kNumberOfTaskSets] = {0};
+
+  for (int task_set = 0; task_set < kNumberOfTaskSets; ++task_set) {
+    (*new_task_set_finished_tasks)[task_set] = CreateTaskSetFinishedTask(
+        task_runner_.get(),
+        base::Bind(&TileManager::DidFinishRunningTileTasks,
+                   task_set_finished_weak_ptr_factory_.GetWeakPtr(),
+                   static_cast<TaskSet>(task_set)));
+  }
+
+  size_t priority = kTileTaskPriorityBase;
+
+  // Build a new task queue containing all task currently needed. Tasks
+  // are added in order of priority, highest priority task first.
+  for (auto& prioritized_tile : tiles_that_need_to_be_rasterized) {
+    Tile* tile = prioritized_tile.tile();
+
+    DCHECK(tile->draw_info().requires_resource());
+    DCHECK(!tile->draw_info().resource_);
+
+    if (!tile->raster_task_.get())
+      tile->raster_task_ = CreateRasterTask(prioritized_tile);
+
+    TaskSetCollection task_sets;
+    if (tile->required_for_activation())
+      task_sets.set(REQUIRED_FOR_ACTIVATION);
+    if (tile->required_for_draw())
+      task_sets.set(REQUIRED_FOR_DRAW);
+    task_sets.set(ALL);
+
+    RasterTask* task = tile->raster_task_.get();
+    DCHECK(!task->HasCompleted());
+
+    for (int task_set = 0; task_set < kNumberOfTaskSets; ++task_set) {
+      if (!task_sets[task_set])
+        continue;
+
+      ++task_count[task_set];
+
+      graph->edges.push_back(TaskGraph::Edge(
+          task, (*new_task_set_finished_tasks)[task_set].get()));
+    }
+
+    InsertNodesForRasterTask(graph, task, task->dependencies(), priority++);
+  }
+
+  for (int task_set = 0; task_set < kNumberOfTaskSets; ++task_set) {
+    InsertNodeForTask(graph, (*new_task_set_finished_tasks)[task_set].get(),
+                      kTaskSetFinishedTaskPriorityBase + task_set,
+                      task_count[task_set]);
+  }
+}
+
+TileManager::MemoryUsage::MemoryUsage()
+    : memory_bytes_(0), resource_count_(0) {}
+
 TileManager::MemoryUsage::MemoryUsage(size_t memory_bytes,
                                       size_t resource_count)
     : memory_bytes_(static_cast<int64>(memory_bytes)),