Prioritizing input and compositor tasks in the blink scheduler.

Source/platform/scheduler/Scheduler.cpp

 // Copyright 2014 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 "config.h"
 #include "platform/scheduler/Scheduler.h"
 
+#include "platform/PlatformThreadData.h"
 #include "platform/Task.h"
+#include "platform/ThreadTimers.h"
 #include "platform/TraceEvent.h"
-#include "platform/TraceLocation.h"
 #include "public/platform/Platform.h"
-#include "public/platform/WebThread.h"
+#include "wtf/ThreadingPrimitives.h"
 
 namespace blink {
 
 namespace {
 
-class MainThreadTaskAdapter : public blink::WebThread::Task {
+// Can be created from any thread.
+// Note if the scheduler gets shutdown, this may be run after.
+class MainThreadIdleTaskAdapter : public WebThread::Task {
 public:
-    explicit MainThreadTaskAdapter(const TraceLocation& location, const Scheduler::Task& task)
-        : m_location(location)
-        , m_task(task)
+    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", "MainThreadTaskAdapter::run",
+        TRACE_EVENT2("blink", "MainThreadIdleTaskAdapter::run",
             "src_file", m_location.fileName(),
             "src_func", m_location.functionName());
-        m_task();
-    }
-
-private:
-    const TraceLocation m_location;
-    Scheduler::Task m_task;
-};
-
-class MainThreadIdleTaskAdapter : public blink::WebThread::Task {
-public:
-    MainThreadIdleTaskAdapter(const Scheduler::IdleTask& idleTask, double allottedTimeMs)
-        : m_idleTask(idleTask)
-        , m_allottedTimeMs(allottedTimeMs)
-    {
-    }
-
-    // WebThread::Task implementation.
-    virtual void run() OVERRIDE
-    {
-        TRACE_EVENT1("blink", "MainThreadIdleTaskAdapter::run", "allottedTime", m_allottedTimeMs);
         m_idleTask(m_allottedTimeMs);
     }
 
 private:
     Scheduler::IdleTask m_idleTask;
     double m_allottedTimeMs;
+    TraceLocation m_location;
 };
 
-}
+} // namespace
+
+// Typically only created from compositor or render threads.
+// Note if the scheduler gets shutdown, this may be run after.
+class Scheduler::MainThreadPendingHighPriorityTaskRunner : public WebThread::Task {
+public:
+    MainThreadPendingHighPriorityTaskRunner()
+    {
+        ASSERT(Scheduler::shared());
+    }
+
+    // WebThread::Task implementation.
+    virtual void run() OVERRIDE
+    {
+        Scheduler* scheduler = Scheduler::shared();

[eseidel, 2014/08/12 16:16:01]

We should add a FIXME that this should be removed once we make shutdown
deterministic.

[alexclarke, 2014/08/13 10:08:58]

Done.

+        if (!scheduler)
+            return;
+        scheduler->runHighPriorityTasks();
+    }
+};
+
+
+// Can be created from any thread.
+// Note if the scheduler gets shutdown, this may be run after.
+class Scheduler::MainThreadPendingTaskRunner : public WebThread::Task {
+public:
+    MainThreadPendingTaskRunner(
+        const Scheduler::Task& task, const TraceLocation& location)
+        : m_task(task, location)
+    {
+        ASSERT(Scheduler::shared());
+    }
+
+    // WebThread::Task implementation.
+    virtual void run() OVERRIDE
+    {
+        Scheduler* scheduler = Scheduler::shared();
+        // This will assert if a posted task outlives blink, which is probably a bug.
+        ASSERT(scheduler);
+        if (scheduler)
+            Scheduler::shared()->runHighPriorityTasks();
+        m_task.run();
+    }
+
+    Scheduler::TracedTask m_task;
+};
 
 Scheduler* Scheduler::s_sharedScheduler = nullptr;
 
 void Scheduler::initializeOnMainThread()
 {
     s_sharedScheduler = new Scheduler();
 }
 
 void Scheduler::shutdown()
 {
     delete s_sharedScheduler;
     s_sharedScheduler = nullptr;
 }
 
 Scheduler* Scheduler::shared()
 {
     return s_sharedScheduler;
 }
 
 Scheduler::Scheduler()
-    : m_mainThread(blink::Platform::current()->currentThread())
-    , m_sharedTimerFunction(nullptr)
+    : m_sharedTimerFunction(nullptr)
+    , m_mainThread(blink::Platform::current()->currentThread())
+    , m_highPriorityTaskCount(0)
 {
 }
 
 Scheduler::~Scheduler()
 {
+    while (hasPendingHighPriorityWork()) {
+        runHighPriorityTasks();
+    }
 }
 
