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 "cc/base/completion_event.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ skipping 42 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 100 matching lines @@
   }
 
   const std::vector<unsigned>& run_task_ids() {
     return run_task_ids_;
   }
 
   const std::vector<unsigned>& on_task_completed_ids() {
     return on_task_completed_ids_;
   }
 
+  unsigned find_run_task_id(unsigned prio, unsigned size) {
+    unsigned id = 9999;
+    for (unsigned i = 0; i < size; ++i) {
+      if (run_task_ids_[i] == prio)
+        id = i;
+    }
+    return id;
+  }

[reveman, 2014/01/10 21:22:03]

not sure why you need this but it looks like you're better of using std::find
instead if necessary.

[sohanjg, 2014/01/13 13:24:51]

Done.

+
  private:
   scoped_ptr<FakeWorkerPool> worker_pool_;

[reveman, 2014/01/10 21:22:03]

why not make this a ScopedVector and add a "const int kWorkerPoolCount = 3"
constant that is used to determine the size? instead of having to manually
create worker pools in each test..

[sohanjg, 2014/01/13 13:24:51]

Done.

   std::vector<unsigned> run_task_ids_;
   std::vector<unsigned> on_task_completed_ids_;

[reveman, 2014/01/10 21:22:03]

and maybe turn these into arrays of vectors. existing functions would need a new
index parameter. ie.

void RunTask(int worker_pool_index, unsigned id) {
  run_task_ids_[worker_pool_index].push_back(id);
}
const std::vector<unsigned>& run_task_ids(int worker_pool_index) {
  return run_task_ids_[worker_pool_index];
}

[sohanjg, 2014/01/13 13:24:51]

Done.

 };
 
 TEST_F(WorkerPoolTest, Basic) {
   EXPECT_EQ(0u, run_task_ids().size());
   EXPECT_EQ(0u, on_task_completed_ids().size());
 
-  worker_pool()->ScheduleTasks(
+  const int pool_size = 3;
+  scoped_ptr<FakeWorkerPool> worker_pools[pool_size];
+
+  for (int i = 0; i < pool_size ; i++)
+    worker_pools[i] = FakeWorkerPool::Create();
+
+  worker_pools[0]->ScheduleTasks(
+        std::vector<FakeWorkerPool::Task>(
+            1,
+            FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
+                                            base::Unretained(this),
+                                            0u),
+                                 base::Bind(&WorkerPoolTest::OnTaskCompleted,
+                                            base::Unretained(this),
+                                            0u),
+                                 base::Closure(),
+                                 1u,
+                                 0u)));
+
+  worker_pools[1]->ScheduleTasks(
       std::vector<FakeWorkerPool::Task>(
           1,
           FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
-                                          base::Unretained(this),
-                                          0u),
-                               base::Bind(&WorkerPoolTest::OnTaskCompleted,
-                                          base::Unretained(this),
-                                          0u),
-                               base::Closure(),
-                               1u,
-                               0u)));
-  RunAllTasks();
-
-  EXPECT_EQ(1u, run_task_ids().size());
-  EXPECT_EQ(1u, on_task_completed_ids().size());
-
-  worker_pool()->ScheduleTasks(
-      std::vector<FakeWorkerPool::Task>(
-          1,
-          FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
                                           base::Unretained(this),
                                           0u),
                                base::Bind(&WorkerPoolTest::OnTaskCompleted,
                                           base::Unretained(this),
                                           0u),
                                base::Bind(&WorkerPoolTest::RunTask,
                                           base::Unretained(this),
                                           0u),
                                1u,
                                0u)));
-  RunAllTasks();
 
-  EXPECT_EQ(3u, run_task_ids().size());
-  EXPECT_EQ(2u, on_task_completed_ids().size());
-
-  worker_pool()->ScheduleTasks(
+  worker_pools[2]->ScheduleTasks(
       std::vector<FakeWorkerPool::Task>(
           1, FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
                                              base::Unretained(this),
                                              0u),
                                   base::Bind(&WorkerPoolTest::OnTaskCompleted,
                                              base::Unretained(this),
                                              0u),
                                   base::Bind(&WorkerPoolTest::RunTask,
                                              base::Unretained(this),
                                              0u),
                                   2u,
                                   0u)));
