Implement idle task scheduling.

Source/platform/scheduler/Scheduler.cpp

Index: Source/platform/scheduler/Scheduler.cpp
diff --git a/Source/platform/scheduler/Scheduler.cpp b/Source/platform/scheduler/Scheduler.cpp
index c88c4203a6a7e3c9ad6a36fb3f400e6ea1a7bcc0..b1429663c24d33f25794425a04a474f2f4bd6bd9 100644
--- a/Source/platform/scheduler/Scheduler.cpp
+++ b/Source/platform/scheduler/Scheduler.cpp
@@ -19,32 +19,6 @@ namespace {
 // The time we should stay in CompositorPriority mode for, after a touch event.
 double kLowSchedulerPolicyAfterTouchTimeSeconds = 0.1;
 
-// Can be created from any thread.
-// Note if the scheduler gets shutdown, this may be run after.
-class MainThreadIdleTaskAdapter : public WebThread::Task {
-public:
-    MainThreadIdleTaskAdapter(const Scheduler::IdleTask& idleTask, double allottedTimeMs, const TraceLocation& location)
-        : m_idleTask(idleTask)
-        , m_allottedTimeMs(allottedTimeMs)
-        , m_location(location)
-    {
-    }
-
-    // WebThread::Task implementation.
-    virtual void run() OVERRIDE
-    {
-        TRACE_EVENT2("blink", "MainThreadIdleTaskAdapter::run",
-            "src_file", m_location.fileName(),
-            "src_func", m_location.functionName());
-        m_idleTask(m_allottedTimeMs);
-    }
-
-private:
-    Scheduler::IdleTask m_idleTask;
-    double m_allottedTimeMs;
-    TraceLocation m_location;
-};
-
 } // namespace
 
 // Typically only created from compositor or render threads.
@@ -86,16 +60,45 @@ public:
     virtual void run() OVERRIDE
     {
         Scheduler* scheduler = Scheduler::shared();
-        // FIXME: This check should't be necessary, tasks should not outlive blink.
+        // FIXME: This check shouldn't be necessary, tasks should not outlive blink.
         ASSERT(scheduler);
         if (scheduler)
-            Scheduler::shared()->runPendingHighPriorityTasksIfInCompositorPriority();
+            scheduler->runPendingHighPriorityTasksIfInCompositorPriority();
+
         m_task.run();
     }
 
-    TracedTask m_task;
+private:
+    TracedStandardTask m_task;
 };
 
+
+// Can be created from any thread.
+// Note if the scheduler gets shutdown, this may be run after.
+class Scheduler::MainThreadPendingIdleTaskRunner : public WebThread::Task {
+public:
+    MainThreadPendingIdleTaskRunner()
+    {
+        ASSERT(Scheduler::shared());
+    }
+
+    // WebThread::Task implementation.
+    virtual void run() OVERRIDE
+    {
+        Scheduler* scheduler = Scheduler::shared();
+        // FIXME: This check shouldn't be necessary, tasks should not outlive blink.
+        ASSERT(scheduler);
+        if (scheduler) {
+            scheduler->runPendingHighPriorityTasksIfInCompositorPriority();
+            scheduler->runPendingIdleTaskIfCanRunIdleTask();
+            // If possible, run the next idle task by reposting on the main thread.
+            scheduler->maybePostMainThreadPendingIdleTask();
+        }
+    }
+
+};
+
+
 Scheduler* Scheduler::s_sharedScheduler = nullptr;
 
 void Scheduler::initializeOnMainThread()
@@ -118,6 +121,7 @@ Scheduler::Scheduler()
     : m_sharedTimerFunction(nullptr)
     , m_mainThread(blink::Platform::current()->currentThread())
     , m_compositorPriorityPolicyEndTimeSeconds(0)
+    , m_currentFrameDeadlineSeconds(0)
     , m_highPriorityTaskCount(0)
     , m_highPriorityTaskRunnerPosted(false)
     , m_schedulerPolicy(Normal)
@@ -131,32 +135,35 @@ Scheduler::~Scheduler()
     }
 }
 