-void Scheduler::scheduleTask(const TraceLocation& location, const Task& task)
-{
-    m_mainThread->postTask(new MainThreadTaskAdapter(location, task));
-}
-
-void Scheduler::scheduleIdleTask(const IdleTask& idleTask)
+void Scheduler::scheduleIdleTask(const TraceLocation& location, const IdleTask& idleTask)
 {
     // TODO: send a real allottedTime here.
-    m_mainThread->postTask(new MainThreadIdleTaskAdapter(idleTask, 0));
+    m_mainThread->postTask(new MainThreadIdleTaskAdapter(idleTask, 0, location));
 }
 
 void Scheduler::postTask(const TraceLocation& location, const Task& task)
 {
-    scheduleTask(location, task);
+    m_mainThread->postTask(new MainThreadPendingTaskRunner(task, location));
 }
 
 void Scheduler::postInputTask(const TraceLocation& location, const Task& task)
 {
-    scheduleTask(location, task);
+    Locker<Mutex> lock(m_pendingTasksMutex);
+    m_pendingInputTasks.append(TracedTask(task, location));
+    atomicIncrement(&m_highPriorityTaskCount);
+    m_mainThread->postTask(new MainThreadPendingHighPriorityTaskRunner());
 }
 
 void Scheduler::postCompositorTask(const TraceLocation& location, const Task& task)
 {
-    scheduleTask(location, task);
+    Locker<Mutex> lock(m_pendingTasksMutex);
+    m_pendingCompositorTasks.append(TracedTask(task, location));
+    atomicIncrement(&m_highPriorityTaskCount);
+    m_mainThread->postTask(new MainThreadPendingHighPriorityTaskRunner());
 }
 
-void Scheduler::postIdleTask(const IdleTask& idleTask)
+void Scheduler::postIdleTask(const TraceLocation& location, const IdleTask& idleTask)
 {
-    scheduleIdleTask(idleTask);
+    scheduleIdleTask(location, idleTask);
 }
 
 void Scheduler::tickSharedTimer()
 {
     TRACE_EVENT0("blink", "Scheduler::tickSharedTimer");
+
+    // Run any high priority tasks that are queued up, otherwise the blink timers will yield immediately.
+    runHighPriorityTasks();
     m_sharedTimerFunction();
+
+    // The blink timers may have just yielded, so run any high priority tasks that where queued up
+    // while the blink timers were executing.
+    runHighPriorityTasks();
+}
+
+void Scheduler::runHighPriorityTasks()
+{
+    TRACE_EVENT0("blink", "Scheduler::runHighPriorityTasks");
+
+    m_pendingTasksMutex.lock();
+    Deque<TracedTask>& inputTasks = m_pendingInputTasks.swapBuffers();

[eseidel, 2014/08/12 16:16:01]

Because this is a bit fragile (if someone called this method on another thread,
for instance) I mgith explain what is keepign this safe.

I might also add an ASSERT(isMainThread()) to the top of this method.

[alexclarke, 2014/08/13 10:08:58]

Done.

+    Deque<TracedTask>& compositorTasks = m_pendingCompositorTasks.swapBuffers();
+    m_pendingTasksMutex.unlock();
+
+    int highPriorityTasksExecuted = 0;
+    while (!inputTasks.isEmpty()) {
+        inputTasks.takeFirst().run();
+        highPriorityTasksExecuted++;
+    }
+
+    while (!compositorTasks.isEmpty()) {
+        compositorTasks.takeFirst().run();
+        highPriorityTasksExecuted++;
+    }
+
+    int highPriorityTaskCount = atomicSubtract(&m_highPriorityTaskCount, highPriorityTasksExecuted);
+    ASSERT_UNUSED(highPriorityTaskCount, highPriorityTaskCount >= 0);
 }
 
 void Scheduler::sharedTimerAdapter()
 {
     shared()->tickSharedTimer();
 }
 
 void Scheduler::setSharedTimerFiredFunction(void (*function)())
 {
     m_sharedTimerFunction = function;
     blink::Platform::current()->setSharedTimerFiredFunction(function ? &Scheduler::sharedTimerAdapter : nullptr);
 }
 
 void Scheduler::setSharedTimerFireInterval(double interval)
 {
     blink::Platform::current()->setSharedTimerFireInterval(interval);
 }
 
 void Scheduler::stopSharedTimer()
 {
     blink::Platform::current()->stopSharedTimer();
 }
 
-bool Scheduler::shouldYieldForHighPriorityWork()
+bool Scheduler::shouldYieldForHighPriorityWork() const
 {
-    return false;
+    return hasPendingHighPriorityWork();
+}
+
+bool Scheduler::hasPendingHighPriorityWork() const
+{
+    return acquireLoad(&m_highPriorityTaskCount) != 0;

[eseidel, 2014/08/12 16:16:01]

We should add a (possibly large) comment here to explain why we chose to use
barrier loads here and teh consquences of not.

We believe we don't actually need to do this, that failing to read this bool
corectly would just result in delaying high priority tasks a little, etc.

[alexclarke, 2014/08/13 10:08:58]

Done.

+}
+
+void Scheduler::TracedTask::run()
+{
+    TRACE_EVENT2("blink", "TracedTask::run",
+        "src_file", m_location.fileName(),
+        "src_func", m_location.functionName());
+    m_task();
 }
 
 } // namespace blink