Shared Raster Worker Threads

cc/resources/worker_pool_unittest.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/worker_pool.h"
 
 #include <vector>
 
+#include "base/memory/scoped_vector.h"
 #include "cc/base/completion_event.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace cc {
 
 namespace {
+  const int kWorkerPoolCount = 3;

[reveman, 2014/01/13 18:37:21]

nit: blank line before and no indent for this line. 

[sohanjg, 2014/01/14 06:21:26]

Done.

 
 class FakeWorkerPoolTaskImpl : public internal::WorkerPoolTask {
  public:
   FakeWorkerPoolTaskImpl(const base::Closure& callback,
                          const base::Closure& reply)
       : callback_(callback),
         reply_(reply) {
   }
 
   // Overridden from internal::WorkerPoolTask:
@@ skipping 28 matching lines @@
                               dependent_count(dependent_count),
                               priority(priority) {
     }
 
     base::Closure callback;
     base::Closure reply;
     base::Closure dependent;
     unsigned dependent_count;
     unsigned priority;
   };
-  FakeWorkerPool() : WorkerPool(1, "test") {}
+  FakeWorkerPool() : WorkerPool() {}
   virtual ~FakeWorkerPool() {}
 
   static scoped_ptr<FakeWorkerPool> Create() {
     return make_scoped_ptr(new FakeWorkerPool);
   }
 
   void ScheduleTasks(const std::vector<Task>& tasks) {
     TaskVector new_tasks;
     TaskVector new_dependents;
     TaskGraph new_graph;
@@ skipping 63 matching lines @@
   DISALLOW_COPY_AND_ASSIGN(FakeWorkerPool);
 };
 
 class WorkerPoolTest : public testing::Test {
  public:
   WorkerPoolTest() {}
   virtual ~WorkerPoolTest() {}
 
   // Overridden from testing::Test:
   virtual void SetUp() OVERRIDE {
-    worker_pool_ = FakeWorkerPool::Create();
+    for (int i = 0; i < kWorkerPoolCount; i++)

[reveman, 2014/01/13 18:37:21]

nit: ++i instead for consistency

+      worker_pools_.push_back(FakeWorkerPool::Create().release());
   }
   virtual void TearDown() OVERRIDE {
-    worker_pool_->Shutdown();
-    worker_pool_->CheckForCompletedTasks();
+    for (ScopedVector<FakeWorkerPool>::iterator i =
+         worker_pools_.begin(); i != worker_pools_.end(); ++i) {

[reveman, 2014/01/13 18:37:21]

There are some style problems here but maybe better to just do "for (int i = 0;
i < kWorkerPoolCount; ++i)" here instead?

[sohanjg, 2014/01/14 06:21:26]

Done.

+      (*i)->Shutdown();
+      (*i)->CheckForCompletedTasks();
+    }
   }
 
   void ResetIds() {
-    run_task_ids_.clear();
-    on_task_completed_ids_.clear();
+    for (int i = 0; i < kWorkerPoolCount; i++) {

[reveman, 2014/01/13 18:37:21]

nit: ++i instead

[sohanjg, 2014/01/14 06:21:26]

Done.

+      run_task_ids_[i].clear();
+      on_task_completed_ids_[i].clear();
+    }
   }
 
-  void RunAllTasks() {
-    worker_pool_->WaitForTasksToComplete();
-    worker_pool_->CheckForCompletedTasks();
+  void RunAllTasksforWorkerPool(int worker_pool_index) {
+    worker_pools_[worker_pool_index]->WaitForTasksToComplete();
+    worker_pools_[worker_pool_index]->CheckForCompletedTasks();
   }
 
-  FakeWorkerPool* worker_pool() {
-    return worker_pool_.get();
+  FakeWorkerPool* worker_pool(int worker_pool_index) {
+    return worker_pools_[worker_pool_index];
   }
 
-  void RunTask(unsigned id) {
-    run_task_ids_.push_back(id);
+  void RunTask(int worker_pool_index, unsigned id) {
+    run_task_ids_[worker_pool_index].push_back(id);
   }
 
-  void OnTaskCompleted(unsigned id) {
-    on_task_completed_ids_.push_back(id);
+  void OnTaskCompleted(int worker_pool_index, unsigned id) {
+    on_task_completed_ids_[worker_pool_index].push_back(id);
   }
 
-  const std::vector<unsigned>& run_task_ids() {
-    return run_task_ids_;
+  const std::vector<unsigned>& run_task_ids(int worker_pool_index) {
+    return run_task_ids_[worker_pool_index];
   }
 
-  const std::vector<unsigned>& on_task_completed_ids() {
-    return on_task_completed_ids_;
+  const std::vector<unsigned>& on_task_completed_ids(int worker_pool_index) {
+    return on_task_completed_ids_[worker_pool_index];
   }
 
  private:
-  scoped_ptr<FakeWorkerPool> worker_pool_;
-  std::vector<unsigned> run_task_ids_;
-  std::vector<unsigned> on_task_completed_ids_;
+  ScopedVector<FakeWorkerPool> worker_pools_;
+  std::vector<unsigned> run_task_ids_[kWorkerPoolCount];
+  std::vector<unsigned> on_task_completed_ids_[kWorkerPoolCount];
 };
 
 TEST_F(WorkerPoolTest, Basic) {

[reveman, 2014/01/13 18:37:21]

Please avoid changing the logic and use a number of "for (int i = 0; i <
kWorkerPoolCount; ++i)" loops here instead so that all that is needed to test
with more worker pool instances is to change kWorkerPoolCount.

[sohanjg, 2014/01/14 06:21:26]

Done.

-  EXPECT_EQ(0u, run_task_ids().size());
-  EXPECT_EQ(0u, on_task_completed_ids().size());
+  EXPECT_EQ(0u, run_task_ids(0).size());
+  EXPECT_EQ(0u, on_task_completed_ids(0).size());
+  EXPECT_EQ(0u, run_task_ids(1).size());
+  EXPECT_EQ(0u, on_task_completed_ids(1).size());
+  EXPECT_EQ(0u, run_task_ids(2).size());
+  EXPECT_EQ(0u, on_task_completed_ids(2).size());
 
-  worker_pool()->ScheduleTasks(
+  worker_pool(0)->ScheduleTasks(
       std::vector<FakeWorkerPool::Task>(
           1,
           FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
                                           base::Unretained(this),
+                                          0,
                                           0u),
                                base::Bind(&WorkerPoolTest::OnTaskCompleted,
                                           base::Unretained(this),
+                                          0,
                                           0u),
                                base::Closure(),
                                1u,
                                0u)));
-  RunAllTasks();
 
-  EXPECT_EQ(1u, run_task_ids().size());
-  EXPECT_EQ(1u, on_task_completed_ids().size());
-
-  worker_pool()->ScheduleTasks(
+  worker_pool(1)->ScheduleTasks(
       std::vector<FakeWorkerPool::Task>(
           1,
           FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
                                           base::Unretained(this),
+                                          1,
                                           0u),
                                base::Bind(&WorkerPoolTest::OnTaskCompleted,
                                           base::Unretained(this),
+                                          1,
                                           0u),
                                base::Bind(&WorkerPoolTest::RunTask,
                                           base::Unretained(this),
+                                          1,
                                           0u),
                                1u,
                                0u)));
-  RunAllTasks();
 
-  EXPECT_EQ(3u, run_task_ids().size());
-  EXPECT_EQ(2u, on_task_completed_ids().size());
-
-  worker_pool()->ScheduleTasks(
+  worker_pool(2)->ScheduleTasks(
       std::vector<FakeWorkerPool::Task>(
           1, FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
                                              base::Unretained(this),
+                                             2,
                                              0u),
                                   base::Bind(&WorkerPoolTest::OnTaskCompleted,
                                              base::Unretained(this),
+                                             2,
                                              0u),
                                   base::Bind(&WorkerPoolTest::RunTask,
                                              base::Unretained(this),
+                                             2,
                                              0u),
                                   2u,
                                   0u)));
-  RunAllTasks();
+  RunAllTasksforWorkerPool(0);
+  RunAllTasksforWorkerPool(1);
+  RunAllTasksforWorkerPool(2);
 
-  EXPECT_EQ(6u, run_task_ids().size());
-  EXPECT_EQ(3u, on_task_completed_ids().size());
+  EXPECT_EQ(1u, run_task_ids(0).size());
+  EXPECT_EQ(1u, on_task_completed_ids(0).size());
+  EXPECT_EQ(2u, run_task_ids(1).size());
+  EXPECT_EQ(1u, on_task_completed_ids(1).size());
+  EXPECT_EQ(3u, run_task_ids(2).size());
+  EXPECT_EQ(1u, on_task_completed_ids(2).size());
 }
 
 TEST_F(WorkerPoolTest, Dependencies) {

[reveman, 2014/01/13 18:37:21]

Same thing with this test. Use "for" loops and don't change the logic of the
test.

[sohanjg, 2014/01/14 06:21:26]

Done.

-  worker_pool()->ScheduleTasks(
+  worker_pool(0)->ScheduleTasks(
       std::vector<FakeWorkerPool::Task>(
           1, FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
                                              base::Unretained(this),
+                                             0,
                                              0u),
                                   base::Bind(&WorkerPoolTest::OnTaskCompleted,
                                              base::Unretained(this),
+                                             0,
                                              0u),
                                   base::Bind(&WorkerPoolTest::RunTask,
                                              base::Unretained(this),
+                                             0,
                                              1u),
                                   1u,
                                   0u)));
-  RunAllTasks();
 
-  // Check if task ran before dependent.
-  ASSERT_EQ(2u, run_task_ids().size());
-  EXPECT_EQ(0u, run_task_ids()[0]);
-  EXPECT_EQ(1u, run_task_ids()[1]);
-  ASSERT_EQ(1u, on_task_completed_ids().size());
-  EXPECT_EQ(0u, on_task_completed_ids()[0]);
-
-  worker_pool()->ScheduleTasks(
+  worker_pool(1)->ScheduleTasks(
       std::vector<FakeWorkerPool::Task>(
           1, FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
                                              base::Unretained(this),
+                                             1,
                                              2u),
                                   base::Bind(&WorkerPoolTest::OnTaskCompleted,
                                              base::Unretained(this),
+                                             1,
                                              2u),
                                   base::Bind(&WorkerPoolTest::RunTask,
                                              base::Unretained(this),
+                                             1,
                                              3u),
                                   2u,
                                   0u)));
-  RunAllTasks();
+  RunAllTasksforWorkerPool(0);
+  RunAllTasksforWorkerPool(1);
+
+  // Check if task ran before dependent.
+  ASSERT_EQ(2u, run_task_ids(0).size());
+  EXPECT_EQ(0u, run_task_ids(0)[0]);
+  EXPECT_EQ(1u, run_task_ids(0)[1]);
+  ASSERT_EQ(1u, on_task_completed_ids(0).size());
+  EXPECT_EQ(0u, on_task_completed_ids(0)[0]);
 
   // Task should only run once.
-  ASSERT_EQ(5u, run_task_ids().size());
-  EXPECT_EQ(2u, run_task_ids()[2]);
-  EXPECT_EQ(3u, run_task_ids()[3]);
-  EXPECT_EQ(3u, run_task_ids()[4]);
-  ASSERT_EQ(2u, on_task_completed_ids().size());
-  EXPECT_EQ(2u, on_task_completed_ids()[1]);
+  ASSERT_EQ(3u, run_task_ids(1).size());
+  EXPECT_EQ(2u, run_task_ids(1)[0]);
+  EXPECT_EQ(3u, run_task_ids(1)[1]);
+  EXPECT_EQ(3u, run_task_ids(1)[2]);
+  ASSERT_EQ(1u, on_task_completed_ids(1).size());
+  EXPECT_EQ(2u, on_task_completed_ids(1)[0]);
 }
 
 TEST_F(WorkerPoolTest, Priority) {

[reveman, 2014/01/13 18:37:21]

And same thing with this test. Use "for" loops and don't change the logic of the
test.

[sohanjg, 2014/01/14 06:21:26]

Done.

   {
     FakeWorkerPool::Task tasks[] = {
         FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
                                         base::Unretained(this),
+                                        0,
                                         0u),
                              base::Bind(&WorkerPoolTest::OnTaskCompleted,
                                         base::Unretained(this),
+                                        0,
                                         0u),
                              base::Bind(&WorkerPoolTest::RunTask,
                                         base::Unretained(this),
+                                        0,
                                         2u),
                              1u,
                              1u),  // Priority 1
         FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
                                         base::Unretained(this),
+                                        0,
                                         1u),
                              base::Bind(&WorkerPoolTest::OnTaskCompleted,
                                         base::Unretained(this),
+                                        0,
                                         1u),
                              base::Bind(&WorkerPoolTest::RunTask,
                                         base::Unretained(this),
+                                        0,
                                         3u),
                              1u,
                              0u)  // Priority 0
     };
