Re-land: cc: Add raster finished signals to RasterWorkerPool.

cc/resources/pixel_buffer_raster_worker_pool.cc

 // Copyright 2013 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/pixel_buffer_raster_worker_pool.h"
 
 #include "base/debug/trace_event.h"
 #include "cc/resources/resource.h"
 #include "third_party/skia/include/core/SkDevice.h"
 
@@ skipping 66 matching lines @@
 
 const int kCheckForCompletedRasterTasksDelayMs = 6;
 
 const size_t kMaxPendingRasterBytes =
     kMaxBytesUploadedPerMs * kCheckForCompletedRasterTasksDelayMs;
 
 }  // namespace
 
 PixelBufferRasterWorkerPool::PixelBufferRasterWorkerPool(
     ResourceProvider* resource_provider,
-    size_t num_threads) : RasterWorkerPool(resource_provider, num_threads),
-                          shutdown_(false),
-                          bytes_pending_upload_(0),
-                          has_performed_uploads_since_last_flush_(false),
-                          check_for_completed_raster_tasks_pending_(false) {
+    size_t num_threads)
+    : RasterWorkerPool(resource_provider, num_threads),
+      shutdown_(false),
+      bytes_pending_upload_(0),
+      has_performed_uploads_since_last_flush_(false),
+      check_for_completed_raster_tasks_pending_(false),
+      num_scheduled_raster_tasks_(0),
+      num_scheduled_raster_tasks_required_for_activation_(0),
+      finished_running_tasks_pending_(false),
+      finished_running_tasks_required_for_activation_pending_(false) {
 }
 
 PixelBufferRasterWorkerPool::~PixelBufferRasterWorkerPool() {
   DCHECK(shutdown_);
   DCHECK(!check_for_completed_raster_tasks_pending_);
   DCHECK_EQ(0u, pixel_buffer_tasks_.size());
   DCHECK_EQ(0u, tasks_with_pending_upload_.size());
   DCHECK_EQ(0u, completed_tasks_.size());
 }
 
 void PixelBufferRasterWorkerPool::Shutdown() {
   shutdown_ = true;
   RasterWorkerPool::Shutdown();
-  CheckForCompletedRasterTasks();
+  RasterWorkerPool::CheckForCompletedTasks();
+  CheckForCompletedUploads();
+  check_for_completed_raster_tasks_callback_.Cancel();
+  check_for_completed_raster_tasks_pending_ = false;
   for (TaskMap::iterator it = pixel_buffer_tasks_.begin();
        it != pixel_buffer_tasks_.end(); ++it) {
     internal::RasterWorkerPoolTask* task = it->first;
     internal::WorkerPoolTask* pixel_buffer_task = it->second.get();
 
     // All inactive tasks needs to be canceled.
     if (!pixel_buffer_task && !task->HasFinishedRunning()) {
       task->DidRun(true);
       completed_tasks_.push_back(task);
     }
   }
+  DCHECK_EQ(completed_tasks_.size(), pixel_buffer_tasks_.size());
 }
 
 void PixelBufferRasterWorkerPool::ScheduleTasks(RasterTask::Queue* queue) {
   TRACE_EVENT0("cc", "PixelBufferRasterWorkerPool::ScheduleTasks");
 
   RasterWorkerPool::SetRasterTasks(queue);
 
+  finished_running_tasks_pending_ = true;
+  finished_running_tasks_required_for_activation_pending_ = true;
+
   // Build new pixel buffer task set.
   TaskMap new_pixel_buffer_tasks;
   for (RasterTaskVector::const_iterator it = raster_tasks().begin();
        it != raster_tasks().end(); ++it) {
     internal::RasterWorkerPoolTask* task = it->get();
     DCHECK(new_pixel_buffer_tasks.find(task) == new_pixel_buffer_tasks.end());
     DCHECK(!task->HasCompleted());
 
     // Use existing pixel buffer task if available.
     TaskMap::iterator pixel_buffer_it = pixel_buffer_tasks_.find(task);
@@ skipping 19 matching lines @@
     // Inactive task can be canceled.
     if (!pixel_buffer_task && !task->HasFinishedRunning()) {
       task->DidRun(true);
       DCHECK(std::find(completed_tasks_.begin(),
                        completed_tasks_.end(),
                        task) == completed_tasks_.end());
       completed_tasks_.push_back(task);
     }
   }
 
+  tasks_required_for_activation_.clear();
+  raster_tasks_required_for_activation_.clear();
+  for (TaskMap::iterator it = new_pixel_buffer_tasks.begin();
+       it != new_pixel_buffer_tasks.end(); ++it) {
+    internal::RasterWorkerPoolTask* task = it->first;
+    if (IsRasterTaskRequiredForActivation(task)) {
+      internal::WorkerPoolTask* pixel_buffer_task = it->second.get();
+
+      tasks_required_for_activation_.insert(task);
+      if (!pixel_buffer_task || !pixel_buffer_task->HasCompleted())
+        raster_tasks_required_for_activation_.insert(task);
+    }
+  }
+
   pixel_buffer_tasks_.swap(new_pixel_buffer_tasks);
 
-  // This will schedule more tasks after checking for completed raster
-  // tasks. It's worth checking for completed tasks when ScheduleTasks()
-  // is called as priorities might have changed and this allows us to
-  // schedule as many new top priority tasks as possible.
-  CheckForCompletedRasterTasks();
+  // Check for completed tasks when ScheduleTasks() is called as
+  // priorities might have changed and this maximizes the number
+  // of top priority tasks that are scheduled.
+  RasterWorkerPool::CheckForCompletedTasks();
+  CheckForCompletedUploads();
+  FlushUploads();
+
+  // Schedule new tasks.
+  ScheduleMoreTasks();
+
+  // Cancel any pending check for completed raster tasks and schedule
+  // another check.
+  check_for_completed_raster_tasks_callback_.Cancel();
+  check_for_completed_raster_tasks_pending_ = false;
+  ScheduleCheckForCompletedRasterTasks();
 }
 
 void PixelBufferRasterWorkerPool::CheckForCompletedTasks() {
   TRACE_EVENT0("cc", "PixelBufferRasterWorkerPool::CheckForCompletedTasks");
 
   RasterWorkerPool::CheckForCompletedTasks();
   CheckForCompletedUploads();
   FlushUploads();
 
-  while (!completed_tasks_.empty()) {
-    internal::RasterWorkerPoolTask* task = completed_tasks_.front().get();
+  TaskDeque completed_tasks;
+  completed_tasks_.swap(completed_tasks);
+
+  while (!completed_tasks.empty()) {
+    internal::RasterWorkerPoolTask* task = completed_tasks.front().get();
     DCHECK(pixel_buffer_tasks_.find(task) != pixel_buffer_tasks_.end());
 
     pixel_buffer_tasks_.erase(task);
 
     task->DidComplete();
     task->DispatchCompletionCallback();
 
-    completed_tasks_.pop_front();
+    completed_tasks.pop_front();
   }
 }
 
 void PixelBufferRasterWorkerPool::OnRasterTasksFinished() {
+  // Early out when more raster tasks are pending.
+  if (num_scheduled_raster_tasks_ != PendingRasterTaskCount())

[vmpstr, 2013/06/26 18:05:07]

I don't understand this check. Shouldn't this be if (PendingRasterTaskCount() !=
0)?

PendingRasterTasks returns the total number of tasks pending raster in the
worker pool. num_scheduled_raster_tasks is the number we last scheduled. They
should only be the same if none of the tasks completed yet, no?

In either case, can you expand the comment a bit to explain the situation?

Also, OnRasterTasksFinished should only be called when the batch of raster tasks
is done. Why isn't that the case?

[reveman, 2013/06/27 09:06:58]

They will be the same if we're not throttled in any way. When throttled, only a
subset of our pending raster tasks have been scheduled.
It is as far as the WorkerPool and base RasterWorkerPool class knows. The
concept of throttling is applied at the PixelBufferRasterWorkerPool level.

+    return;
+
   // Call CheckForCompletedTasks() when we've finished running all raster
-  // tasks needed since last time ScheduleMoreTasks() was called. This
+  // tasks needed since last time ScheduleTasks() was called. This
   // reduces latency when processing only a small number of raster tasks.
   CheckForCompletedRasterTasks();
 }
 
+void PixelBufferRasterWorkerPool::OnRasterTasksRequiredForActivationFinished() {
+  if (raster_tasks_required_for_activation_.empty())
+    return;
+
+  // Early out when more raster tasks required for activation are pending.
+  if (raster_tasks_required_for_activation_.size() !=

[vmpstr, 2013/06/26 18:05:07]

I guess I'm missing something, since I have the same confusion about this check.
With no other scheduletasks being called, we remove things from
raster_tasks_required_for_activation_, but we never decrease num_..., so we
early out all but the first time?

[reveman, 2013/06/27 09:06:58]

Not sure what you mean by "first time". These functions are only called once for
each SetRasterTaskGraph call. And the reason we early out is that we only want
to handle these calls when they truly mean "raster finished" or "raster required
for activation finished".

It's not a disaster if we have an extra CheckForCompletedRasterTasks call but if
we don't take throttling into account we easily end up in a
SetRasterTaskGraph/CheckForCompletedRasterTask busy loop or keep calling
CheckForCompletedRasterTask after each task has finished.

[vmpstr, 2013/06/27 16:32:12]

What I mean is that, in the case where we schedule everything (we're not bound
by anything), it would be the case that
|raster_tasks_required_for_activation_.size()| ==
|num_scheduled_raster_tasks_required_for_activation_|. But, when a single task
finishes, we remove it from  raster_tasks_required_for_activation_ (via
OnRasterTaskCompleted). In that case we don't update num_scheduled... (or at
least I don't see where). From then on, all calls to this function will early
out, no?

[reveman, 2013/06/27 22:33:18]

hm, OnRasterTaskCompleted is usually not called before OnRasterFinished* unless
the tile manager calls RasterWorkerPool::CheckForCompletedTasks(), in which case
you're right that we might delay this notification up to 6ms.

+      num_scheduled_raster_tasks_required_for_activation_)
+    return;
+
+  OnRasterTasksFinished();
+}
+
 void PixelBufferRasterWorkerPool::FlushUploads() {
   if (!has_performed_uploads_since_last_flush_)
     return;
 
   resource_provider()->ShallowFlushIfSupported();
   has_performed_uploads_since_last_flush_ = false;
 }
 
 void PixelBufferRasterWorkerPool::CheckForCompletedUploads() {
   TaskDeque tasks_with_completed_uploads;
@@ skipping 55 matching lines @@
 
     bytes_pending_upload_ -= task->resource()->bytes();
 
     task->DidRun(false);
 
     DCHECK(std::find(completed_tasks_.begin(),
                      completed_tasks_.end(),
                      task) == completed_tasks_.end());
     completed_tasks_.push_back(task);
 
+    tasks_required_for_activation_.erase(task);
+
     tasks_with_completed_uploads.pop_front();
   }
 }
 
 void PixelBufferRasterWorkerPool::ScheduleCheckForCompletedRasterTasks() {
   if (check_for_completed_raster_tasks_pending_)
     return;
 
   check_for_completed_raster_tasks_callback_.Reset(
       base::Bind(&PixelBufferRasterWorkerPool::CheckForCompletedRasterTasks,
                  base::Unretained(this)));
   base::MessageLoopProxy::current()->PostDelayedTask(
       FROM_HERE,
       check_for_completed_raster_tasks_callback_.callback(),
       base::TimeDelta::FromMilliseconds(kCheckForCompletedRasterTasksDelayMs));
   check_for_completed_raster_tasks_pending_ = true;
 }
 
 void PixelBufferRasterWorkerPool::CheckForCompletedRasterTasks() {
   TRACE_EVENT0(
       "cc", "PixelBufferRasterWorkerPool::CheckForCompletedRasterTasks");
 
   check_for_completed_raster_tasks_callback_.Cancel();
   check_for_completed_raster_tasks_pending_ = false;
 
   RasterWorkerPool::CheckForCompletedTasks();
   CheckForCompletedUploads();
   FlushUploads();
 
-  ScheduleMoreTasks();
+  if (PendingRasterTaskCount())
+    ScheduleMoreTasks();
 
-  // Make sure another check for completed uploads is scheduled
-  // while there is still pending uploads left.
-  if (!tasks_with_pending_upload_.empty())
+  // Schedule another check for completed raster tasks while there are
+  // pending raster tasks or pending uploads.
+  if (PendingRasterTaskCount() || !tasks_with_pending_upload_.empty())
     ScheduleCheckForCompletedRasterTasks();
+
+  NotifyClientIfFinishedRunningTasksRequiredForActivation();
+  NotifyClientIfFinishedRunningTasks();
 }
 
 void PixelBufferRasterWorkerPool::ScheduleMoreTasks() {
   TRACE_EVENT0("cc", "PixelBufferRasterWorkerPool::ScheduleMoreTasks");
 
   size_t bytes_pending_upload = bytes_pending_upload_;
   size_t bytes_pending_raster = 0;
 
+  num_scheduled_raster_tasks_ = 0;
+  num_scheduled_raster_tasks_required_for_activation_ = 0;
+
   RasterTaskGraph graph;
   for (RasterTaskVector::const_iterator it = raster_tasks().begin();
        it != raster_tasks().end(); ++it) {
     internal::RasterWorkerPoolTask* task = it->get();
 
     // |pixel_buffer_tasks_| contains all tasks that have not yet completed.
     TaskMap::iterator pixel_buffer_it = pixel_buffer_tasks_.find(task);
     if (pixel_buffer_it == pixel_buffer_tasks_.end())
       continue;
 
@@ skipping 24 matching lines @@
     size_t new_bytes_pending_raster = bytes_pending_raster;
     new_bytes_pending_raster += task->resource()->bytes();
     if (new_bytes_pending_raster > kMaxPendingRasterBytes)
       break;
 
     // Update both |bytes_pending_raster| and |bytes_pending_upload|
     // now that task has cleared all throttling limits.
     bytes_pending_raster = new_bytes_pending_raster;
     bytes_pending_upload = new_bytes_pending_upload;
 
+    ++num_scheduled_raster_tasks_;
+    if (IsRasterTaskRequiredForActivation(task))
+      ++num_scheduled_raster_tasks_required_for_activation_;
+
     // Use existing pixel buffer task if available.
     if (pixel_buffer_task) {
-      graph.InsertRasterTask(pixel_buffer_task, task->dependencies());
+      graph.InsertRasterTask(pixel_buffer_task,
+                             task->dependencies(),
+                             IsRasterTaskRequiredForActivation(task));
       continue;
     }
 
     // Request a pixel buffer. This will reserve shared memory.
     resource_provider()->AcquirePixelBuffer(task->resource()->id());
 
     // MapPixelBuffer() returns NULL if context was lost at the time
     // AcquirePixelBuffer() was called. For simplicity we still post
     // a raster task that is essentially a noop in these situations.
     uint8* buffer = resource_provider()->MapPixelBuffer(
         task->resource()->id());
 
     scoped_refptr<internal::WorkerPoolTask> new_pixel_buffer_task(
         new PixelBufferWorkerPoolTaskImpl(
             task,
             buffer,
             base::Bind(&PixelBufferRasterWorkerPool::OnRasterTaskCompleted,
                        base::Unretained(this),
                        make_scoped_refptr(task))));
     pixel_buffer_tasks_[task] = new_pixel_buffer_task;
-    graph.InsertRasterTask(new_pixel_buffer_task.get(), task->dependencies());
+    graph.InsertRasterTask(new_pixel_buffer_task.get(),
+                           task->dependencies(),
+                           IsRasterTaskRequiredForActivation(task));
   }
 
+  // We might fail to schedule pending raster tasks when upload bound.
+  if (!bytes_pending_raster)
+    return;
+
   SetRasterTaskGraph(&graph);
-
-  // At least one task that could need an upload is now pending, schedule
-  // a check for completed raster tasks to ensure this upload is dispatched
-  // without too much latency.
-  if (bytes_pending_raster)
-    ScheduleCheckForCompletedRasterTasks();
 }
 
 void PixelBufferRasterWorkerPool::OnRasterTaskCompleted(
     scoped_refptr<internal::RasterWorkerPoolTask> task,
     bool was_canceled,
     bool needs_upload) {
   TRACE_EVENT2("cc", "PixelBufferRasterWorkerPool::OnRasterTaskCompleted",
                "was_canceled", was_canceled,
                "needs_upload", needs_upload);
 
   DCHECK(pixel_buffer_tasks_.find(task.get()) != pixel_buffer_tasks_.end());
 
   // Balanced with MapPixelBuffer() call in ScheduleMoreTasks().
   resource_provider()->UnmapPixelBuffer(task->resource()->id());
 
+  raster_tasks_required_for_activation_.erase(task);
+
   if (!needs_upload) {
     resource_provider()->ReleasePixelBuffer(task->resource()->id());
     task->DidRun(was_canceled);
     DCHECK(std::find(completed_tasks_.begin(),
                      completed_tasks_.end(),
                      task) == completed_tasks_.end());
     completed_tasks_.push_back(task);
+    tasks_required_for_activation_.erase(task);
     return;
   }
 
   resource_provider()->BeginSetPixels(task->resource()->id());
   has_performed_uploads_since_last_flush_ = true;
 
   bytes_pending_upload_ += task->resource()->bytes();
   tasks_with_pending_upload_.push_back(task);
 }
 
+unsigned PixelBufferRasterWorkerPool::PendingRasterTaskCount() const {
+  unsigned num_completed_raster_tasks =
+      tasks_with_pending_upload_.size() + completed_tasks_.size();
+  DCHECK_GE(pixel_buffer_tasks_.size(), num_completed_raster_tasks);
+  return pixel_buffer_tasks_.size() - num_completed_raster_tasks;
+}
+
+void PixelBufferRasterWorkerPool::
+    NotifyClientIfFinishedRunningTasksRequiredForActivation() {
+  if (!finished_running_tasks_required_for_activation_pending_)
+    return;
+
+  if (!tasks_required_for_activation_.empty())
+    return;
+
+  client()->DidFinishedRunningTasksRequiredForActivation();
+  finished_running_tasks_required_for_activation_pending_ = false;
+}
+
+void PixelBufferRasterWorkerPool::NotifyClientIfFinishedRunningTasks() {
+  if (!finished_running_tasks_pending_)
+    return;
+
+  if (PendingRasterTaskCount())
+    return;
+
+  if (!tasks_with_pending_upload_.empty())
+    return;
+
+  client()->DidFinishedRunningTasks();
+  finished_running_tasks_pending_ = false;
+}
+
 }  // namespace cc