-  RunAllTasks();
+  for (int i = 0; i < pool_size ; i++) {
+    worker_pools[i]->WaitForTasksToComplete();
+    worker_pools[i]->CheckForCompletedTasks();
+  }
 
   EXPECT_EQ(6u, run_task_ids().size());
   EXPECT_EQ(3u, on_task_completed_ids().size());
 }
 
 TEST_F(WorkerPoolTest, Dependencies) {
-  worker_pool()->ScheduleTasks(
+  const int pool_size = 2;
+  scoped_ptr<FakeWorkerPool> worker_pools[pool_size];
+  for (int i = 0; i < pool_size ; i++)
+    worker_pools[i] = FakeWorkerPool::Create();
+
+  worker_pools[0]->ScheduleTasks(
       std::vector<FakeWorkerPool::Task>(
           1, FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
                                              base::Unretained(this),
                                              0u),
                                   base::Bind(&WorkerPoolTest::OnTaskCompleted,
                                              base::Unretained(this),
                                              0u),
                                   base::Bind(&WorkerPoolTest::RunTask,
                                              base::Unretained(this),
                                              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_pools[1]->ScheduleTasks(
       std::vector<FakeWorkerPool::Task>(
           1, FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
                                              base::Unretained(this),
                                              2u),
                                   base::Bind(&WorkerPoolTest::OnTaskCompleted,
                                              base::Unretained(this),
                                              2u),
                                   base::Bind(&WorkerPoolTest::RunTask,
                                              base::Unretained(this),
                                              3u),
                                   2u,
                                   0u)));
-  RunAllTasks();
+  for (int i = 0; i < pool_size ; i++) {
+    worker_pools[i]->WaitForTasksToComplete();
+    worker_pools[i]->CheckForCompletedTasks();
+  }
 
-  // Task should only run once.
+  // Check if task ran before dependent.
   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]);
+  unsigned id_task = find_run_task_id(0u, run_task_ids().size());
+  unsigned id_dep = find_run_task_id(1u, run_task_ids().size());
+  EXPECT_EQ((id_dep-id_task) > 0, 1);
+  id_task = find_run_task_id(2u, run_task_ids().size());
+  id_dep = find_run_task_id(3u, run_task_ids().size());
+  EXPECT_EQ((id_dep-id_task) > 0, 1);
   ASSERT_EQ(2u, on_task_completed_ids().size());
   EXPECT_EQ(2u, on_task_completed_ids()[1]);
 }
 
 TEST_F(WorkerPoolTest, Priority) {
+  const int pool_size = 4;
+  scoped_ptr<FakeWorkerPool> worker_pools[pool_size];
+  for (int i = 0; i < pool_size ; i++)
+    worker_pools[i] = FakeWorkerPool::Create();
+
   {
-    FakeWorkerPool::Task tasks[] = {
+    FakeWorkerPool::Task tasks1[] = {
         FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
                                         base::Unretained(this),
                                         0u),
                              base::Bind(&WorkerPoolTest::OnTaskCompleted,
                                         base::Unretained(this),
                                         0u),
                              base::Bind(&WorkerPoolTest::RunTask,
                                         base::Unretained(this),
                                         2u),
                              1u,
                              1u),  // Priority 1
         FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
                                         base::Unretained(this),
                                         1u),
                              base::Bind(&WorkerPoolTest::OnTaskCompleted,
                                         base::Unretained(this),
                                         1u),
                              base::Bind(&WorkerPoolTest::RunTask,
                                         base::Unretained(this),
                                         3u),
                              1u,
                              0u)  // Priority 0
     };
-    worker_pool()->ScheduleTasks(
-        std::vector<FakeWorkerPool::Task>(tasks, tasks + arraysize(tasks)));
+    FakeWorkerPool::Task tasks2[] = {
+        FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
+                                        base::Unretained(this),
+                                        0u),
+                             base::Bind(&WorkerPoolTest::OnTaskCompleted,
+                                        base::Unretained(this),
+                                        0u),
+                             base::Bind(&WorkerPoolTest::RunTask,
+                                        base::Unretained(this),
+                                        4u),
+                             1u,
+                             2u),  // Priority 2
+        FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
+                                        base::Unretained(this),
+                                        1u),
+                             base::Bind(&WorkerPoolTest::OnTaskCompleted,
+                                        base::Unretained(this),
+                                        1u),
+                             base::Bind(&WorkerPoolTest::RunTask,
+                                        base::Unretained(this),
+                                        5u),
+                             1u,
+                             3u)  // Priority 3
+    };
+    worker_pools[0]->ScheduleTasks(
+        std::vector<FakeWorkerPool::Task>(tasks1, tasks1 + arraysize(tasks1)));
+    worker_pools[1]->ScheduleTasks(
+        std::vector<FakeWorkerPool::Task>(tasks2, tasks2 + arraysize(tasks2)));
   }
-  RunAllTasks();
+  for (int i = 0; i < 2 ; i++) {
+    worker_pools[i]->WaitForTasksToComplete();
+    worker_pools[i]->CheckForCompletedTasks();
+  }
 
   // Check if tasks ran in order of priority.
