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 <limits>
 #include <string>
 
@@ skipping 135 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) {
-  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";
+// Task priorities that make sure that the task set done tasks run before any
+// other remaining tasks.
+const size_t kRequiredForActivationDoneTaskPriority = 1u;
+const size_t kRequiredForDrawDoneTaskPriority = 2u;
+const size_t kAllDoneTaskPriority = 3u;
+
+// For correctness, |kTileTaskPriorityBase| must be greater than
+// all task set done task priorities.
+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));
   }
 
-  NOTREACHED();
-  return "Invalid TaskSet";
+  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);
+};
+
 }  // 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) {
-  TRACE_EVENT1("cc", "TileManager::DidFinishRunningTileTasks", "task_set",
-               TaskSetName(task_set));
+void TileManager::DidFinishRunningTileTasksRequiredForActivation() {
+  TRACE_EVENT0("cc",
+               "TileManager::DidFinishRunningTileTasksRequiredForActivation");
+  signals_.ready_to_activate = true;
+  signals_check_notifier_.Schedule();
+
+  DidFinishRunningAnyTileTasks();
+}
+
+void TileManager::DidFinishRunningTileTasksRequiredForDraw() {
+  TRACE_EVENT0("cc", "TileManager::DidFinishRunningTileTasksRequiredForDraw");
+  signals_.ready_to_draw = true;
+  signals_check_notifier_.Schedule();
+
+  DidFinishRunningAnyTileTasks();
+}
+
+void TileManager::DidFinishRunningAllTileTasks() {
+  TRACE_EVENT0("cc", "TileManager::DidFinishRunningAllTileTasks");
   DCHECK(resource_pool_);
   DCHECK(tile_task_runner_);
 
-  switch (task_set) {
-    case ALL: {
-      has_scheduled_tile_tasks_ = false;
+  has_scheduled_tile_tasks_ = false;
 
-      bool memory_usage_above_limit = resource_pool_->memory_usage_bytes() >
-                                      global_state_.soft_memory_limit_in_bytes;
+  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
-        // notifications than always having to schedule a new task.
-        // http://crbug.com/498439
-        signals_.all_tile_tasks_completed = true;
-        signals_check_notifier_.Schedule();
-        return;
-      }
-
-      more_tiles_need_prepare_check_notifier_.Schedule();
-      return;
-    }
-    case REQUIRED_FOR_ACTIVATION:
-      signals_.ready_to_activate = true;
-      signals_check_notifier_.Schedule();
-      return;
-
-    case REQUIRED_FOR_DRAW:
-      signals_.ready_to_draw = true;
-      signals_check_notifier_.Schedule();
-      return;
+  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
+    // notifications than always having to schedule a new task.
+    // http://crbug.com/498439
+    signals_.all_tile_tasks_completed = true;
+    signals_check_notifier_.Schedule();
+    return;
   }
 
-  NOTREACHED();
+  more_tiles_need_prepare_check_notifier_.Schedule();
+
+  DidFinishRunningAnyTileTasks();
+}
+
+void TileManager::DidFinishRunningAnyTileTasks() {
+  if (has_scheduled_tile_tasks_) {
+    TRACE_EVENT_ASYNC_STEP_INTO1("cc", "ScheduledTasks", this, "running",
+                                 "state", ScheduledTasksStateAsValue());

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

can we call this directly in DidFinishRunningTileTasksRequiredForActivation and
DidFinishRunningTileTasksRequiredForDraw?

[ericrk, 2015/12/03 00:43:22]

yup - that makes sense.

+  } else {
+    TRACE_EVENT_ASYNC_END0("cc", "ScheduledTasks", this);

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

and this directly in DidFinishRunningAllTileTasks and remove this
DidFinishRunningAnyTileTasks helper function?

[ericrk, 2015/12/03 00:43:22]

Done.

+  }
 }
 
 bool TileManager::PrepareTiles(
     const GlobalStateThatImpactsTilePriority& state) {
   ++prepare_tiles_count_;
 
   TRACE_EVENT1("cc", "TileManager::PrepareTiles", "prepare_tiles_id",
                prepare_tiles_count_);
 
   if (!tile_task_runner_) {
@@ skipping 262 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 (!has_scheduled_tile_tasks_) {
+    TRACE_EVENT_ASYNC_BEGIN0("cc", "ScheduledTasks", this);
+  }
+
+  // Cancel existing OnTaskSetFinished callbacks.
+  task_set_finished_weak_ptr_factory_.InvalidateWeakPtrs();
 
   // 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();
+  // We need to keep the existing *_done_task_ tasks alive until after we
+  // call ScheduleTasks on our tile_task_runner_. BuildTaskGraph will replace
+  // these, so hold references here.
+  scoped_refptr<TileTask> old_required_for_activation_done_task;
+  scoped_refptr<TileTask> old_required_for_draw_done_task;
+  scoped_refptr<TileTask> old_all_done_task;

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

nit: how about "scoped_refptr<TileTask> new_*_done_task" here and then
"*_done_task_ = new_*_done_task_" after ScheduleTasks call and avoid all the
std::swap-ing? I think that would be a bit easier to understand.

[ericrk, 2015/12/03 00:43:22]

restructured things to avoid this.

+  std::swap(old_required_for_activation_done_task,
+            required_for_activation_done_task_);
+  std::swap(old_required_for_draw_done_task, required_for_draw_done_task_);
+  std::swap(old_all_done_task, all_done_task_);
 
-    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));
-  }
+  BuildTaskGraph(tiles_that_need_to_be_rasterized);
 
   // 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_);
 
   // 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();
 
   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->BeginDictionary("tasks_pending");
