Prioritizing input and compositor tasks in the blink scheduler.

Review URL: https://codereview.chromium.org/439923006

git-svn-id: svn://svn.chromium.org/blink/trunk@180171 bbb929c8-8fbe-4397-9dbb-9b2b20218538

third_party/WebKit/Source/platform/scheduler/Scheduler.cpp

@@ -5,59 +5,93 @@
 #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/MainThread.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();
+        m_idleTask(m_allottedTimeMs);
     }
 
 private:
-    const TraceLocation m_location;
-    Scheduler::Task m_task;
+    Scheduler::IdleTask m_idleTask;
+    double m_allottedTimeMs;
+    TraceLocation m_location;
 };
 
-class MainThreadIdleTaskAdapter : public blink::WebThread::Task {
+} // 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:
-    MainThreadIdleTaskAdapter(const Scheduler::IdleTask& idleTask, double allottedTimeMs)
-        : m_idleTask(idleTask)
-        , m_allottedTimeMs(allottedTimeMs)
+    MainThreadPendingHighPriorityTaskRunner()
     {
+        ASSERT(Scheduler::shared());
     }
 
     // WebThread::Task implementation.
     virtual void run() OVERRIDE
     {
-        TRACE_EVENT1("blink", "MainThreadIdleTaskAdapter::run", "allottedTime", m_allottedTimeMs);
-        m_idleTask(m_allottedTimeMs);
+        Scheduler* scheduler = Scheduler::shared();
+        // FIXME: This check should't be necessary, tasks should not outlive blink.
+        ASSERT(scheduler);
+        if (!scheduler)
+            return;
+        scheduler->runHighPriorityTasks();
     }
-
-private:
-    Scheduler::IdleTask m_idleTask;
-    double m_allottedTimeMs;
 };
 
-}
+
+// 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();
+        // FIXME: This check should't be necessary, tasks should not outlive blink.
+        ASSERT(scheduler);
+        if (scheduler)
+            Scheduler::shared()->runHighPriorityTasks();
+        m_task.run();
+    }
+
+    Scheduler::TracedTask m_task;
+};
 
 Scheduler* Scheduler::s_sharedScheduler = nullptr;
 
@@ -78,50 +112,89 @@ Scheduler* Scheduler::shared()
 }
 
 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()
+{
+    ASSERT(isMainThread());
+    TRACE_EVENT0("blink", "Scheduler::runHighPriorityTasks");
+
+    // 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>& inputTasks = m_pendingInputTasks.swapBuffers();
+    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()
@@ -145,9 +218,27 @@ void Scheduler::stopSharedTimer()
     blink::Platform::current()->stopSharedTimer();
 }
 
-bool Scheduler::shouldYieldForHighPriorityWork()
+bool Scheduler::shouldYieldForHighPriorityWork() const
+{
+    return hasPendingHighPriorityWork();
+}
+
+bool Scheduler::hasPendingHighPriorityWork() const
+{
+    // This method is expected to be run on the main thread, but the high priority tasks will be posted by
+    // other threads. We could use locks here, but this function is (sometimes) called a lot by
+    // ThreadTimers::sharedTimerFiredInternal so we decided to use atomics + barrier loads here which
+    // should be cheaper.
+    // NOTE it's possible the barrier read is overkill here, since delayed yielding isn't a big deal.
+    return acquireLoad(&m_highPriorityTaskCount) != 0;
+}
+
+void Scheduler::TracedTask::run()
 {
-    return false;
+    TRACE_EVENT2("blink", "TracedTask::run",
+        "src_file", m_location.fileName(),
+        "src_func", m_location.functionName());
+    m_task();
 }
 
 } // namespace blink

third_party/WebKit/Source/platform/scheduler/Scheduler.h

@@ -6,8 +6,11 @@
 #define Scheduler_h
 
 #include "platform/PlatformExport.h"
+#include "platform/TraceLocation.h"
+#include "wtf/DoubleBufferedDeque.h"
 #include "wtf/Functional.h"
 #include "wtf/Noncopyable.h"
