scheduler: Post high priority tasks directly to the message loop

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"
@@ skipping 33 matching lines @@
     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()
+    MainThreadPendingHighPriorityTaskRunner(
+        const Scheduler::Task& task, const TraceLocation& location, const char* traceName)
+        : m_task(task, location, traceName)
     {
         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)
-            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.
 class Scheduler::MainThreadPendingTaskRunner : public WebThread::Task {
 public:
     MainThreadPendingTaskRunner(
         const Scheduler::Task& task, const TraceLocation& location, const char* traceName)
         : m_task(task, location, traceName)
     {
         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()->runPendingHighPriorityTasksIfInCompositorPriority();
         m_task.run();
+        if (Scheduler* scheduler = Scheduler::shared()) {
+            scheduler->updatePolicy();
+        }
     }
 
     TracedTask m_task;
 };
 
 Scheduler* Scheduler::s_sharedScheduler = nullptr;
 
 void Scheduler::initializeOnMainThread()
 {
     s_sharedScheduler = new Scheduler();
@@ skipping 15 matching lines @@
     , m_mainThread(blink::Platform::current()->currentThread())
     , m_compositorPriorityPolicyEndTimeSeconds(0)
     , m_highPriorityTaskCount(0)
     , m_highPriorityTaskRunnerPosted(false)
     , m_schedulerPolicy(Normal)
 {
 }
 
 Scheduler::~Scheduler()
 {
-    while (hasPendingHighPriorityWork()) {
-        swapQueuesAndRunPendingTasks();
-    }
 }
 
 void Scheduler::willBeginFrame(const WebBeginFrameArgs& args)
 {
     // TODO: Use frame deadline and interval to schedule idle tasks.
 }
 
 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));
 }
 
 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);
+}
+
 void Scheduler::postTask(const TraceLocation& location, const Task& task)
 {
     m_mainThread->postTask(new MainThreadPendingTaskRunner(task, location, "Scheduler::MainThreadTask"));
 }
 
 void Scheduler::postInputTask(const TraceLocation& location, const Task& task)
 {
     postHighPriorityTaskInternal(location, task, "Scheduler::InputTask");
 }
 
 void Scheduler::didReceiveInputEvent()
 {
     enterSchedulerPolicy(CompositorPriority);
 }
 
 void Scheduler::postCompositorTask(const TraceLocation& location, const Task& task)
 {
     postHighPriorityTaskInternal(location, task, "Scheduler::CompositorTask");
 }
 
 void Scheduler::postIpcTask(const TraceLocation& location, const Task& task)
 {
     // FIXME: we want IPCs to be high priority, but we can't currently do that because some of them can take a very long
     // time to process. These need refactoring but we need to add some infrastructure to identify them.
     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);
 }
 
 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();
 }
 
 void Scheduler::setSharedTimerFiredFunction(void (*function)())
 {
     m_sharedTimerFunction = function;
     blink::Platform::current()->setSharedTimerFiredFunction(function ? &Scheduler::sharedTimerAdapter : nullptr);
 }
@@ skipping 18 matching lines @@
 }
 
 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;
 }

[picksi1, 2014/10/06 13:21:02]

Does this function need to check the policy too? If we're in normal mode should
we pretend we have no high priority work?

[Sami, 2014/10/06 14:00:47]

No, I don't think there's a reason to lie here. This function should do what it
says. Actually what I'll do is fold this function into shouldYield() since
that's the only place this is used.

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

That's done in shouldYieldForHighPriorityWork, this function is fine.

 
 Scheduler::SchedulerPolicy Scheduler::schedulerPolicy() const
 {
     ASSERT(isMainThread());
     // It's important not to miss the transition from normal to low latency mode, otherwise we're likely to
     // delay the processing of input tasks. Since that transition is triggered by a different thread, we
     // need either a lock or a memory barrier, and the memory barrier is probably cheaper.
     return static_cast<SchedulerPolicy>(acquireLoad(&m_schedulerPolicy));
 }
 
 void Scheduler::enterSchedulerPolicy(SchedulerPolicy schedulerPolicy)
 {
     Locker<Mutex> lock(m_pendingTasksMutex);
     enterSchedulerPolicyLocked(schedulerPolicy);
 }
 
 void Scheduler::enterSchedulerPolicyLocked(SchedulerPolicy schedulerPolicy)
 {
     ASSERT(m_pendingTasksMutex.locked());
     if (schedulerPolicy == CompositorPriority)
         m_compositorPriorityPolicyEndTimeSeconds = Platform::current()->monotonicallyIncreasingTime() + kLowSchedulerPolicyAfterTouchTimeSeconds;
 
     releaseStore(&m_schedulerPolicy, schedulerPolicy);
     TRACE_COUNTER1(TRACE_DISABLED_BY_DEFAULT("blink.scheduler"), "SchedulerPolicy", schedulerPolicy);
 }
 
 } // namespace blink