+  state->SetBoolean("ready_to_activate", signals_.ready_to_activate);
+  state->SetBoolean("ready_to_draw", signals_.ready_to_draw);
+  state->SetBoolean("all_tile_tasks_completed",
+                    signals_.all_tile_tasks_completed);
+  state->EndDictionary();
+  return state;
 }
 
+// Utility function that can be used to create a "Task set finished" task that
+// posts |callback| to |task_runner| when run.
+scoped_refptr<TileTask> TileManager::CreateTaskSetFinishedTask(
+    void (TileManager::*callback)()) {
+  return make_scoped_refptr(new TaskSetFinishedTaskImpl(
+      task_runner_.get(),
+      base::Bind(callback, task_set_finished_weak_ptr_factory_.GetWeakPtr())));
+}

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

nit: would it not be easier to read if this helper was removed and the code was
just inlined below? up to you

[ericrk, 2015/12/03 00:43:22]

Done.

+
+void TileManager::BuildTaskGraph(
+    const PrioritizedTileVector& tiles_that_need_to_be_rasterized) {
+  // Track the number of dependents for each *_done task.
+  size_t required_for_activate_count = 0;
+  size_t required_for_draw_count = 0;
+  size_t all_count = 0;
+
+  size_t priority = kTileTaskPriorityBase;
+
+  graph_.Reset();
+
+  required_for_activation_done_task_ = CreateTaskSetFinishedTask(
+      &TileManager::DidFinishRunningTileTasksRequiredForActivation);
+  required_for_draw_done_task_ = CreateTaskSetFinishedTask(
+      &TileManager::DidFinishRunningTileTasksRequiredForDraw);
+  all_done_task_ =
+      CreateTaskSetFinishedTask(&TileManager::DidFinishRunningAllTileTasks);

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

ah, I see why you did the std::swap in the other function now. would it not be
easier to create those tasks by the caller and just pass raw pointers to them to
this function? I think I'd perfer that but either way is fine with me

[ericrk, 2015/12/03 00:43:22]

Inlined this function, so it's no longer an issue - got rid of the swap stuff.

+
+  // 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_) {
+      tile->raster_task_ = CreateRasterTask(prioritized_tile);
+    }
+
+    RasterTask* task = tile->raster_task_.get();
+    DCHECK(!task->HasCompleted());
+
+    if (!tile->raster_task_.get())
+      tile->raster_task_ = CreateRasterTask(prioritized_tile);
+
+    if (tile->required_for_activation()) {
+      required_for_activate_count++;
+      graph_.edges.push_back(
+          TaskGraph::Edge(task, required_for_activation_done_task_.get()));
+    }
+    if (tile->required_for_draw()) {
+      required_for_draw_count++;
+      graph_.edges.push_back(
+          TaskGraph::Edge(task, required_for_draw_done_task_.get()));
+    }
+    all_count++;
+    graph_.edges.push_back(TaskGraph::Edge(task, all_done_task_.get()));
+
+    InsertNodesForRasterTask(&graph_, task, task->dependencies(), priority++);
+  }
+
+  InsertNodeForTask(&graph_, required_for_activation_done_task_.get(),
+                    kRequiredForActivationDoneTaskPriority,
+                    required_for_activate_count);
+  InsertNodeForTask(&graph_, required_for_draw_done_task_.get(),
+                    kRequiredForDrawDoneTaskPriority, required_for_draw_count);
+  InsertNodeForTask(&graph_, all_done_task_.get(), kAllDoneTaskPriority,
+                    all_count);
+}
+
+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