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

Unified Diff: content/renderer/raster_worker_pool.cc

Issue 1666283002: Reland - Refactor signaling in RWP (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 4 years, 10 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: content/renderer/raster_worker_pool.cc
diff --git a/content/renderer/raster_worker_pool.cc b/content/renderer/raster_worker_pool.cc
index 0924c0a5118b24e3a97717b8e842d3045fe55286..b365a5ce575feb29d028f619a95b6ae566646266 100644
--- a/content/renderer/raster_worker_pool.cc
+++ b/content/renderer/raster_worker_pool.cc
@@ -24,16 +24,14 @@ class RasterWorkerPoolThread : public base::SimpleThread {
explicit RasterWorkerPoolThread(const std::string& name_prefix,
const Options& options,
RasterWorkerPool* pool,
- std::vector<cc::TaskCategory> categories)
- : SimpleThread(name_prefix, options),
- pool_(pool),
- categories_(categories) {}
+ cc::TaskCategory category)
+ : SimpleThread(name_prefix, options), pool_(pool), category_(category) {}
- void Run() override { pool_->Run(categories_); }
+ void Run() override { pool_->Run(category_); }
private:
RasterWorkerPool* const pool_;
- const std::vector<cc::TaskCategory> categories_;
+ const cc::TaskCategory category_;
};
} // namespace
@@ -116,7 +114,13 @@ class RasterWorkerPool::RasterWorkerPoolSequencedTaskRunner
RasterWorkerPool::RasterWorkerPool()
: namespace_token_(GetNamespaceToken()),
- has_ready_to_run_tasks_cv_(&lock_),
+ nonconcurrent_foreground_has_ready_to_run_tasks_cv_(&lock_),
+ foreground_has_ready_to_run_tasks_cv_(&lock_),
+ background_has_ready_to_run_tasks_cv_(&lock_),
+ nonconcurrent_foreground_running_task_count_(0),
+ foreground_running_task_count_(0),
+ background_running_task_count_(0),
+ target_running_task_count_(0),
has_namespaces_with_finished_running_tasks_cv_(&lock_),
shutdown_(false) {}
@@ -124,28 +128,38 @@ void RasterWorkerPool::Start(
int num_threads,
const base::SimpleThread::Options& thread_options) {
DCHECK(threads_.empty());
- while (threads_.size() < static_cast<size_t>(num_threads)) {
- // Determine the categories that each thread can run.
- std::vector<cc::TaskCategory> task_categories;
- // The first thread can run nonconcurrent tasks.
- if (threads_.size() == 0) {
- task_categories.push_back(cc::TASK_CATEGORY_NONCONCURRENT_FOREGROUND);
- }
+ target_running_task_count_ = num_threads;
- // All threads can run foreground tasks.
- task_categories.push_back(cc::TASK_CATEGORY_FOREGROUND);
+ // Start a single thread for nonconcurrent foreground work.
+ {
+ scoped_ptr<base::SimpleThread> thread(new RasterWorkerPoolThread(
+ base::StringPrintf("CompositorTileWorker%u",
+ static_cast<unsigned>(threads_.size() + 1))
+ .c_str(),
+ thread_options, this, cc::TASK_CATEGORY_NONCONCURRENT_FOREGROUND));
+ thread->Start();
+ threads_.push_back(std::move(thread));
+ }
- // The last thread can run background tasks.
- if (threads_.size() == (static_cast<size_t>(num_threads) - 1)) {
- task_categories.push_back(cc::TASK_CATEGORY_BACKGROUND);
- }
+ // start |num_threads| threads for foreground work.
+ for (int i = 0; i < num_threads; i++) {
+ scoped_ptr<base::SimpleThread> thread(new RasterWorkerPoolThread(
+ base::StringPrintf("CompositorTileWorker%u",
+ static_cast<unsigned>(threads_.size() + 1))
+ .c_str(),
+ thread_options, this, cc::TASK_CATEGORY_FOREGROUND));
+ thread->Start();
+ threads_.push_back(std::move(thread));
+ }
+ // Start a single thread for background work.
+ {
scoped_ptr<base::SimpleThread> thread(new RasterWorkerPoolThread(
base::StringPrintf("CompositorTileWorker%u",
static_cast<unsigned>(threads_.size() + 1))
.c_str(),
- thread_options, this, task_categories));
+ thread_options, this, cc::TASK_CATEGORY_BACKGROUND));
thread->Start();
threads_.push_back(std::move(thread));
}
@@ -165,7 +179,9 @@ void RasterWorkerPool::Shutdown() {
shutdown_ = true;
// Wake up all workers so they exit.
- has_ready_to_run_tasks_cv_.Broadcast();
+ nonconcurrent_foreground_has_ready_to_run_tasks_cv_.Broadcast();
+ foreground_has_ready_to_run_tasks_cv_.Broadcast();
+ background_has_ready_to_run_tasks_cv_.Broadcast();
}
while (!threads_.empty()) {
threads_.back()->Join();
@@ -211,19 +227,25 @@ bool RasterWorkerPool::RunsTasksOnCurrentThread() const {
return true;
}
-void RasterWorkerPool::Run(const std::vector<cc::TaskCategory>& categories) {
+void RasterWorkerPool::Run(cc::TaskCategory category) {
base::AutoLock lock(lock_);
+ base::ConditionVariable& cv = ConditionVariableForCategory(category);
while (true) {
- if (!RunTaskWithLockAcquired(categories)) {
+ if (!ShouldRunTaskForCategory(category)) {
+ // We are no longer running tasks, which may allow another category to
+ // start running. Signal other worker threads.
+ SignalTaskReadyConditionVariables();
+
// Exit when shutdown is set and no more tasks are pending.
if (shutdown_)
break;
// Wait for more tasks.
- has_ready_to_run_tasks_cv_.Wait();
+ cv.Wait();
continue;
}
+ RunTaskWithLockAcquired(category);
}
}
@@ -266,9 +288,8 @@ void RasterWorkerPool::ScheduleTasksWithLockAcquired(cc::NamespaceToken token,
work_queue_.ScheduleTasks(token, graph);
- // If there is more work available, wake up the other worker threads.
- if (work_queue_.HasReadyToRunTasks())
- has_ready_to_run_tasks_cv_.Broadcast();
+ // There may be more work available, so wake up another worker thread.
+ SignalTaskReadyConditionVariables();
}
void RasterWorkerPool::WaitForTasksToFinishRunning(cc::NamespaceToken token) {
@@ -288,6 +309,10 @@ void RasterWorkerPool::WaitForTasksToFinishRunning(cc::NamespaceToken token) {
while (!work_queue_.HasFinishedRunningTasksInNamespace(task_namespace))
has_namespaces_with_finished_running_tasks_cv_.Wait();
+
+ // There may be other namespaces that have finished running tasks, so wake
+ // up another origin thread.
+ has_namespaces_with_finished_running_tasks_cv_.Signal();
}
}
@@ -310,19 +335,7 @@ void RasterWorkerPool::CollectCompletedTasksWithLockAcquired(
work_queue_.CollectCompletedTasks(token, completed_tasks);
}
-bool RasterWorkerPool::RunTaskWithLockAcquired(
- const std::vector<cc::TaskCategory>& categories) {
- for (const auto& category : categories) {
- if (work_queue_.HasReadyToRunTasksForCategory(category)) {
- RunTaskInCategoryWithLockAcquired(category);
- return true;
- }
- }
- return false;
-}
-
-void RasterWorkerPool::RunTaskInCategoryWithLockAcquired(
- cc::TaskCategory category) {
+void RasterWorkerPool::RunTaskWithLockAcquired(cc::TaskCategory category) {
TRACE_EVENT0("toplevel", "TaskGraphRunner::RunTask");
lock_.AssertAcquired();
@@ -330,6 +343,12 @@ void RasterWorkerPool::RunTaskInCategoryWithLockAcquired(
auto prioritized_task = work_queue_.GetNextTaskToRun(category);
cc::Task* task = prioritized_task.task;
+ // Create a ScopedTaskCount to increment the task count while running.
+ ScopedTaskCount scoped_task_count = ScopedTaskCountForCategory(category);
+
+ // There may be more work available, so wake up another worker thread.
+ SignalTaskReadyConditionVariables();
+
// Call WillRun() before releasing |lock_| and running task.
task->WillRun();
@@ -344,14 +363,69 @@ void RasterWorkerPool::RunTaskInCategoryWithLockAcquired(
work_queue_.CompleteTask(prioritized_task);
- // We may have just dequeued more tasks, wake up the other worker threads.
- if (work_queue_.HasReadyToRunTasks())
- has_ready_to_run_tasks_cv_.Broadcast();
-
// If namespace has finished running all tasks, wake up origin threads.
if (work_queue_.HasFinishedRunningTasksInNamespace(
prioritized_task.task_namespace))
- has_namespaces_with_finished_running_tasks_cv_.Broadcast();
+ has_namespaces_with_finished_running_tasks_cv_.Signal();
+}
+
+bool RasterWorkerPool::ShouldRunTaskForCategory(cc::TaskCategory category) {
+ // During shutdown we want to flush threads as fast as possible - don't bother
+ // enforcing running task limits.
+ if (!shutdown_) {
+ // Enforce running task limits.
+ int foreground_thread_count = nonconcurrent_foreground_running_task_count_ +
+ foreground_running_task_count_;
+ int total_thread_count =
+ foreground_thread_count + background_running_task_count_;
+ if (total_thread_count >= target_running_task_count_) {
+ return false;
+ }
+
+ if (category == cc::TASK_CATEGORY_BACKGROUND &&
+ foreground_thread_count > 0) {
+ return false;
+ }
+ }
+
+ return work_queue_.HasReadyToRunTasksForCategory(category);
+}
+
+void RasterWorkerPool::SignalTaskReadyConditionVariables() {
+ lock_.AssertAcquired();
+ if (ShouldRunTaskForCategory(cc::TASK_CATEGORY_NONCONCURRENT_FOREGROUND)) {
+ nonconcurrent_foreground_has_ready_to_run_tasks_cv_.Signal();
+ }
+ if (ShouldRunTaskForCategory(cc::TASK_CATEGORY_FOREGROUND)) {
+ foreground_has_ready_to_run_tasks_cv_.Signal();
+ }
+ if (ShouldRunTaskForCategory(cc::TASK_CATEGORY_BACKGROUND)) {
+ background_has_ready_to_run_tasks_cv_.Signal();
+ }
+}
+
+RasterWorkerPool::ScopedTaskCount RasterWorkerPool::ScopedTaskCountForCategory(
+ cc::TaskCategory category) {
+ switch (category) {
+ case cc::TASK_CATEGORY_NONCONCURRENT_FOREGROUND:
+ return ScopedTaskCount(&nonconcurrent_foreground_running_task_count_);
+ case cc::TASK_CATEGORY_FOREGROUND:
+ return ScopedTaskCount(&foreground_running_task_count_);
+ case cc::TASK_CATEGORY_BACKGROUND:
+ return ScopedTaskCount(&background_running_task_count_);
+ }
+}
+
+base::ConditionVariable& RasterWorkerPool::ConditionVariableForCategory(
+ cc::TaskCategory category) {
+ switch (category) {
+ case cc::TASK_CATEGORY_NONCONCURRENT_FOREGROUND:
+ return nonconcurrent_foreground_has_ready_to_run_tasks_cv_;
+ case cc::TASK_CATEGORY_FOREGROUND:
+ return foreground_has_ready_to_run_tasks_cv_;
+ case cc::TASK_CATEGORY_BACKGROUND:
+ return background_has_ready_to_run_tasks_cv_;
+ }
}
RasterWorkerPool::ClosureTask::ClosureTask(const base::Closure& closure)
« content/renderer/raster_worker_pool.h ('K') | « content/renderer/raster_worker_pool.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698