compositor-worker: Initialize CW machinery plumbing to compositor and fire CW rAF callbacks.

components/scheduler/child/compositor_worker_scheduler.cc

Index: components/scheduler/child/compositor_worker_scheduler.cc
diff --git a/components/scheduler/child/compositor_worker_scheduler.cc b/components/scheduler/child/compositor_worker_scheduler.cc
index 38a8a4a5129a03c043863e94f4ed5d657c4f0bae..c6e753d1e239b22abbb059d35ea19b781430c076 100644
--- a/components/scheduler/child/compositor_worker_scheduler.cc
+++ b/components/scheduler/child/compositor_worker_scheduler.cc
@@ -123,9 +123,9 @@ scoped_refptr<TaskQueue> CompositorWorkerScheduler::DefaultTaskRunner() {
 
 scoped_refptr<scheduler::SingleThreadIdleTaskRunner>
 CompositorWorkerScheduler::IdleTaskRunner() {
-  // TODO(sad): Not having a task-runner for idle tasks means v8 has to fallback
-  // to inline GC, which might cause jank.
-  return nullptr;
+  return make_scoped_refptr(new SingleThreadIdleTaskRunner(
+      thread_->task_runner(), thread_->task_runner(), this,

[Sami, 2016/05/05 14:56:08]

This is posting idle tasks as regular tasks which is probably not what we want.
For one, that would mean they run at the same priority as regular tasks. I think
we want to add an idle helper here instead.

[flackr, 2016/05/05 15:13:15]

See comment below, this is just an interim workaround to be able to use oilpan
types on the compositor thread (only after a CompositorWorker is created) which
we must fix before launching compositor worker. Would you be comfortable having
a TODO(bug) here to block launching compositor worker on?

[Ian Vollick, 2016/05/05 16:09:18]

Yep, making idle tasks have the same priority as regular tasks on the compositor
thread was intentional.

There's no guarantee that we'll ever be idle and, actually, it seems like
there'd
be a pretty good chance we'd starve idle work if we tried to wait for
quiescence.

I.e., I'm not sure we really want the notion of "idle time" on the compositor
thread
but since some oilpan machinery expects this scheduler to exist, we've
implemented
it with semantics more appropriate for the compositor thread.

There's a good chance I'm misunderstanding something, though, since I know very
little about scheduling. Please let me know if this reasoning is faulty.

flackr@ also pointed out something noteworthy offline:
Even if we did want real idle scheduling, it would likely cause unacceptable
performance regressions. sadrul@ had to revert a previous patch because of this.
See https://codereview.chromium.org/1589463002 for context.

[Sami, 2016/05/05 16:53:39]

A big fat TODO is probably fine as long as this is experimental, but for
actually launching this I think we'd need to have actual idle task support
(which basically involves removing CompositorWorkerTaskRunnerWrapper, adding an
IdleHelper and some plumbing for BeginFrames).

[Sami, 2016/05/05 16:53:39]

That's fine -- idle work is designed to be indefinitely starveable.
 
BeginFrame-based idle scheduling sounds like a good fit for the compositor
worker: we should do gc between a frame being drawn and the next vsync.
Otherwise gc will be randomly triggered whenever user script is running.
This performance regression was caused by a bug somewhere in the scheduling
machinery, i.e., it wasn't a fundamental issue with the architecture. We should
still investigate this, since it's causing a fair amount technical debt in this
file.

[flackr, 2016/05/05 18:36:00]

Added a big fat TODO.

+      "compositor.scheduler"));
 }
 
 bool CompositorWorkerScheduler::CanExceedIdleDeadlineIfRequired() const {
@@ -148,4 +148,13 @@ void CompositorWorkerScheduler::RemoveTaskObserver(
 
 void CompositorWorkerScheduler::Shutdown() {}
 
+void CompositorWorkerScheduler::OnIdleTaskPosted() {}
+
+base::TimeTicks CompositorWorkerScheduler::WillProcessIdleTask() {
+  // TODO(flackr): Return the next frame time as the deadline instead.
+  return base::TimeTicks::Now() + base::TimeDelta::FromMillisecondsD(16.7);

[Sami, 2016/05/05 14:56:08]

This would give all idle tasks 16ms to run, which can easily result in v8
happily chewing through a full frame's worth of time doing gc. We should
probably hold off on adding idle task support until we can provide proper
deadlines and prioritization for these tasks.

[flackr, 2016/05/05 15:13:15]

The idle task runner seems to be necessary in order to use oilpan types. We only
add this fake idle task runner when a CompositorWorker is created (i.e. when you
have turned on the experimental feature and used it). I would consider resolving
the idle task issue blocking before we could launch compositor worker or have
this created normally. Does that help alleviate concerns?

[Sami, 2016/05/05 16:53:39]

One thing rmcilroy@ pointed out was that oilpan doesn't actually do any gc until
it is given a long idle window (~50ms), so I'm not sure how well this will work
in practice.

Another issue is that since we don't have actual idle windows here, v8 might end
up re-posting idle tasks until it runs out of garbage, starving out all other
work in the mean time -- although Ross tells me this might have changed
recently.

So yeah, solving these are definitely blocking issues :)

[flackr, 2016/05/05 18:36:00]

Added a big fat TODO :-)

+}
+
+void CompositorWorkerScheduler::DidProcessIdleTask() {}
+
 }  // namespace scheduler