Expose a SequencedTaskRunner that runs tasks on raster threads.

content/renderer/render_thread_impl.cc

Index: content/renderer/render_thread_impl.cc
diff --git a/content/renderer/render_thread_impl.cc b/content/renderer/render_thread_impl.cc
index fc9c738e504f0b1df8be57fb4d7d93ebbfb20844..d0162fa7e80d23be92a53a5c4774857c23ee9f9d 100644
--- a/content/renderer/render_thread_impl.cc
+++ b/content/renderer/render_thread_impl.cc
@@ -266,23 +266,6 @@ class RenderViewZoomer : public RenderViewVisitor {
   DISALLOW_COPY_AND_ASSIGN(RenderViewZoomer);
 };
 
-class CompositorRasterThread : public base::SimpleThread {
- public:
-  CompositorRasterThread(cc::TaskGraphRunner* task_graph_runner,
-                         const std::string& name_prefix,
-                         base::SimpleThread::Options options)
-      : base::SimpleThread(name_prefix, options),
-        task_graph_runner_(task_graph_runner) {}
-
-  // Overridden from base::SimpleThread:
-  void Run() override { task_graph_runner_->Run(); }
-
- private:
-  cc::TaskGraphRunner* task_graph_runner_;
-
-  DISALLOW_COPY_AND_ASSIGN(CompositorRasterThread);
-};
-
 std::string HostToCustomHistogramSuffix(const std::string& host) {
   if (host == "mail.google.com")
     return ".gmail";
@@ -394,6 +377,141 @@ blink::WebGraphicsContext3D::Attributes GetOffscreenAttribs() {
   return attributes;
 }
 
+class RasterWorkerPool : public base::SequencedTaskRunner {
+ public:
+  RasterWorkerPool() : namespace_token_(){};

[reveman, 2015/06/30 04:47:52]

; after function definition?

[Daniele Castagna, 2015/06/30 15:43:49]

Done.

+
+  void Start(int num_raster_threads,
+             const base::SimpleThread::Options& thread_options) {

[reveman, 2015/06/30 04:47:51]

Can we create whatever is possible in ctor and just start the threads when Start
is called? So that in ctor; we create stuff, Start; starts processing tasks,
Shutdown stops processing tasks, dtor destroys everything.

[Daniele Castagna, 2015/06/30 15:43:50]

Done.

+    DCHECK(!task_graph_runner_);
+    task_graph_runner_.reset(new cc::TaskGraphRunner);
+    namespace_token_ = task_graph_runner_->GetNamespaceToken();
+
+    while (raster_threads_.size() < static_cast<size_t>(num_raster_threads)) {
+      scoped_ptr<RasterThread> raster_thread(new RasterThread(
+          task_graph_runner_.get(),
+          base::StringPrintf("CompositorTileWorker%u",
+                             static_cast<unsigned>(raster_threads_.size() + 1))
+              .c_str(),
+          thread_options));
+      raster_thread->Start();
+      raster_threads_.push_back(raster_thread.Pass());
+    }
+  }
+
+  void Shutdown() {
+    // Shutdown raster threads.
+    task_graph_runner_->Shutdown();
+    while (!raster_threads_.empty()) {
+      raster_threads_.back()->Join();
+      raster_threads_.pop_back();
+    }
+  }
+
+  // Overridden from base::TaskRunner:
+  bool PostDelayedTask(const tracked_objects::Location& from_here,
+                       const base::Closure& task,
+                       base::TimeDelta delay) override {
+    DCHECK(task_graph_runner_);
+    return PostNonNestableDelayedTask(from_here, task, delay);
+  }
+
+  // Overridden from base::TaskRunner:
+  bool RunsTasksOnCurrentThread() const override { return true; }
+
+  // Overridden from base::SequencedTaskRunner:
+  bool PostNonNestableDelayedTask(const tracked_objects::Location& from_here,
+                                  const base::Closure& task,
+                                  base::TimeDelta delay) override {
+    base::AutoLock lock(lock_);
+    DCHECK(task_graph_runner_);
+    DCHECK(!raster_threads_.empty());
+
+    // Remove completed tasks.
+    DCHECK(completed_tasks_.empty());
+    task_graph_runner_->CollectCompletedTasks(namespace_token_,
+                                              &completed_tasks_);
+    DCHECK_LE(completed_tasks_.size(), tasks_.size());
+    DCHECK(std::equal(completed_tasks_.begin(), completed_tasks_.end(),
+                      tasks_.begin()));
+    tasks_.erase(tasks_.begin(), tasks_.begin() + completed_tasks_.size());
+    completed_tasks_.clear();
+
+    tasks_.push_back(make_scoped_refptr(new TaskGraphRunnerClosureTask(task)));
+
+    graph_.Reset();
+    for (const auto& task : tasks_) {
+      cc::TaskGraph::Node node(task.get(), 0, graph_.nodes.size());
+      if (graph_.nodes.size()) {
+        graph_.edges.push_back(
+            cc::TaskGraph::Edge(graph_.nodes.back().task, node.task));
+      }
+      graph_.nodes.push_back(node);
+    }
+
+    task_graph_runner_->ScheduleTasks(namespace_token_, &graph_);
+    return true;
+  }
+
+  cc::TaskGraphRunner* GetTaskGraphRunner() { return task_graph_runner_.get(); }
+
+ protected:
+  ~RasterWorkerPool() override{};
+
+ private:
+  // Simple Task for the TaskGraphRunner that wraps a closure.
+  class TaskGraphRunnerClosureTask : public cc::Task {

[reveman, 2015/06/30 04:47:52]

"ClosureTask" sufficient name?

[Daniele Castagna, 2015/06/30 15:43:50]

Done.

+   public:
+    TaskGraphRunnerClosureTask(const base::Closure& closure)
+        : closure_(closure) {}
+    void RunOnWorkerThread() override {
+      closure_.Run();
+      closure_.Reset();
+    };
+
+   protected:
+    ~TaskGraphRunnerClosureTask() override {}
+
+   private:
+    base::Closure closure_;
+
+    DISALLOW_COPY_AND_ASSIGN(TaskGraphRunnerClosureTask);
+  };
+
+  // Simple thread that calls Run on the TaskGraphRunner.
+  class RasterThread : public base::SimpleThread {

[reveman, 2015/06/30 04:47:51]

can we use DelegateSimpleThread instead and have RasterWorkerPool implement Run?

[Daniele Castagna, 2015/06/30 15:43:49]

Done.

It'd be nicer if DelegateSimpleThread took a closure. I think having
RasterWorkerPool implement DelegateSimpleThread::Delegate is misleading, but
convenient.

+   public:
+    RasterThread(cc::TaskGraphRunner* task_graph_runner,
+                 const std::string& name_prefix,
+                 base::SimpleThread::Options options)
+        : base::SimpleThread(name_prefix, options),
+          task_graph_runner_(task_graph_runner) {}
+
+    // Overridden from base::SimpleThread:
+    void Run() override { task_graph_runner_->Run(); }
+
+   private:
+    cc::TaskGraphRunner* task_graph_runner_;
+
+    DISALLOW_COPY_AND_ASSIGN(RasterThread);
+  };
+
+  // The actual threads where work is done.
+  ScopedVector<base::SimpleThread> raster_threads_;
+  scoped_ptr<cc::TaskGraphRunner> task_graph_runner_;

[reveman, 2015/06/30 04:47:52]

Can this just be "cc::TaskGraphRunner task_graph_runner_"?

[Daniele Castagna, 2015/06/30 15:43:50]

Done.

+
+  // Lock for the SequencedTaskRunner implementation state.

[reveman, 2015/06/30 04:47:52]

what is that state? everything below? please make that more clear.

[Daniele Castagna, 2015/06/30 15:43:50]

Done.

+  base::Lock lock_;
+  // List of tasks currently queued up for execution.
+  TaskGraphRunnerClosureTask::Vector tasks_;
+  // Cached vector to avoid allocation when getting the list of complete tasks.
+  TaskGraphRunnerClosureTask::Vector completed_tasks_;
+  // Current graph set for scheduling.

[reveman, 2015/06/30 04:47:52]

not really true, is it? this is used to avoid unnecessary heap allocations,
right?

[Daniele Castagna, 2015/06/30 15:43:49]

This is the last graph scheduled with ::ScheduleTasks, changing the comment
since I didn't notice that the object is modified after that call.

+  cc::TaskGraph graph_;
+  // Namespace where the SequencedTaskRunner tasks run.
+  cc::NamespaceToken namespace_token_;
+};
+
 }  // namespace
 
 // For measuring memory usage after each task. Behind a command line flag.
