cc: Generalize raster task notifications

cc/resources/gpu_raster_worker_pool.cc

 // Copyright 2014 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/resources/gpu_raster_worker_pool.h"
 
 #include "base/debug/trace_event.h"
 #include "cc/output/context_provider.h"
 #include "cc/resources/resource.h"
 #include "cc/resources/resource_provider.h"
@@ skipping 14 matching lines @@
 
 GpuRasterWorkerPool::GpuRasterWorkerPool(base::SequencedTaskRunner* task_runner,
                                          ContextProvider* context_provider,
                                          ResourceProvider* resource_provider)
     : task_runner_(task_runner),
       task_graph_runner_(new TaskGraphRunner),
       namespace_token_(task_graph_runner_->GetNamespaceToken()),
       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_);
 }
 
 GpuRasterWorkerPool::~GpuRasterWorkerPool() {
   DCHECK_EQ(0u, completed_tasks_.size());
 }
 
 Rasterizer* GpuRasterWorkerPool::AsRasterizer() {
   return this;
 }
 
 void GpuRasterWorkerPool::SetClient(RasterizerClient* client) {
   client_ = client;
 }
 
 void GpuRasterWorkerPool::Shutdown() {
   TRACE_EVENT0("cc", "GpuRasterWorkerPool::Shutdown");
 
   TaskGraph empty;
   task_graph_runner_->ScheduleTasks(namespace_token_, &empty);
   task_graph_runner_->WaitForTasksToFinishRunning(namespace_token_);
 }
 
 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;
 
   graph_.Reset();
 
   // 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();
        ++it) {
     const RasterTaskQueue::Item& item = *it;
     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() {
   TRACE_EVENT0("cc", "GpuRasterWorkerPool::CheckForCompletedTasks");
 
   task_graph_runner_->CollectCompletedTasks(namespace_token_,
                                             &completed_tasks_);
   for (Task::Vector::const_iterator it = completed_tasks_.begin();
        it != completed_tasks_.end();
        ++it) {
     RasterizerTask* task = static_cast<RasterizerTask*>(it->get());
 
     task->WillComplete();
     task->CompleteOnOriginThread(this);
     task->DidComplete();
 
     task->RunReplyOnOriginThread();
   }
   completed_tasks_.clear();
 }
 
 RasterBuffer* GpuRasterWorkerPool::AcquireBufferForRaster(RasterTask* task) {
   return resource_provider_->AcquireGpuRasterBuffer(task->resource()->id());
 }
 
 void GpuRasterWorkerPool::ReleaseBufferForRaster(RasterTask* task) {
   resource_provider_->ReleaseGpuRasterBuffer(task->resource()->id());
 }
 
-void GpuRasterWorkerPool::OnRasterFinished() {
-  TRACE_EVENT0("cc", "GpuRasterWorkerPool::OnRasterFinished");
+void GpuRasterWorkerPool::OnRasterTaskSetFinished(TaskSet task_set) {
+  TRACE_EVENT1("cc",
+               "GpuRasterWorkerPool::OnRasterTaskSetFinished",
+               "task_set",
+               task_set);
 
-  DCHECK(raster_tasks_pending_);
-  raster_tasks_pending_ = false;
-  client_->DidFinishRunningTasks();
-}
-
-void GpuRasterWorkerPool::OnRasterRequiredForActivationFinished() {
-  TRACE_EVENT0("cc",
-               "GpuRasterWorkerPool::OnRasterRequiredForActivationFinished");
-
-  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() {
   if (run_tasks_on_origin_thread_pending_)
     return;
 
   task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&GpuRasterWorkerPool::RunTasksOnOriginThread,
                  weak_ptr_factory_.GetWeakPtr()));
   run_tasks_on_origin_thread_pending_ = true;
 }
 
 void GpuRasterWorkerPool::RunTasksOnOriginThread() {
   TRACE_EVENT0("cc", "GpuRasterWorkerPool::RunTasksOnOriginThread");
 
   DCHECK(run_tasks_on_origin_thread_pending_);
   run_tasks_on_origin_thread_pending_ = false;
 
   ScopedGpuRaster gpu_raster(context_provider_);
   task_graph_runner_->RunUntilIdle();
 }
 
 }  // namespace cc