+#include "wtf/ThreadingPrimitives.h"
 
 namespace blink {
 class WebThread;
@@ -15,8 +18,6 @@ class WebThread;
 
 namespace blink {
 
-class TraceLocation;
-
 // The scheduler is an opinionated gateway for arranging work to be run on the
 // main thread. It decides which tasks get priority over others based on a
 // scheduling policy and the overall system state.
@@ -36,12 +37,12 @@ public:
     void postInputTask(const TraceLocation&, const Task&);
     void postCompositorTask(const TraceLocation&, const Task&);
     void postTask(const TraceLocation&, const Task&); // For generic (low priority) tasks.
-    void postIdleTask(const IdleTask&); // For non-critical tasks which may be reordered relative to other task types.
+    void postIdleTask(const TraceLocation&, const IdleTask&); // For non-critical tasks which may be reordered relative to other task types.
 
     // Returns true if there is high priority work pending on the main thread
     // and the caller should yield to let the scheduler service that work.
-    // Can be called on the main thread.
-    bool shouldYieldForHighPriorityWork();
+    // Can be called on any thread.
+    bool shouldYieldForHighPriorityWork() const;
 
     // The shared timer can be used to schedule a periodic callback which may
     // get preempted by higher priority work.
@@ -50,19 +51,49 @@ public:
     void stopSharedTimer();
 
 private:
+    class MainThreadPendingTaskRunner;
+    class MainThreadPendingHighPriorityTaskRunner;
+    friend class MainThreadPendingTaskRunner;
+    friend class MainThreadPendingHighPriorityTaskRunner;
+
     Scheduler();
     ~Scheduler();
 
-    void scheduleTask(const TraceLocation&, const Task&);
-    void scheduleIdleTask(const IdleTask&);
+    void scheduleIdleTask(const TraceLocation&, const IdleTask&);
 
     static void sharedTimerAdapter();
     void tickSharedTimer();
 
+    bool hasPendingHighPriorityWork() const;
+    void runHighPriorityTasks();
+
     static Scheduler* s_sharedScheduler;
-    blink::WebThread* m_mainThread;
 
+    class TracedTask {
+    public:
+        TracedTask(const Task& task, const TraceLocation& location)
+            : m_task(task)
+            , m_location(location) { }
+
+        void run();
+
+    private:
+        Task m_task;
+        TraceLocation m_location;
+    };
+
+    // Should only be accessed from the main thread.
     void (*m_sharedTimerFunction)();
+
+    // These members can be accessed from any thread.
+    WebThread* m_mainThread;
+
+    // This mutex protects calls to the pending task queues.
+    Mutex m_pendingTasksMutex;
+    DoubleBufferedDeque<TracedTask> m_pendingInputTasks;
+    DoubleBufferedDeque<TracedTask> m_pendingCompositorTasks;
+
+    volatile int m_highPriorityTaskCount;
 };
 
 } // namespace blink

third_party/WebKit/Source/platform/scheduler/SchedulerTest.cpp

@@ -10,9 +10,13 @@
 #include "public/platform/Platform.h"
 #include "public/platform/WebThread.h"
 
+#include <gmock/gmock.h>
 #include <gtest/gtest.h>
+#include <string>
+#include <vector>
 
 using blink::Scheduler;