-void Scheduler::willBeginFrame(const WebBeginFrameArgs& args)
+void Scheduler::willBeginFrame(double frameDeadlineSeconds)
 {
-    // TODO: Use frame deadline and interval to schedule idle tasks.
+    m_currentFrameCommitted = false;
+    m_currentFrameDeadlineSeconds = frameDeadlineSeconds;
+    // TODO: Schedule a deferred task here to run idle work if didCommitFrameToCompositor never get's called
 }
 
 void Scheduler::didCommitFrameToCompositor()
 {
-    // TODO: Trigger the frame deadline immediately.
-}
-
-void Scheduler::scheduleIdleTask(const TraceLocation& location, const IdleTask& idleTask)
-{
-    // TODO: send a real allottedTime here.
-    m_mainThread->postTask(new MainThreadIdleTaskAdapter(idleTask, 0, location));
+    m_currentFrameCommitted = true;
+    maybePostMainThreadPendingIdleTask();
 }
 
 void Scheduler::postHighPriorityTaskInternal(const TraceLocation& location, const Task& task, const char* traceName)
 {
     Locker<Mutex> lock(m_pendingTasksMutex);
 
-    m_pendingHighPriorityTasks.append(TracedTask(task, location, traceName));
+    m_pendingHighPriorityTasks.append(TracedStandardTask(task, location, traceName));
     atomicIncrement(&m_highPriorityTaskCount);
     maybePostMainThreadPendingHighPriorityTaskRunner();
     TRACE_COUNTER1(TRACE_DISABLED_BY_DEFAULT("blink.scheduler"), "PendingHighPriorityTasks", m_highPriorityTaskCount);
 }
 
+void Scheduler::postIdleTaskInternal(const TraceLocation& location, const IdleTask& idleTask, const char* traceName)
+{
+    Locker<Mutex> lock(m_pendingTasksMutex);
+    m_pendingIdleTasks.append(TracedIdleTask(idleTask, location, traceName));
+}
+
 void Scheduler::postTask(const TraceLocation& location, const Task& task)
 {
     m_mainThread->postTask(new MainThreadPendingTaskRunner(task, location, "Scheduler::MainThreadTask"));
@@ -184,6 +191,11 @@ void Scheduler::postIpcTask(const TraceLocation& location, const Task& task)
     m_mainThread->postTask(new MainThreadPendingTaskRunner(task, location, "Scheduler::IpcTask"));
 }
 
+void Scheduler::postIdleTask(const TraceLocation& location, const IdleTask& idleTask)
+{
+    postIdleTaskInternal(location, idleTask, "Scheduler::IdleTask");
+}
+
 void Scheduler::maybePostMainThreadPendingHighPriorityTaskRunner()
 {
     ASSERT(m_pendingTasksMutex.locked());
@@ -193,9 +205,17 @@ void Scheduler::maybePostMainThreadPendingHighPriorityTaskRunner()
     m_highPriorityTaskRunnerPosted = true;
 }
 
-void Scheduler::postIdleTask(const TraceLocation& location, const IdleTask& idleTask)
+bool Scheduler::maybePostMainThreadPendingIdleTask()
 {
-    scheduleIdleTask(location, idleTask);
+    TRACE_EVENT0("blink", "Scheduler::maybePostMainThreadPendingIdleTask");
+    if (canRunIdleTask()) {

[alexclarke, 2014/10/03 09:59:19]

Which threads will this function get called from?  If it's only the Blink
scheduler thread then this is fine (please add an assert if so), otherwise I'm
not sure this is safe and I suspect it needs to be guarded by the mutex.

Likewise willBeginFrame and didCommitFrameToCompositor should be too. I hope
that doesn't open the possibility of deadlocking.

[Sami, 2014/10/03 10:39:21]

Looks like m_pendingIdleTasks is the only bit of cross-thread data here, right?

[rmcilroy, 2014/10/03 21:41:21]

Right, mpendingIdleTasks is the only bit of cross-thread data here.  I've added
the check that this is only called on one thread.

+        Locker<Mutex> lock(m_pendingTasksMutex);
+        if (!m_pendingIdleTasks.isEmpty()) {
+            m_mainThread->postTask(new MainThreadPendingIdleTaskRunner());
+            return true;
+        }
+    }
+    return false;
 }
 
 void Scheduler::tickSharedTimer()
@@ -221,6 +241,20 @@ bool Scheduler::runPendingHighPriorityTasksIfInCompositorPriority()
     return swapQueuesAndRunPendingTasks();
 }
 
+bool Scheduler::runPendingIdleTaskIfCanRunIdleTask()

[alexclarke, 2014/10/03 09:59:19]

nit: how about maybeRunPendingIdleTask?

[rmcilroy, 2014/10/03 21:41:21]

Done.