-    worker_pool()->ScheduleTasks(
+    worker_pool(0)->ScheduleTasks(
         std::vector<FakeWorkerPool::Task>(tasks, tasks + arraysize(tasks)));
   }
-  RunAllTasks();
-
-  // Check if tasks ran in order of priority.
-  ASSERT_EQ(4u, run_task_ids().size());
-  EXPECT_EQ(1u, run_task_ids()[0]);
-  EXPECT_EQ(3u, run_task_ids()[1]);
-  EXPECT_EQ(0u, run_task_ids()[2]);
-  EXPECT_EQ(2u, run_task_ids()[3]);
-  ASSERT_EQ(2u, on_task_completed_ids().size());
-  EXPECT_EQ(1u, on_task_completed_ids()[0]);
-  EXPECT_EQ(0u, on_task_completed_ids()[1]);
 
   ResetIds();
   {
     std::vector<FakeWorkerPool::Task> tasks;
     tasks.push_back(
         FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
                                         base::Unretained(this),
+                                        1,
                                         0u),
                              base::Bind(&WorkerPoolTest::OnTaskCompleted,
                                         base::Unretained(this),
+                                        1,
                                         0u),
                              base::Bind(&WorkerPoolTest::RunTask,
                                         base::Unretained(this),
+                                        1,
                                         3u),
                              1u,    // 1 dependent
                              1u));  // Priority 1
     tasks.push_back(
         FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
                                         base::Unretained(this),
+                                        1,
                                         1u),
                              base::Bind(&WorkerPoolTest::OnTaskCompleted,
                                         base::Unretained(this),
+                                        1,
                                         1u),
                              base::Bind(&WorkerPoolTest::RunTask,
                                         base::Unretained(this),
+                                        1,
                                         4u),
                              2u,    // 2 dependents
                              1u));  // Priority 1
     tasks.push_back(
         FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
                                         base::Unretained(this),
+                                        1,
                                         2u),
                              base::Bind(&WorkerPoolTest::OnTaskCompleted,
                                         base::Unretained(this),
+                                        1,
                                         2u),
                              base::Bind(&WorkerPoolTest::RunTask,
                                         base::Unretained(this),
+                                        1,
                                         5u),
                              1u,    // 1 dependent
                              0u));  // Priority 0