+using namespace std;
 
 namespace {
 
@@ -116,6 +120,8 @@ private:
 class SchedulerTest : public testing::Test {
 public:
     SchedulerTest()
+        : m_reentrantCount(0)
+        , m_maxRecursion(4)
     {
         Scheduler::initializeOnMainThread();
         m_scheduler = Scheduler::shared();
@@ -131,9 +137,45 @@ public:
         m_platformSupport.runPendingTasks();
     }
 
+    void appendToVector(string value)
+    {
+        m_order.push_back(value);
+    }
+
+    void appendToVectorReentrantTask()
+    {
+        m_reentrantOrder.push_back(m_reentrantCount++);
+
+        if (m_reentrantCount > m_maxRecursion)
+            return;
+        Scheduler::shared()->postTask(FROM_HERE, WTF::bind(&SchedulerTest::appendToVectorReentrantTask, this));
+    }
+
+    void appendToVectorReentrantInputTask()
+    {
+        m_reentrantOrder.push_back(m_reentrantCount++);
+
+        if (m_reentrantCount > m_maxRecursion)
+            return;
+        m_scheduler->postInputTask(FROM_HERE, WTF::bind(&SchedulerTest::appendToVectorReentrantInputTask, this));
+    }
+
+    void appendToVectorReentrantCompositorTask()
+    {
+        m_reentrantOrder.push_back(m_reentrantCount++);
+
+        if (m_reentrantCount > m_maxRecursion)
+            return;
+        m_scheduler->postCompositorTask(FROM_HERE, WTF::bind(&SchedulerTest::appendToVectorReentrantCompositorTask, this));
+    }
+
 protected:
     SchedulerTestingPlatformSupport m_platformSupport;
     Scheduler* m_scheduler;
+    std::vector<string> m_order;
+    std::vector<int> m_reentrantOrder;
+    int m_reentrantCount;
+    int m_maxRecursion;
 };
 
 void orderedTestTask(int value, int* result)
@@ -154,10 +196,10 @@ void idleTestTask(int value, int* result, double allottedTime)
 TEST_F(SchedulerTest, TestPostTask)
 {
     int result = 0;
-    m_scheduler->postTask(FROM_HERE, bind(&orderedTestTask, 1, &result));
-    m_scheduler->postTask(FROM_HERE, bind(&orderedTestTask, 2, &result));
-    m_scheduler->postTask(FROM_HERE, bind(&orderedTestTask, 3, &result));
-    m_scheduler->postTask(FROM_HERE, bind(&orderedTestTask, 4, &result));
+    m_scheduler->postTask(FROM_HERE, WTF::bind(&orderedTestTask, 1, &result));
+    m_scheduler->postTask(FROM_HERE, WTF::bind(&orderedTestTask, 2, &result));
+    m_scheduler->postTask(FROM_HERE, WTF::bind(&orderedTestTask, 3, &result));
+    m_scheduler->postTask(FROM_HERE, WTF::bind(&orderedTestTask, 4, &result));
     runPendingTasks();
     EXPECT_EQ(0x1234, result);
 }
@@ -165,10 +207,10 @@ TEST_F(SchedulerTest, TestPostTask)
 TEST_F(SchedulerTest, TestPostMixedTaskTypes)
 {
     int result = 0;
-    m_scheduler->postTask(FROM_HERE, bind(&unorderedTestTask, 1, &result));
-    m_scheduler->postInputTask(FROM_HERE, bind(&unorderedTestTask, 2, &result));
-    m_scheduler->postCompositorTask(FROM_HERE, bind(&unorderedTestTask, 4, &result));
-    m_scheduler->postTask(FROM_HERE, bind(&unorderedTestTask, 8, &result));
+    m_scheduler->postTask(FROM_HERE, WTF::bind(&unorderedTestTask, 1, &result));
+    m_scheduler->postInputTask(FROM_HERE, WTF::bind(&unorderedTestTask, 2, &result));
+    m_scheduler->postCompositorTask(FROM_HERE, WTF::bind(&unorderedTestTask, 4, &result));
+    m_scheduler->postTask(FROM_HERE, WTF::bind(&unorderedTestTask, 8, &result));
     runPendingTasks();
     EXPECT_EQ(15, result);
 }
@@ -201,12 +243,51 @@ TEST_F(SchedulerTest, TestIdleTask)
 {
     // TODO: Check task allottedTime when implemented in the scheduler.
     int result = 0;
-    m_scheduler->postIdleTask(bind<double>(&idleTestTask, 1, &result));
-    m_scheduler->postIdleTask(bind<double>(&idleTestTask, 1, &result));
-    m_scheduler->postIdleTask(bind<double>(&idleTestTask, 1, &result));
-    m_scheduler->postIdleTask(bind<double>(&idleTestTask, 1, &result));
+    m_scheduler->postIdleTask(FROM_HERE, WTF::bind<double>(&idleTestTask, 1, &result));
+    m_scheduler->postIdleTask(FROM_HERE, WTF::bind<double>(&idleTestTask, 1, &result));
+    m_scheduler->postIdleTask(FROM_HERE, WTF::bind<double>(&idleTestTask, 1, &result));
+    m_scheduler->postIdleTask(FROM_HERE, WTF::bind<double>(&idleTestTask, 1, &result));
     runPendingTasks();
     EXPECT_EQ(4, result);
 }
 
+TEST_F(SchedulerTest, TestTaskPrioritization)
+{
+    m_scheduler->postTask(FROM_HERE, WTF::bind(&SchedulerTest::appendToVector, this, string("L1")));
+    m_scheduler->postTask(FROM_HERE, WTF::bind(&SchedulerTest::appendToVector, this, string("L2")));
+    m_scheduler->postInputTask(FROM_HERE, WTF::bind(&SchedulerTest::appendToVector, this, string("I1")));
+    m_scheduler->postInputTask(FROM_HERE, WTF::bind(&SchedulerTest::appendToVector, this, string("I2")));
+    m_scheduler->postCompositorTask(FROM_HERE, WTF::bind(&SchedulerTest::appendToVector, this, string("C1")));
+    m_scheduler->postCompositorTask(FROM_HERE, WTF::bind(&SchedulerTest::appendToVector, this, string("C2")));
+
+    runPendingTasks();
+    EXPECT_THAT(m_order, testing::ElementsAre(
+        string("I1"), string("I2"), string("C1"), string("C2"), string("L1"), string("L2")));
+}
+
+TEST_F(SchedulerTest, TestRentrantTask)
+{
+    m_scheduler->postTask(FROM_HERE, WTF::bind(&SchedulerTest::appendToVectorReentrantTask, this));
+    runPendingTasks();
+
+    EXPECT_THAT(m_reentrantOrder, testing::ElementsAre(0, 1, 2, 3, 4));
+}
+
+
+TEST_F(SchedulerTest, TestRentrantInputTaskDuringShutdown)
+{
+    m_scheduler->postInputTask(FROM_HERE, WTF::bind(&SchedulerTest::appendToVectorReentrantInputTask, this));
+    Scheduler::shutdown();
+
+    EXPECT_THAT(m_reentrantOrder, testing::ElementsAre(0, 1, 2, 3, 4));
+}
+
+TEST_F(SchedulerTest, TestRentrantCompositorTaskDuringShutdown)
+{
+    m_scheduler->postCompositorTask(FROM_HERE, WTF::bind(&SchedulerTest::appendToVectorReentrantCompositorTask, this));
+    Scheduler::shutdown();
+
+    EXPECT_THAT(m_reentrantOrder, testing::ElementsAre(0, 1, 2, 3, 4));
+}
+
 } // namespace

