Prioritizing input and compositor tasks in the blink scheduler.

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 61ae7aef1d486bd618417a2d2d62c14c08db06bc..d7a8d1b437ca4cd876b4f4561a5fe5477c6c501a 100644
--- a/Source/platform/scheduler/Scheduler.cpp
+++ b/Source/platform/scheduler/Scheduler.cpp
@@ -5,38 +5,18 @@
 #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/Atomics.h"
 
 namespace blink {
 
 namespace {
 
-class MainThreadTaskAdapter : public blink::WebThread::Task {
-public:
-    explicit MainThreadTaskAdapter(const TraceLocation& location, const Scheduler::Task& task)
-        : m_location(location)
-        , m_task(task)
-    {
-    }
-
-    // WebThread::Task implementation.
-    virtual void run() OVERRIDE
-    {
-        TRACE_EVENT2("blink", "MainThreadTaskAdapter::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)
@@ -57,6 +37,16 @@ private:
     double m_allottedTimeMs;
 };
 
+class MainThreadPendingTaskRunner : public blink::WebThread::Task {
+public:
+    // WebThread::Task implementation.
+    virtual void run() OVERRIDE
+    {
+        Scheduler::shared()->runPendingTasks();
+        Scheduler::shared()->allowMainThreadTaskPosting();
+    }
+};
+
 }
 
 Scheduler* Scheduler::s_sharedScheduler = nullptr;
@@ -79,17 +69,15 @@ Scheduler* Scheduler::shared()
 
 Scheduler::Scheduler()
     : m_mainThread(blink::Platform::current()->currentThread())
+    , m_mainThreadTaskPosted(0)
+    , m_blinkShouldNotYield(1)
     , m_sharedTimerFunction(nullptr)
 {
 }
 
 Scheduler::~Scheduler()
 {
-}
-
-void Scheduler::scheduleTask(const TraceLocation& location, const Task& task)
-{
-    m_mainThread->postTask(new MainThreadTaskAdapter(location, task));
+    runPendingTasks();
 }
 
 void Scheduler::scheduleIdleTask(const IdleTask& idleTask)
@@ -100,17 +88,24 @@ void Scheduler::scheduleIdleTask(const IdleTask& idleTask)
 
 void Scheduler::postTask(const TraceLocation& location, const Task& task)
 {
-    scheduleTask(location, task);
+    m_pendingBlinkTasks.append(TracedTask(task, location));
+    maybePostTaskLoopOnMainThread();
 }
 
 void Scheduler::postInputTask(const TraceLocation& location, const Task& task)
 {
-    scheduleTask(location, task);
+    m_pendingInputTasks.pushMaySpinlock(new TracedTask(task, location));
+    maybePostTaskLoopOnMainThread();
+
+    setBlinkShouldYield();
 }
 
 void Scheduler::postCompositorTask(const TraceLocation& location, const Task& task)
 {
-    scheduleTask(location, task);
+    m_pendingCompositorTasks.pushMaySpinlock(new TracedTask(task, location));
+    maybePostTaskLoopOnMainThread();
+
+    setBlinkShouldYield();
 }
 
 void Scheduler::postIdleTask(const IdleTask& idleTask)
@@ -121,9 +116,67 @@ void Scheduler::postIdleTask(const IdleTask& idleTask)
 void Scheduler::tickSharedTimer()
 {
     TRACE_EVENT0("blink", "Scheduler::tickSharedTimer");
+
+    // Execute any pending tasks.
+    if (shouldYieldForHighPriorityWork())
+        runPendingTasks();
+
     m_sharedTimerFunction();
 }
 
+void Scheduler::runPendingTasks()
+{
+    TRACE_EVENT0("blink", "Scheduler::runPendingTasks");
+
+    // Execute tasks from the queues in order of priority.
+    for (;;) {
+        TracedTask* task = m_pendingInputTasks.pop();

[Sami, 2014/08/06 10:19:52]

How about making this slightly safer by wrapping the pointer into a scoped
container:

    OwnPtr<TracedTask> task = adoptPtr(m_pendingInputTasks.pop());

[alexclarke, 2014/08/07 12:08:06]

Done.

+        if (task) {
+            task->run();
+            delete task;
+            continue;
+        }
+
+        task = m_pendingCompositorTasks.pop();
+        if (task) {
+            task->run();
+            delete task;
+            continue;
+        }
+
+        if (m_pendingBlinkTasks.isEmpty())
+            break;
+
+        m_pendingBlinkTasks.takeFirst().run();

[Sami, 2014/08/06 10:19:52]

It seems like it's possible for low priority tasks to completely hog the main
thread if new ones just keep getting posted. How about just running one low
priority task per call to runPendingTasks() and if more are pending just call
maybePostTaskLoopOnMainThread()?

[alexclarke, 2014/08/07 12:08:06]

It's also possible for compositor tasks to do something similar.  E.g. If we're
on a very heavy page, it's possible for multiple calls to BeginMainFrame to
buffer up.

Lets discuss this offline.

+    }
+
+    clearBlinkShouldYield();
+}
+
+void Scheduler::maybePostTaskLoopOnMainThread()
+{
+    if (atomicTestAndSetToOne(&m_mainThreadTaskPosted))
+        return;
+
+    m_mainThread->postTask(new MainThreadPendingTaskRunner());
+}
+
+void Scheduler::allowMainThreadTaskPosting()
+{
+    // Allow posting of the main thread task again.
+    atomicSetOneToZero(&m_mainThreadTaskPosted);
+}
+
+void Scheduler::setBlinkShouldYield()
+{
+    atomicSetOneToZero(&m_blinkShouldNotYield);
+}
+
+void Scheduler::clearBlinkShouldYield()
+{
+    atomicTestAndSetToOne(&m_blinkShouldNotYield);
+}
+
 void Scheduler::sharedTimerAdapter()
 {
     shared()->tickSharedTimer();
@@ -147,7 +200,15 @@ void Scheduler::stopSharedTimer()
 
 bool Scheduler::shouldYieldForHighPriorityWork()
 {
-    return false;
+    return !atomicTestAndSetToOne(&m_blinkShouldNotYield);
+}
+
+void Scheduler::TracedTask::run()
+{
+    TRACE_EVENT2("blink", "TracedTask::run",
+        "src_file", m_location.fileName(),
+        "src_func", m_location.functionName());
+    m_task();
 }
 
 } // namespace blink