-  ASSERT_EQ(4u, run_task_ids().size());
+  ASSERT_EQ(8u, 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(0u, run_task_ids()[4]);
+  EXPECT_EQ(4u, run_task_ids()[5]);
+  EXPECT_EQ(1u, run_task_ids()[6]);
+  EXPECT_EQ(5u, run_task_ids()[7]);
+  ASSERT_EQ(4u, on_task_completed_ids().size());
   EXPECT_EQ(1u, on_task_completed_ids()[0]);
   EXPECT_EQ(0u, on_task_completed_ids()[1]);
+  EXPECT_EQ(0u, on_task_completed_ids()[2]);
+  EXPECT_EQ(1u, on_task_completed_ids()[3]);
 
   ResetIds();
   {
-    std::vector<FakeWorkerPool::Task> tasks;
-    tasks.push_back(
+    std::vector<FakeWorkerPool::Task> tasks1;
+    tasks1.push_back(
         FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
                                         base::Unretained(this),
                                         0u),
                              base::Bind(&WorkerPoolTest::OnTaskCompleted,
                                         base::Unretained(this),
                                         0u),
                              base::Bind(&WorkerPoolTest::RunTask,
                                         base::Unretained(this),
                                         3u),
                              1u,    // 1 dependent
                              1u));  // Priority 1
-    tasks.push_back(
+    tasks1.push_back(
         FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
                                         base::Unretained(this),
                                         1u),
                              base::Bind(&WorkerPoolTest::OnTaskCompleted,
                                         base::Unretained(this),
                                         1u),
                              base::Bind(&WorkerPoolTest::RunTask,
                                         base::Unretained(this),
                                         4u),
                              2u,    // 2 dependents
                              1u));  // Priority 1
-    tasks.push_back(
+    tasks1.push_back(
         FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
                                         base::Unretained(this),
                                         2u),
                              base::Bind(&WorkerPoolTest::OnTaskCompleted,
                                         base::Unretained(this),
                                         2u),
                              base::Bind(&WorkerPoolTest::RunTask,
                                         base::Unretained(this),
                                         5u),
                              1u,    // 1 dependent
                              0u));  // Priority 0
-    worker_pool()->ScheduleTasks(tasks);
+    worker_pools[3]->ScheduleTasks(tasks1);
+
+    std::vector<FakeWorkerPool::Task> tasks2;
+    tasks2.push_back(
+        FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
+                                        base::Unretained(this),
+                                        5u),
+                             base::Bind(&WorkerPoolTest::OnTaskCompleted,
+                                        base::Unretained(this),
+                                        5u),
+                             base::Bind(&WorkerPoolTest::RunTask,
+                                        base::Unretained(this),
+                                        6u),
+                             3u,    // 3 dependent
+                             3u));  // Priority 3
+    tasks2.push_back(
+        FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
+                                        base::Unretained(this),
+                                        4u),
+                             base::Bind(&WorkerPoolTest::OnTaskCompleted,
+                                        base::Unretained(this),
+                                        4u),
+                             base::Bind(&WorkerPoolTest::RunTask,
+                                        base::Unretained(this),
+                                        7u),
+                             3u,    // 3 dependents
+                             2u));  // Priority 2
+    tasks2.push_back(
+        FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
+                                        base::Unretained(this),
+                                        3u),
+                             base::Bind(&WorkerPoolTest::OnTaskCompleted,
+                                        base::Unretained(this),
+                                        3u),
+                             base::Bind(&WorkerPoolTest::RunTask,
+                                        base::Unretained(this),
+                                        8u),
+                             4u,    // 4 dependent
+                             0u));  // Priority 0
+    worker_pools[4]->ScheduleTasks(tasks2);
   }
-  RunAllTasks();
+  for (int i = 2; i < pool_size ; i++) {
+    worker_pools[i]->WaitForTasksToComplete();
+    worker_pools[i]->CheckForCompletedTasks();
+  }
 
   // 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());
+  ASSERT_LE(6u, run_task_ids().size());
+  EXPECT_EQ(3u, run_task_ids()[0]);
+  EXPECT_EQ(8u, run_task_ids()[1]);
+  EXPECT_EQ(2u, run_task_ids()[2]);
+  EXPECT_EQ(5u, run_task_ids()[3]);
+  EXPECT_EQ(8u, run_task_ids()[4]);
+  EXPECT_EQ(8u, run_task_ids()[5]);
+  EXPECT_EQ(8u, run_task_ids()[6]);
+  ASSERT_EQ(6u, 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]);
+  EXPECT_EQ(3u, on_task_completed_ids()[3]);
+  EXPECT_EQ(4u, on_task_completed_ids()[4]);
+  EXPECT_EQ(5u, on_task_completed_ids()[5]);
 }

[reveman, 2014/01/10 21:22:03]

The way you're modifying these tests make it really hard see if the logic is
changing or not. Please try to keep the code and logic as intact as possible.
That makes the changes much easier for me to review.

I think adding a vector of WorkerPool instances to WorkerPoolTest as I suggested
above is a good way to accomplish this.

[sohanjg, 2014/01/13 13:24:51]

Done.

 
 }  // namespace
 
 }  // namespace cc