-    worker_pool()->ScheduleTasks(tasks);
+    worker_pool(1)->ScheduleTasks(tasks);
   }
-  RunAllTasks();
+  RunAllTasksforWorkerPool(0);
+  RunAllTasksforWorkerPool(1);
+
+  // Check if tasks ran in order of priority.
+  ASSERT_EQ(4u, run_task_ids(0).size());
+  EXPECT_EQ(1u, run_task_ids(0)[0]);
+  EXPECT_EQ(3u, run_task_ids(0)[1]);
+  EXPECT_EQ(0u, run_task_ids(0)[2]);
+  EXPECT_EQ(2u, run_task_ids(0)[3]);
+  ASSERT_EQ(2u, on_task_completed_ids(0).size());
+  EXPECT_EQ(1u, on_task_completed_ids(0)[0]);
+  EXPECT_EQ(0u, on_task_completed_ids(0)[1]);
 
   // Check if tasks ran in order of priority and that task with more
   // dependents ran first when priority is the same.
-  ASSERT_LE(3u, run_task_ids().size());
-  EXPECT_EQ(2u, run_task_ids()[0]);
-  EXPECT_EQ(5u, run_task_ids()[1]);
-  EXPECT_EQ(1u, run_task_ids()[2]);
-  ASSERT_EQ(3u, on_task_completed_ids().size());
-  EXPECT_EQ(2u, on_task_completed_ids()[0]);
-  EXPECT_EQ(1u, on_task_completed_ids()[1]);
-  EXPECT_EQ(0u, on_task_completed_ids()[2]);
+  ASSERT_LE(3u, run_task_ids(1).size());
+  EXPECT_EQ(2u, run_task_ids(1)[0]);
+  EXPECT_EQ(5u, run_task_ids(1)[1]);
+  EXPECT_EQ(1u, run_task_ids(1)[2]);
+  ASSERT_EQ(3u, on_task_completed_ids(1).size());
+  EXPECT_EQ(2u, on_task_completed_ids(1)[0]);
+  EXPECT_EQ(1u, on_task_completed_ids(1)[1]);
+  EXPECT_EQ(0u, on_task_completed_ids(1)[2]);
 }
 
 }  // namespace
 
 }  // namespace cc