Move TaskGraph creation to TileManager

cc/tiles/tile_manager.cc

 // Copyright 2012 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 "cc/tiles/tile_manager.h"
 
 #include <algorithm>
+#include <array>
 #include <limits>
 #include <string>
 
 #include "base/bind.h"
 #include "base/json/json_writer.h"
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
 #include "base/numerics/safe_conversions.h"
 #include "base/trace_event/trace_event_argument.h"
 #include "cc/base/histograms.h"
@@ skipping 128 matching lines @@
   uint64_t resource_content_id_;
   int source_frame_number_;
   bool analyze_picture_;
   const base::Callback<void(const DisplayListRasterSource::SolidColorAnalysis&,
                             bool)> reply_;
   scoped_ptr<RasterBuffer> raster_buffer_;
 
   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";
     case TileManager::REQUIRED_FOR_ACTIVATION:
       return "REQUIRED_FOR_ACTIVATION";
     case TileManager::REQUIRED_FOR_DRAW:
       return "REQUIRED_FOR_DRAW";
   }
 
   NOTREACHED();
   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()
     : completed_count(0u), canceled_count(0u) {}
 
 scoped_refptr<base::trace_event::ConvertableToTraceFormat>
 RasterTaskCompletionStatsAsValue(const RasterTaskCompletionStats& stats) {
   scoped_refptr<base::trace_event::TracedValue> state =
       new base::trace_event::TracedValue();
   state->SetInteger("completed_count",
@@ skipping 29 matching lines @@
       did_oom_on_last_assign_(false),
       more_tiles_need_prepare_check_notifier_(
           task_runner_.get(),
           base::Bind(&TileManager::CheckIfMoreTilesNeedToBePrepared,
                      base::Unretained(this))),
       signals_check_notifier_(task_runner_.get(),
                               base::Bind(&TileManager::CheckAndIssueSignals,
                                          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();
 }
 
 void TileManager::FinishTasksAndCleanUp() {
   if (!tile_task_runner_)
     return;
 
   global_state_ = GlobalStateThatImpactsTilePriority();
 
-  TileTaskQueue empty;
-  tile_task_runner_->ScheduleTasks(&empty);
-  orphan_raster_tasks_.clear();
+  // This cancels tasks if possible, finishes pending tasks, and release any
+  // uninitialized resources.
+  tile_task_runner_->Shutdown();
 
-  // This should finish all 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();
   CleanUpReleasedTiles();
 
   tile_task_runner_ = nullptr;
   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,
                                TileTaskRunner* tile_task_runner,
                                size_t scheduled_raster_task_limit) {
   DCHECK(!tile_task_runner_);
   DCHECK(tile_task_runner);
 
   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) {
   released_tiles_.push_back(tile);
 }
 
 void TileManager::FreeResourcesForReleasedTiles() {
   for (auto* tile : released_tiles_)
     FreeResourcesForTile(tile);
 }
 
 void TileManager::CleanUpReleasedTiles() {
   std::vector<Tile*> tiles_to_retain;
   for (auto* tile : released_tiles_) {
     if (tile->HasRasterTask()) {
       tiles_to_retain.push_back(tile);
       continue;
     }
 
     DCHECK(!tile->draw_info().has_resource());
     DCHECK(tiles_.find(tile->id()) != tiles_.end());
     tiles_.erase(tile->id());
 
     image_decode_controller_.SubtractLayerUsedCount(tile->layer_id());
     delete tile;
   }
   released_tiles_.swap(tiles_to_retain);
 }
 
 void TileManager::DidFinishRunningTileTasks(TaskSet task_set) {

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

I think this would be much cleaner refactored as three different functions
instead of one function with enum and a switch statement.
DidFinishRunningTileTasksRequiredForActivation, etc.

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

Done.

   TRACE_EVENT1("cc", "TileManager::DidFinishRunningTileTasks", "task_set",
                TaskSetName(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;
 
       bool memory_usage_above_limit = resource_pool_->memory_usage_bytes() >
                                       global_state_.soft_memory_limit_in_bytes;
 
       if (all_tiles_that_need_to_be_rasterized_are_scheduled_ &&
           !memory_usage_above_limit) {
         // TODO(ericrk): We should find a better way to safely handle re-entrant
@@ skipping 294 matching lines @@
 void TileManager::FreeResourcesForTileAndNotifyClientIfTileWasReadyToDraw(
     Tile* tile) {
   bool was_ready_to_draw = tile->draw_info().IsReadyToDraw();
   FreeResourcesForTile(tile);
   if (was_ready_to_draw)
     client_->NotifyTileStateChanged(tile);
 }
 
 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.
   resource_pool_->ReduceResourceUsage();
 
   // 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(
     const PrioritizedTile& prioritized_tile) {
   Tile* tile = prioritized_tile.tile();
   uint64_t resource_content_id = 0;
   Resource* resource = nullptr;
   if (use_partial_raster_ && tile->invalidated_id()) {
     // TODO(danakj): For resources that are in use, we should still grab them
     // and copy from them instead of rastering everything. crbug.com/492754
@@ skipping 34 matching lines @@
 
 void TileManager::OnRasterTaskCompleted(
     Tile::Id tile_id,
     Resource* resource,
     const DisplayListRasterSource::SolidColorAnalysis& analysis,
     bool was_canceled) {
   DCHECK(tiles_.find(tile_id) != tiles_.end());
 
   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) {
     ++flush_stats_.canceled_count;
     // TODO(ericrk): If more partial raster work is done in the future, it may
     // be worth returning the resource to the pool with its previous ID (not
     // currently tracked). crrev.com/1370333002/#ps40001 has a possible method
     // of achieving this.
     resource_pool_->ReleaseResource(resource, 0 /* content_id */);
     return;
@@ skipping 43 matching lines @@
   DCHECK(tiles_.find(tile->id()) == tiles_.end());
 
   tiles_[tile->id()] = tile.get();
   image_decode_controller_.AddLayerUsedCount(tile->layer_id());
   return tile;
 }
 
 void TileManager::SetTileTaskRunnerForTesting(
     TileTaskRunner* tile_task_runner) {
   tile_task_runner_ = tile_task_runner;
-  tile_task_runner_->SetClient(this);
 }
 
 bool TileManager::AreRequiredTilesReadyToDraw(
     RasterTilePriorityQueue::Type type) const {
   scoped_ptr<RasterTilePriorityQueue> raster_priority_queue(
       client_->BuildRasterQueue(global_state_.tree_priority, type));
   // It is insufficient to check whether the raster queue we constructed is
   // empty. The reason for this is that there are situations (rasterize on
   // demand) when the tile both needs raster and it's ready to draw. Hence, we
   // have to iterate the queue to check whether the required tiles are ready to
@@ skipping 153 matching lines @@
 }
 
 ResourceFormat TileManager::DetermineResourceFormat(const Tile* tile) const {
   return tile_task_runner_->GetResourceFormat(!tile->is_opaque());
 }
 
 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)),
       resource_count_(static_cast<int>(resource_count)) {
   // MemoryUsage is constructed using size_ts, since it deals with memory and
   // the inputs are typically size_t. However, during the course of usage (in
   // particular operator-=) can cause internal values to become negative. Thus,
   // member variables are signed.
   DCHECK_LE(memory_bytes,
             static_cast<size_t>(std::numeric_limits<int64>::max()));
@@ skipping 54 matching lines @@
 void TileManager::Signals::reset() {
   ready_to_activate = false;
   did_notify_ready_to_activate = false;
   ready_to_draw = false;
   did_notify_ready_to_draw = false;
   all_tile_tasks_completed = false;
   did_notify_all_tile_tasks_completed = false;
 }
 
 }  // namespace cc