scheduler: Post high priority tasks directly to the message loop

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..9720600e1b35150f7ad4569d5f8f9bdf4bfc969d 100644
--- a/Source/platform/scheduler/Scheduler.cpp
+++ b/Source/platform/scheduler/Scheduler.cpp
@@ -51,7 +51,9 @@ private:
 // Note if the scheduler gets shutdown, this may be run after.
 class Scheduler::MainThreadPendingHighPriorityTaskRunner : public WebThread::Task {
 public:
-    MainThreadPendingHighPriorityTaskRunner()
+    MainThreadPendingHighPriorityTaskRunner(
+        const Scheduler::Task& task, const TraceLocation& location, const char* traceName)
+        : m_task(task, location, traceName)
     {
         ASSERT(Scheduler::shared());
     }
@@ -59,17 +61,16 @@ public:
     // 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)
-            return;
-        // NOTE we must unconditionally execute high priority tasks here, since if we're not in CompositorPriority
-        // mode, then this is the only place where high priority tasks will be executed.
-        scheduler->swapQueuesRunPendingTasksAndAllowHighPriorityTaskRunnerPosting();
+        m_task.run();
+        if (Scheduler* scheduler = Scheduler::shared()) {
+            scheduler->updatePolicy();
+            scheduler->didRunHighPriorityTask();
+        }
     }
-};
 
+private:
+    TracedTask m_task;
+};
 
 // Can be created from any thread.
 // Note if the scheduler gets shutdown, this may be run after.
@@ -85,12 +86,10 @@ public:
     // 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()->runPendingHighPriorityTasksIfInCompositorPriority();
         m_task.run();
+        if (Scheduler* scheduler = Scheduler::shared()) {
+            scheduler->updatePolicy();
+        }
     }
 
     TracedTask m_task;
@@ -126,9 +125,6 @@ Scheduler::Scheduler()
 
 Scheduler::~Scheduler()
 {
-    while (hasPendingHighPriorityWork()) {
-        swapQueuesAndRunPendingTasks();
-    }
 }
 
 void Scheduler::willBeginFrame(const WebBeginFrameArgs& args)
@@ -149,11 +145,14 @@ void Scheduler::scheduleIdleTask(const TraceLocation& location, const IdleTask&
 
 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_mainThread->postTask(new MainThreadPendingHighPriorityTaskRunner(task, location, traceName));
     atomicIncrement(&m_highPriorityTaskCount);
-    maybePostMainThreadPendingHighPriorityTaskRunner();
+    TRACE_COUNTER1(TRACE_DISABLED_BY_DEFAULT("blink.scheduler"), "PendingHighPriorityTasks", m_highPriorityTaskCount);
+}
+
+void Scheduler::didRunHighPriorityTask()
+{
+    atomicDecrement(&m_highPriorityTaskCount);
     TRACE_COUNTER1(TRACE_DISABLED_BY_DEFAULT("blink.scheduler"), "PendingHighPriorityTasks", m_highPriorityTaskCount);
 }
 
@@ -184,15 +183,6 @@ void Scheduler::postIpcTask(const TraceLocation& location, const Task& task)
     m_mainThread->postTask(new MainThreadPendingTaskRunner(task, location, "Scheduler::IpcTask"));
 }
 
-void Scheduler::maybePostMainThreadPendingHighPriorityTaskRunner()
-{
-    ASSERT(m_pendingTasksMutex.locked());
-    if (m_highPriorityTaskRunnerPosted)
-        return;
-    m_mainThread->postTask(new MainThreadPendingHighPriorityTaskRunner());
-    m_highPriorityTaskRunnerPosted = true;
-}
-
 void Scheduler::postIdleTask(const TraceLocation& location, const IdleTask& idleTask)
 {
     scheduleIdleTask(location, idleTask);
@@ -201,78 +191,19 @@ void Scheduler::postIdleTask(const TraceLocation& location, const IdleTask& idle
 void Scheduler::tickSharedTimer()
 {
     TRACE_EVENT0("blink", "Scheduler::tickSharedTimer");
-
-    // Run any high priority tasks that are queued up, otherwise the blink timers will yield immediately.
-    bool workDone = runPendingHighPriorityTasksIfInCompositorPriority();

[alexclarke, 2014/10/06 14:11:03]

I suspect under some circumstances this patch will partially starve the blink
timers.  If a high priority task is already in the queue, only a single blink
timer will fire before the shared timer function yields and reposts itself.

That maybe OK for an interim solution.

[Sami, 2014/10/06 14:59:18]

We figured out offline that this should be fine because the timer tasks are
never completely starved: we always run at least one every time the shared timer
fires and another shared timer is scheduled when that yields.

     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.
-    if (!workDone)
-        runPendingHighPriorityTasksIfInCompositorPriority();
 }
 
-bool Scheduler::runPendingHighPriorityTasksIfInCompositorPriority()
+void Scheduler::updatePolicy()
 {
     ASSERT(isMainThread());
-    if (schedulerPolicy() != CompositorPriority)
-        return false;
-
-    return swapQueuesAndRunPendingTasks();
-}
-
-bool Scheduler::swapQueuesAndRunPendingTasks()
-{
-    ASSERT(isMainThread());
-
-    // 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();
-    maybeEnterNormalSchedulerPolicy();
-    m_pendingTasksMutex.unlock();
-    return executeHighPriorityTasks(highPriorityTasks);
-}
-
-void Scheduler::swapQueuesRunPendingTasksAndAllowHighPriorityTaskRunnerPosting()
-{
-    ASSERT(isMainThread());
-
-    // 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();
-    m_highPriorityTaskRunnerPosted = false;
-    maybeEnterNormalSchedulerPolicy();
-    m_pendingTasksMutex.unlock();
-    executeHighPriorityTasks(highPriorityTasks);
-}
-
-void Scheduler::maybeEnterNormalSchedulerPolicy()
-{
-    ASSERT(isMainThread());
-    ASSERT(m_pendingTasksMutex.locked());
+    Locker<Mutex> lock(m_pendingTasksMutex);

[alexclarke, 2014/10/06 14:11:03]

I think we can rename this mutex now.

[Sami, 2014/10/06 14:59:18]

Done.

 
     // Go back to the normal scheduler policy if enough time has elapsed.
     if (schedulerPolicy() == CompositorPriority && Platform::current()->monotonicallyIncreasingTime() > m_compositorPriorityPolicyEndTimeSeconds)
         enterSchedulerPolicyLocked(Normal);
 }
 
-bool Scheduler::executeHighPriorityTasks(Deque<TracedTask>& highPriorityTasks)
-{
-    TRACE_EVENT0("blink", "Scheduler::executeHighPriorityTasks");
-    int highPriorityTasksExecuted = 0;
-    while (!highPriorityTasks.isEmpty()) {
-        highPriorityTasks.takeFirst().run();
-        highPriorityTasksExecuted++;
-    }
-
-    int highPriorityTaskCount = atomicSubtract(&m_highPriorityTaskCount, highPriorityTasksExecuted);
-    ASSERT_UNUSED(highPriorityTaskCount, highPriorityTaskCount >= 0);
-    TRACE_COUNTER1(TRACE_DISABLED_BY_DEFAULT("blink.scheduler"), "PendingHighPriorityTasks", m_highPriorityTaskCount);
-    return highPriorityTasksExecuted > 0;
-}
-
 void Scheduler::sharedTimerAdapter()
 {
     shared()->tickSharedTimer();