@@ -647,8 +765,6 @@ void RenderThreadImpl::Init() {
   memory_pressure_listener_.reset(new base::MemoryPressureListener(
       base::Bind(&RenderThreadImpl::OnMemoryPressure, base::Unretained(this))));
 
-  compositor_task_graph_runner_.reset(new cc::TaskGraphRunner);
-
   is_gather_pixel_refs_enabled_ = false;
 
   int num_raster_threads = 0;
@@ -672,17 +788,9 @@ void RenderThreadImpl::Init() {
     thread_options.set_priority(base::ThreadPriority::BACKGROUND);
   }
 #endif
-  while (compositor_raster_threads_.size() <
-         static_cast<size_t>(num_raster_threads)) {
-    scoped_ptr<CompositorRasterThread> raster_thread(new CompositorRasterThread(
-        compositor_task_graph_runner_.get(),
-        base::StringPrintf("CompositorTileWorker%u",
-                           static_cast<unsigned>(
-                               compositor_raster_threads_.size() + 1)).c_str(),
-        thread_options));
-    raster_thread->Start();
-    compositor_raster_threads_.push_back(raster_thread.Pass());
-  }
+
+  raster_worker_pool_ = make_scoped_refptr(new RasterWorkerPool());

[reveman, 2015/06/30 04:47:51]

move this to ctor?

[Daniele Castagna, 2015/06/30 15:43:49]

Done.

+  raster_worker_pool_->Start(num_raster_threads, thread_options);
 
   // TODO(boliu): In single process, browser main loop should set up the
   // discardable memory manager, and should skip this if kSingleProcess.
@@ -761,13 +869,8 @@ void RenderThreadImpl::Shutdown() {
 
   compositor_thread_.reset();
 
-  // Shutdown raster threads.
-  compositor_task_graph_runner_->Shutdown();
-  while (!compositor_raster_threads_.empty()) {
-    compositor_raster_threads_.back()->Join();
-    compositor_raster_threads_.pop_back();
-  }
-  compositor_task_graph_runner_.reset();
+  raster_worker_pool_->Shutdown();
+  raster_worker_pool_ = nullptr;

[reveman, 2015/06/30 04:47:52]

We need Shutdown here but maybe just let the dtor take care of dropping ref now
that this is ref-counted

[Daniele Castagna, 2015/06/30 15:43:50]

Done.

 
   main_input_callback_.Cancel();
   input_handler_manager_.reset();
@@ -1460,7 +1563,7 @@ RenderThreadImpl::CreateExternalBeginFrameSource(int routing_id) {
 }
 
 cc::TaskGraphRunner* RenderThreadImpl::GetTaskGraphRunner() {
-  return compositor_task_graph_runner_.get();
+  return raster_worker_pool_->GetTaskGraphRunner();
 }
 
 bool RenderThreadImpl::IsGatherPixelRefsEnabled() {
@@ -1797,6 +1900,11 @@ RenderThreadImpl::GetMediaThreadTaskRunner() {
   return media_thread_->task_runner();
 }
 
+scoped_refptr<base::SequencedTaskRunner>
+RenderThreadImpl::GetWorkerSequencedTaskRunner() {
+  return raster_worker_pool_;
+}
+
 void RenderThreadImpl::SampleGamepads(blink::WebGamepads* data) {
   blink_platform_impl_->sampleGamepads(*data);
 }