third_party/WebKit/Source/wtf/DoubleBufferedDeque.h

+// 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.
+
+#ifndef DoubleBufferedDeque_h
+#define DoubleBufferedDeque_h
+
+#include "wtf/Deque.h"
+#include "wtf/Noncopyable.h"
+
+namespace WTF {
+
+// A helper class for managing double buffered deques, typically where the client locks when appending or swapping.
+template <typename T> class DoubleBufferedDeque {
+    WTF_MAKE_NONCOPYABLE(DoubleBufferedDeque);
+public:
+    DoubleBufferedDeque()
+        : m_activeIndex(0) { }
+
+    void append(const T& value)
+    {
+        m_queue[m_activeIndex].append(value);
+    }
+
+    bool isEmpty() const
+    {
+        return m_queue[m_activeIndex].isEmpty();
+    }
+
+    Deque<T>& swapBuffers()
+    {
+        int oldIndex = m_activeIndex;
+        m_activeIndex ^= 1;
+        ASSERT(m_queue[m_activeIndex].isEmpty());
+        return m_queue[oldIndex];
+    }
+
+private:
+    Deque<T> m_queue[2];
+    int m_activeIndex;
+};
+
+} // namespace WTF
+
+using WTF::DoubleBufferedDeque;
+
+#endif // DoubleBufferedDeque_h

third_party/WebKit/Source/wtf/DoubleBufferedDequeTest.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 "wtf/DoubleBufferedDeque.h"
+
+#include <gtest/gtest.h>
+
+namespace {
+
+typedef testing::Test DoubleBufferedDequeTest;
+
+TEST(DoubleBufferedDequeTest, TestIsEmpty)
+{
+    DoubleBufferedDeque<int> queue;
+
+    EXPECT_TRUE(queue.isEmpty());
+    queue.append(1);
+    EXPECT_FALSE(queue.isEmpty());
+}
+
+TEST(DoubleBufferedDequeTest, TestIsEmptyAfterSwapBuffers)
+{
+    DoubleBufferedDeque<int> queue;
+    queue.append(1);
+
+    queue.swapBuffers();
+    EXPECT_TRUE(queue.isEmpty());
+}
+
+TEST(DoubleBufferedDequeTest, TestDoubleBuffering)
+{
+    DoubleBufferedDeque<int> queue;
+    queue.append(1);
+    queue.append(10);
+    queue.append(100);
+
+    {
+        Deque<int>& deque = queue.swapBuffers();
+        EXPECT_EQ(1, deque.takeFirst());
+        EXPECT_EQ(10, deque.takeFirst());
+        EXPECT_EQ(100, deque.takeFirst());
+    }
+    queue.append(2);
+
+    EXPECT_EQ(2, queue.swapBuffers().takeFirst());
+    queue.append(3);
+
+    EXPECT_EQ(3, queue.swapBuffers().takeFirst());
+}
+
+} // namespace

third_party/WebKit/Source/wtf/wtf.gypi

@@ -42,6 +42,7 @@
             'DefaultAllocator.cpp',
             'DefaultAllocator.h',
             'Deque.h',
+            'DoubleBufferedDeque.h',
             'DoublyLinkedList.h',
             'DynamicAnnotations.cpp',
             'DynamicAnnotations.h',
@@ -211,6 +212,7 @@
             'ArrayBufferBuilderTest.cpp',
             'CheckedArithmeticTest.cpp',
             'DequeTest.cpp',
+            'DoubleBufferedDequeTest.cpp',
             'FunctionalTest.cpp',
             'HashMapTest.cpp',
             'HashSetTest.cpp',