cc: Generalize raster task notifications

cc/resources/gpu_raster_worker_pool.cc

Index: cc/resources/gpu_raster_worker_pool.cc
diff --git a/cc/resources/gpu_raster_worker_pool.cc b/cc/resources/gpu_raster_worker_pool.cc
index 36d62aad8a195ecdb2cad7f75df0f25d6fd48ee0..ba37c328061eac051fa2e1d978cc946504cfce71 100644
--- a/cc/resources/gpu_raster_worker_pool.cc
+++ b/cc/resources/gpu_raster_worker_pool.cc
@@ -32,8 +32,6 @@ GpuRasterWorkerPool::GpuRasterWorkerPool(base::SequencedTaskRunner* task_runner,
       context_provider_(context_provider),
       resource_provider_(resource_provider),
       run_tasks_on_origin_thread_pending_(false),
-      raster_tasks_pending_(false),
-      raster_tasks_required_for_activation_pending_(false),
       raster_finished_weak_ptr_factory_(this),
       weak_ptr_factory_(this) {
   DCHECK(context_provider_);
@@ -62,14 +60,7 @@ void GpuRasterWorkerPool::Shutdown() {
 void GpuRasterWorkerPool::ScheduleTasks(RasterTaskQueue* queue) {
   TRACE_EVENT0("cc", "GpuRasterWorkerPool::ScheduleTasks");
 
-  DCHECK_EQ(queue->required_for_activation_count,
-            static_cast<size_t>(
-                std::count_if(queue->items.begin(),
-                              queue->items.end(),
-                              RasterTaskQueue::Item::IsRequiredForActivation)));
-
-  raster_tasks_pending_ = true;
-  raster_tasks_required_for_activation_pending_ = true;
+  raster_task_sets_pending_.set();

[reveman, 2014/09/10 16:41:49]

is it worth adding a comment here that makes it clear that this marks all task
sets as pending?

[ernstm, 2014/09/10 18:26:18]

Done.

 
   unsigned priority = kRasterTaskPriorityBase;
 
@@ -78,19 +69,22 @@ void GpuRasterWorkerPool::ScheduleTasks(RasterTaskQueue* queue) {
   // Cancel existing OnRasterFinished callbacks.
   raster_finished_weak_ptr_factory_.InvalidateWeakPtrs();
 
-  scoped_refptr<RasterizerTask>
-      new_raster_required_for_activation_finished_task(
-          CreateRasterRequiredForActivationFinishedTask(
-              queue->required_for_activation_count,
-              task_runner_.get(),
-              base::Bind(
-                  &GpuRasterWorkerPool::OnRasterRequiredForActivationFinished,
-                  raster_finished_weak_ptr_factory_.GetWeakPtr())));
-  scoped_refptr<RasterizerTask> new_raster_finished_task(
-      CreateRasterFinishedTask(
-          task_runner_.get(),
-          base::Bind(&GpuRasterWorkerPool::OnRasterFinished,
-                     raster_finished_weak_ptr_factory_.GetWeakPtr())));
+  scoped_refptr<RasterizerTask> new_task_set_finished_tasks[kNumberOfTaskSets];
+
+  TaskSetSizes task_set_sizes(queue);

[reveman, 2014/09/10 16:41:49]

We wouldn't need this if not for the synthetic delays. I think we need a better
way to handle this. How about we implement this delay in the tile manager by
delaying the actual raster tasks required by activation instead? We can land a
patch that changes this prior to this patch and you simply don't have to deal
with it here.

We can then replace "TaskSetSizes task_set_sizes" with "size_t
task_counts[lNumberOfTaskSets]" and compute it as we add tasks in the loop
below.

Wdyt?

[ernstm, 2014/09/10 18:26:18]

I'd prefer to do this as a follow-up.

[reveman, 2014/09/10 19:41:27]

I have to insist on doing this prior to this patch unless you see a problem with
my proposal. This patch is going to include awkward rasterizer API, more
complexity and more code unless we do this refactor first. I can take a look at
getting this done if you like? Or maybe you can ask Sami who added the original
SyntheticDelay support to raster tasks.

[ernstm, 2014/09/10 20:48:06]

Could we simply put the delay into the REQUIRED _FOR_ACTIVATION branch of
TileManger::DidFinishRunningTasks?

[reveman, 2014/09/10 21:00:52]

I don't think so. That will cause us to block on the compositor thread instead
of a worker thread.

Not sure exactly how these delays are supposed to be used but it seems like we'd
ideally begin the delay timer when scheduling tasks and wait for the delay when
running the last raster task required for activation. That would probably be
more correct than our current behavior of just always delaying the "finished"
task by independent of how long it actually took to run the real raster tasks.

+
+  for (TaskSet task_set = 0; task_set < kNumberOfTaskSets; ++task_set) {
+    base::debug::TraceEventSyntheticDelay* synthetic_delay = NULL;
+    if (task_set_sizes[task_set] > 0)
+      synthetic_delay = client_->SyntheticDelayForTaskSet(task_set);
+
+    new_task_set_finished_tasks[task_set] = CreateRasterFinishedTask(
+        task_runner_.get(),
+        base::Bind(&GpuRasterWorkerPool::OnRasterTaskSetFinished,
+                   raster_finished_weak_ptr_factory_.GetWeakPtr(),
+                   task_set),
+        synthetic_delay);
+  }
 
   for (RasterTaskQueue::Item::Vector::const_iterator it = queue->items.begin();
        it != queue->items.end();
@@ -99,34 +93,31 @@ void GpuRasterWorkerPool::ScheduleTasks(RasterTaskQueue* queue) {
     RasterTask* task = item.task;
     DCHECK(!task->HasCompleted());
 
-    if (item.required_for_activation) {
-      graph_.edges.push_back(TaskGraph::Edge(
-          task, new_raster_required_for_activation_finished_task.get()));
+    for (TaskSet task_set = 0; task_set < kNumberOfTaskSets; task_set++) {
+      if (!it->task_sets[task_set])

[reveman, 2014/09/10 16:41:49]

use item.task_sets instead of it->task_sets

[ernstm, 2014/09/10 18:26:18]

Done.

+        continue;
+
+      graph_.edges.push_back(
+          TaskGraph::Edge(task, new_task_set_finished_tasks[task_set].get()));
     }
 
     InsertNodesForRasterTask(&graph_, task, task->dependencies(), priority++);
-
-    graph_.edges.push_back(
-        TaskGraph::Edge(task, new_raster_finished_task.get()));
   }
 
-  InsertNodeForTask(&graph_,
-                    new_raster_required_for_activation_finished_task.get(),
-                    kRasterRequiredForActivationFinishedTaskPriority,
-                    queue->required_for_activation_count);
-  InsertNodeForTask(&graph_,
-                    new_raster_finished_task.get(),
-                    kRasterFinishedTaskPriority,
-                    queue->items.size());
+  for (TaskSet task_set = 0; task_set < kNumberOfTaskSets; task_set++) {
+    InsertNodeForTask(&graph_,
+                      new_task_set_finished_tasks[task_set].get(),
+                      kRasterTaskSetFinishedTaskPriority,
+                      task_set_sizes[task_set]);
+  }
 
   ScheduleTasksOnOriginThread(this, &graph_);
   task_graph_runner_->ScheduleTasks(namespace_token_, &graph_);
 
   ScheduleRunTasksOnOriginThread();
 
-  raster_finished_task_ = new_raster_finished_task;
-  raster_required_for_activation_finished_task_ =
-      new_raster_required_for_activation_finished_task;
+  for (TaskSet task_set = 0; task_set < kNumberOfTaskSets; task_set++)
+    task_set_finished_tasks_[task_set] = new_task_set_finished_tasks[task_set];

[reveman, 2014/09/10 16:41:49]

Maybe have |task_set_finished_tasks_| be a std::vector but keep
new_task_set_finished_tasks an array so you could use std::copy here instead of
a for loop. Current code is fine if you prefer it.

[ernstm, 2014/09/10 18:26:18]

We can use std::copy on two arrays as well. I like the arrays a bit better for
this case, so I kept them and just switched to std::copy.

[reveman, 2014/09/10 19:41:27]

Great. I didn't know std::copy worked with plain old arrays.

 }
 
 void GpuRasterWorkerPool::CheckForCompletedTasks() {
@@ -156,21 +147,15 @@ void GpuRasterWorkerPool::ReleaseBufferForRaster(RasterTask* task) {
   resource_provider_->ReleaseGpuRasterBuffer(task->resource()->id());
 }
 
-void GpuRasterWorkerPool::OnRasterFinished() {
-  TRACE_EVENT0("cc", "GpuRasterWorkerPool::OnRasterFinished");
-
-  DCHECK(raster_tasks_pending_);
-  raster_tasks_pending_ = false;
-  client_->DidFinishRunningTasks();
-}
-
-void GpuRasterWorkerPool::OnRasterRequiredForActivationFinished() {
-  TRACE_EVENT0("cc",
-               "GpuRasterWorkerPool::OnRasterRequiredForActivationFinished");
+void GpuRasterWorkerPool::OnRasterTaskSetFinished(TaskSet task_set) {
+  TRACE_EVENT1("cc",
+               "GpuRasterWorkerPool::OnRasterTaskSetFinished",
+               "task_set",
+               task_set);
 
-  DCHECK(raster_tasks_required_for_activation_pending_);
-  raster_tasks_required_for_activation_pending_ = false;
-  client_->DidFinishRunningTasksRequiredForActivation();
+  DCHECK(raster_task_sets_pending_[task_set]);
+  raster_task_sets_pending_[task_set] = false;
+  client_->DidFinishRunningTaskSet(task_set);
 }
 
 void GpuRasterWorkerPool::ScheduleRunTasksOnOriginThread() {