Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(230)

Unified Diff: cc/resources/pixel_buffer_raster_worker_pool.cc

Issue 17351017: Re-land: cc: Add raster finished signals to RasterWorkerPool. (Closed) Base URL: http://git.chromium.org/chromium/src.git@new-graph-build
Patch Set: new approach Created 7 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: cc/resources/pixel_buffer_raster_worker_pool.cc
diff --git a/cc/resources/pixel_buffer_raster_worker_pool.cc b/cc/resources/pixel_buffer_raster_worker_pool.cc
index 14a2536876e2c9f3b9a5f2f56a083db7020c2a3a..fee55b00bbcf819feb6bb84a5da265c43b525033 100644
--- a/cc/resources/pixel_buffer_raster_worker_pool.cc
+++ b/cc/resources/pixel_buffer_raster_worker_pool.cc
@@ -5,6 +5,7 @@
#include "cc/resources/pixel_buffer_raster_worker_pool.h"
#include "base/debug/trace_event.h"
+#include "cc/base/scoped_ptr_deque.h"
#include "cc/resources/resource.h"
#include "third_party/skia/include/core/SkDevice.h"
@@ -59,6 +60,31 @@ class PixelBufferWorkerPoolTaskImpl : public internal::WorkerPoolTask {
DISALLOW_COPY_AND_ASSIGN(PixelBufferWorkerPoolTaskImpl);
};
+class RasterFinishedWorkerPoolTaskImpl : public internal::WorkerPoolTask {
+ public:
+ RasterFinishedWorkerPoolTaskImpl(
+ base::MessageLoopProxy* origin_loop,
+ const base::Closure& on_raster_finished_callback)
+ : origin_loop_(origin_loop),
+ on_raster_finished_callback_(on_raster_finished_callback) {
+ }
+
+ // Overridden from internal::WorkerPoolTask:
+ virtual void RunOnWorkerThread(unsigned thread_index) OVERRIDE {
+ TRACE_EVENT0("cc", "RasterFinishedWorkerPoolTaskImpl::RunOnWorkerThread");
+ origin_loop_->PostTask(FROM_HERE, on_raster_finished_callback_);
+ }
+ virtual void CompleteOnOriginThread() OVERRIDE {}
+
+ private:
+ virtual ~RasterFinishedWorkerPoolTaskImpl() {}
+
+ scoped_refptr<base::MessageLoopProxy> origin_loop_;
+ const base::Closure on_raster_finished_callback_;
+
+ DISALLOW_COPY_AND_ASSIGN(RasterFinishedWorkerPoolTaskImpl);
+};
+
// If we raster too fast we become upload bound, and pending
// uploads consume memory. For maximum upload throughput, we would
// want to allow for upload_throughput * pipeline_time of pending
@@ -84,11 +110,16 @@ const size_t kMaxPendingRasterBytes =
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),
+ weak_ptr_factory_(this),
+ schedule_raster_tasks_count_(0),
+ finished_running_tasks_pending_(false),
+ finished_running_tasks_required_for_activation_pending_(false) {
}
PixelBufferRasterWorkerPool::~PixelBufferRasterWorkerPool() {
@@ -101,8 +132,12 @@ PixelBufferRasterWorkerPool::~PixelBufferRasterWorkerPool() {
void PixelBufferRasterWorkerPool::Shutdown() {
shutdown_ = true;
+ weak_ptr_factory_.InvalidateWeakPtrs();
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;
@@ -114,6 +149,7 @@ void PixelBufferRasterWorkerPool::Shutdown() {
completed_tasks_.push_back(task);
}
}
+ DCHECK_EQ(completed_tasks_.size(), pixel_buffer_tasks_.size());
}
void PixelBufferRasterWorkerPool::ScheduleTasks(RasterTask::Queue* queue) {
@@ -121,6 +157,9 @@ void PixelBufferRasterWorkerPool::ScheduleTasks(RasterTask::Queue* queue) {
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();
@@ -160,13 +199,31 @@ void PixelBufferRasterWorkerPool::ScheduleTasks(RasterTask::Queue* queue) {
}
}
+ 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))
+ 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() {
@@ -176,8 +233,11 @@ void PixelBufferRasterWorkerPool::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);
@@ -186,17 +246,10 @@ void PixelBufferRasterWorkerPool::CheckForCompletedTasks() {
task->CompleteOnOriginThread();
task->DidComplete();
- completed_tasks_.pop_front();
+ completed_tasks.pop_front();
}
}
-void PixelBufferRasterWorkerPool::OnRasterTasksFinished() {
- // Call CheckForCompletedTasks() when we've finished running all raster
- // tasks needed since last time ScheduleMoreTasks() was called. This
- // reduces latency when processing only a small number of raster tasks.
- CheckForCompletedRasterTasks();
-}
-
void PixelBufferRasterWorkerPool::FlushUploads() {
if (!has_performed_uploads_since_last_flush_)
return;
@@ -272,6 +325,8 @@ void PixelBufferRasterWorkerPool::CheckForCompletedUploads() {
task) == completed_tasks_.end());
completed_tasks_.push_back(task);
+ tasks_required_for_activation_.erase(task);
+
tasks_with_completed_uploads.pop_front();
}
}
@@ -301,21 +356,44 @@ void PixelBufferRasterWorkerPool::CheckForCompletedRasterTasks() {
CheckForCompletedUploads();
FlushUploads();
- ScheduleMoreTasks();
+ // Determine if client should be notified and update pending notification
+ // status before scheduling more raster tasks.
+ bool notify_client_that_finished_running_tasks_required_for_activation =
+ NotifyClientThatFinishedRunningTasksRequiredForActivation();
vmpstr 2013/06/27 23:31:21 ShouldNotify... I think we have to rethink the nam
reveman 2013/06/28 17:26:07 I agree. I'll come up with a better name asap. Let
+ bool notify_client_that_finished_running_tasks =
+ NotifyClientThatFinishedRunningTasks();
- // Make sure another check for completed uploads is scheduled
- // while there is still pending uploads left.
- if (!tasks_with_pending_upload_.empty())
+ if (PendingRasterTaskCount())
+ ScheduleMoreTasks();
+
+ // Schedule another check for completed raster tasks while there are
+ // pending raster tasks or pending uploads.
+ if (PendingRasterTaskCount() || !tasks_with_pending_upload_.empty())
ScheduleCheckForCompletedRasterTasks();
+
+ if (notify_client_that_finished_running_tasks_required_for_activation)
+ client()->DidFinishedRunningTasksRequiredForActivation();
+ if (notify_client_that_finished_running_tasks)
+ client()->DidFinishedRunningTasks();
}
void PixelBufferRasterWorkerPool::ScheduleMoreTasks() {
TRACE_EVENT0("cc", "PixelBufferRasterWorkerPool::ScheduleMoreTasks");
+ ++schedule_raster_tasks_count_;
+
+ enum RasterTaskType {
+ PREPAINT_TYPE = 0,
+ REQUIRED_FOR_ACTIVATION_TYPE = 1,
+ NUM_TYPES = 2
+ };
+ ScopedPtrDeque<GraphNode> tasks[NUM_TYPES];
+ unsigned priority = 2u; // 0-1 reserved for RasterFinished tasks.
+ TaskGraph graph;
+
size_t bytes_pending_upload = bytes_pending_upload_;
size_t bytes_pending_raster = 0;
- RasterTaskGraph graph;
for (RasterTaskVector::const_iterator it = raster_tasks().begin();
it != raster_tasks().end(); ++it) {
internal::RasterWorkerPoolTask* task = it->get();
@@ -359,9 +437,17 @@ void PixelBufferRasterWorkerPool::ScheduleMoreTasks() {
bytes_pending_raster = new_bytes_pending_raster;
bytes_pending_upload = new_bytes_pending_upload;
+ RasterTaskType type = IsRasterTaskRequiredForActivation(task) ?
+ REQUIRED_FOR_ACTIVATION_TYPE :
+ PREPAINT_TYPE;
+
// Use existing pixel buffer task if available.
if (pixel_buffer_task) {
- graph.InsertRasterTask(pixel_buffer_task, task->dependencies());
+ tasks[type].push_back(
+ CreateRasterTaskGraphNode(pixel_buffer_task,
+ task->dependencies(),
+ priority++,
+ &graph));
continue;
}
@@ -382,16 +468,113 @@ void PixelBufferRasterWorkerPool::ScheduleMoreTasks() {
base::Unretained(this),
make_scoped_refptr(task))));
pixel_buffer_tasks_[task] = new_pixel_buffer_task;
- graph.InsertRasterTask(new_pixel_buffer_task.get(), task->dependencies());
+ tasks[type].push_back(
+ CreateRasterTaskGraphNode(new_pixel_buffer_task.get(),
+ task->dependencies(),
+ priority++,
+ &graph));
}
- SetRasterTaskGraph(&graph);
+ scoped_refptr<internal::WorkerPoolTask> new_raster_finished_task;
+
+ DCHECK_LE(tasks[PREPAINT_TYPE].size() +
vmpstr 2013/06/27 23:31:21 The comments from irwp apply in in this area as we
reveman 2013/06/28 17:26:07 PTAL.
+ tasks[REQUIRED_FOR_ACTIVATION_TYPE].size(),
+ PendingRasterTaskCount());
+ // Schedule OnRasterFinished call when notification is pending
+ // and throttling is not preventing all pending tasks from
+ // being scheduled.
+ if ((tasks[PREPAINT_TYPE].size() +
+ tasks[REQUIRED_FOR_ACTIVATION_TYPE].size()) ==
+ PendingRasterTaskCount() &&
+ finished_running_tasks_pending_) {
+ new_raster_finished_task = make_scoped_refptr(
+ new RasterFinishedWorkerPoolTaskImpl(
+ base::MessageLoopProxy::current(),
+ base::Bind(&PixelBufferRasterWorkerPool::OnRasterFinished,
+ weak_ptr_factory_.GetWeakPtr(),
+ schedule_raster_tasks_count_)));
+ scoped_ptr<GraphNode> raster_finished_node(
+ new GraphNode(new_raster_finished_task.get(),
+ 1u)); // Priority 1
+ for (unsigned i = 0; i < NUM_TYPES; ++i) {
+ for (unsigned j = 0; j < tasks[i].size(); ++j) {
+ raster_finished_node->add_dependency();
+ tasks[i][j]->add_dependent(raster_finished_node.get());
+ }
+ }
+ graph.set(new_raster_finished_task.get(), raster_finished_node.Pass());
+ }
- // 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();
+ scoped_refptr<internal::WorkerPoolTask>
+ new_raster_required_for_activation_finished_task;
+
+ DCHECK_LE(tasks[REQUIRED_FOR_ACTIVATION_TYPE].size(),
+ tasks_required_for_activation_.size());
+ // Schedule OnRasterRequiredForActivationFinished call when
+ // notification is pending and throttling is not preventing
+ // all pending tasks required for activation from being scheduled.
+ if (tasks[REQUIRED_FOR_ACTIVATION_TYPE].size() ==
+ tasks_required_for_activation_.size() &&
+ finished_running_tasks_required_for_activation_pending_) {
+ new_raster_required_for_activation_finished_task = make_scoped_refptr(
+ new RasterFinishedWorkerPoolTaskImpl(
+ base::MessageLoopProxy::current(),
+ base::Bind(&PixelBufferRasterWorkerPool::
+ OnRasterRequiredForActivationFinished,
+ weak_ptr_factory_.GetWeakPtr(),
+ schedule_raster_tasks_count_)));
+
+ scoped_ptr<GraphNode> raster_required_for_activation_finished_node(
+ new GraphNode(new_raster_required_for_activation_finished_task.get(),
+ 0u)); // Priority 0
+ for (unsigned j = 0;
+ j < tasks[REQUIRED_FOR_ACTIVATION_TYPE].size();
+ ++j) {
+ raster_required_for_activation_finished_node->add_dependency();
+ tasks[REQUIRED_FOR_ACTIVATION_TYPE][j]->add_dependent(
+ raster_required_for_activation_finished_node.get());
+ }
+ graph.set(new_raster_required_for_activation_finished_task.get(),
+ raster_required_for_activation_finished_node.Pass());
+ }
+
+ for (unsigned i = 0; i < NUM_TYPES; ++i) {
+ while (!tasks[i].empty()) {
+ scoped_ptr<GraphNode> node = tasks[i].take_front();
+ internal::WorkerPoolTask* task = node->task();
+ graph.set(task, node.Pass());
+ }
+ }
+
+ SetTaskGraph(&graph);
+
+ raster_finished_task_.swap(new_raster_finished_task);
+ raster_required_for_activation_finished_task_.swap(
+ new_raster_required_for_activation_finished_task);
+}
+
+void PixelBufferRasterWorkerPool::OnRasterFinished(
+ int64 schedule_raster_tasks_count) {
+ TRACE_EVENT1("cc", "PixelBufferRasterWorkerPool::OnRasterFinished",
+ "schedule_raster_tasks_count", schedule_raster_tasks_count);
+ DCHECK_GE(schedule_raster_tasks_count_, schedule_raster_tasks_count);
+ // Call CheckForCompletedRasterTasks() if this callback is
+ // associated with the last call to SetRasterTaskGraph().
+ if (schedule_raster_tasks_count_ == schedule_raster_tasks_count)
+ CheckForCompletedRasterTasks();
+}
+
+void PixelBufferRasterWorkerPool::OnRasterRequiredForActivationFinished(
+ int64 schedule_raster_tasks_count) {
+ TRACE_EVENT1(
+ "cc",
+ "PixelBufferRasterWorkerPool::OnRasterRequiredForActivationFinished",
+ "schedule_raster_tasks_count", schedule_raster_tasks_count);
+ DCHECK_GE(schedule_raster_tasks_count_, schedule_raster_tasks_count);
+ // Call CheckForCompletedRasterTasks() if this callback is
+ // associated with the last call to SetRasterTaskGraph().
+ if (schedule_raster_tasks_count_ == schedule_raster_tasks_count)
+ CheckForCompletedRasterTasks();
}
void PixelBufferRasterWorkerPool::OnRasterTaskCompleted(
@@ -414,6 +597,7 @@ void PixelBufferRasterWorkerPool::OnRasterTaskCompleted(
completed_tasks_.end(),
task) == completed_tasks_.end());
completed_tasks_.push_back(task);
+ tasks_required_for_activation_.erase(task);
return;
}
@@ -424,4 +608,37 @@ void PixelBufferRasterWorkerPool::OnRasterTaskCompleted(
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;
+}
+
+bool PixelBufferRasterWorkerPool::
+ NotifyClientThatFinishedRunningTasksRequiredForActivation() {
+ if (!finished_running_tasks_required_for_activation_pending_)
+ return false;
+
+ if (!tasks_required_for_activation_.empty())
+ return false;
+
+ finished_running_tasks_required_for_activation_pending_ = false;
+ return true;
+}
+
+bool PixelBufferRasterWorkerPool::NotifyClientThatFinishedRunningTasks() {
+ if (!finished_running_tasks_pending_)
+ return false;
+
+ if (PendingRasterTaskCount())
+ return false;
+
+ if (!tasks_with_pending_upload_.empty())
+ return false;
+
+ finished_running_tasks_pending_ = false;
+ return true;
+}
+
} // namespace cc

Powered by Google App Engine
This is Rietveld 408576698