+{
+    ASSERT(isMainThread());
+    m_pendingTasksMutex.lock();

[Sami, 2014/10/03 10:39:21]

Is there a way we could rewrite this to use a Locker? For example by having a
takeFirstTask(m_pendingIdleTasks) helper that handles locking?

[rmcilroy, 2014/10/03 21:41:21]

I originally tried to write this to use a locker, however it wasn't really
possible if I keep the isEmpty check since I would need to be able to return a
NULL task (which I can't do unless I allocate on the heap), or pass a task to be
filled in, which seems a bad design.

Saying that, we shouldn't really need to check whether the queue is empty since
this method will only be run if there was a task on the queue already, so I have
rewritten it without this check, wdyt?

+    if (canRunIdleTask() && !m_pendingIdleTasks.isEmpty()) {

[alexclarke, 2014/10/03 09:59:18]

While personally I like it your way, I'm under the impression it's more
consistent with blink style to invert the if like so:

if (!canRunIdleTask() || m_pendingIdleTasks.isEmpty()) {
   m_pendingTasksMutex.unlock();
   return false;
}

[rmcilroy, 2014/10/03 21:41:21]

Done.

+        TracedIdleTask task = m_pendingIdleTasks.takeFirst();
+        m_pendingTasksMutex.unlock();
+        task.run();
+        return true;
+    }
+    m_pendingTasksMutex.unlock();
+    return false;
+}
+
 bool Scheduler::swapQueuesAndRunPendingTasks()
 {
     ASSERT(isMainThread());
@@ -228,7 +262,7 @@ bool Scheduler::swapQueuesAndRunPendingTasks()
     // These locks guard against another thread posting input or compositor tasks while we swap the buffers.
     // One the buffers have been swapped we can safely access the returned deque without having to lock.
     m_pendingTasksMutex.lock();
-    Deque<TracedTask>& highPriorityTasks = m_pendingHighPriorityTasks.swapBuffers();
+    Deque<TracedStandardTask>& highPriorityTasks = m_pendingHighPriorityTasks.swapBuffers();
     maybeEnterNormalSchedulerPolicy();
     m_pendingTasksMutex.unlock();
     return executeHighPriorityTasks(highPriorityTasks);
@@ -241,7 +275,7 @@ void Scheduler::swapQueuesRunPendingTasksAndAllowHighPriorityTaskRunnerPosting()
     // These locks guard against another thread posting input or compositor tasks while we swap the buffers.
     // One the buffers have been swapped we can safely access the returned deque without having to lock.
     m_pendingTasksMutex.lock();
-    Deque<TracedTask>& highPriorityTasks = m_pendingHighPriorityTasks.swapBuffers();
+    Deque<TracedStandardTask>& highPriorityTasks = m_pendingHighPriorityTasks.swapBuffers();
     m_highPriorityTaskRunnerPosted = false;
     maybeEnterNormalSchedulerPolicy();
     m_pendingTasksMutex.unlock();
@@ -258,7 +292,7 @@ void Scheduler::maybeEnterNormalSchedulerPolicy()
         enterSchedulerPolicyLocked(Normal);
 }
 
-bool Scheduler::executeHighPriorityTasks(Deque<TracedTask>& highPriorityTasks)
+bool Scheduler::executeHighPriorityTasks(Deque<TracedStandardTask>& highPriorityTasks)
 {
     TRACE_EVENT0("blink", "Scheduler::executeHighPriorityTasks");
     int highPriorityTasksExecuted = 0;
@@ -313,6 +347,17 @@ bool Scheduler::hasPendingHighPriorityWork() const
     return acquireLoad(&m_highPriorityTaskCount) != 0;
 }
 
+double Scheduler::currentFrameDeadlineSeconds() const
+{
+    // TODO: Make idle time more fine-grain chunks when in Compositor priority.
+    return m_currentFrameDeadlineSeconds;

[Sami, 2014/10/03 10:39:21]

ASSERT(isMainThread());

[rmcilroy, 2014/10/03 21:41:21]

Done.

+}
+
+bool Scheduler::canRunIdleTask() const
+{
+    return m_currentFrameCommitted && (m_currentFrameDeadlineSeconds > Platform::current()->monotonicallyIncreasingTime());

[Sami, 2014/10/03 10:39:21]

ASSERT(isMainThread());

[rmcilroy, 2014/10/03 21:41:21]

Done.

+}
+
 Scheduler::SchedulerPolicy Scheduler::schedulerPolicy() const
 {
     ASSERT(isMainThread());