Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2425)

Unified Diff: Source/platform/scheduler/Scheduler.cpp

Issue 439923006: Prioritizing input and compositor tasks in the blink scheduler. (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@master
Patch Set: Respoding to more feedback Created 6 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: Source/platform/scheduler/Scheduler.cpp
diff --git a/Source/platform/scheduler/Scheduler.cpp b/Source/platform/scheduler/Scheduler.cpp
index 61ae7aef1d486bd618417a2d2d62c14c08db06bc..11a3cb0bb219bc208e24dcbe22b2818f31db039a 100644
--- a/Source/platform/scheduler/Scheduler.cpp
+++ b/Source/platform/scheduler/Scheduler.cpp
@@ -5,60 +5,61 @@
#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 {
+class MainThreadIdleTaskAdapter : public blink::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)
+ : m_idleTask(idleTask)
+ , m_allottedTimeMs(allottedTimeMs)
eseidel 2014/08/07 17:18:15 Thank you for including the type in the name. :)
Sami 2014/08/07 18:22:21 +1, I wouldn't object to a clone of base/time.h's
alexclarke 2014/08/08 13:32:23 I also think that's a good idea, but lets do it in
alexclarke 2014/08/08 13:32:23 Acknowledged.
{
}
// WebThread::Task implementation.
virtual void run() OVERRIDE
{
- TRACE_EVENT2("blink", "MainThreadTaskAdapter::run",
- "src_file", m_location.fileName(),
- "src_func", m_location.functionName());
- m_task();
+ TRACE_EVENT1("blink", "MainThreadIdleTaskAdapter::run", "allottedTime", m_allottedTimeMs);
+ m_idleTask(m_allottedTimeMs);
}
private:
- const TraceLocation m_location;
- Scheduler::Task m_task;
+ Scheduler::IdleTask m_idleTask;
+ double m_allottedTimeMs;
};
-class MainThreadIdleTaskAdapter : public blink::WebThread::Task {
+} // namespace
+
+class Scheduler::MainThreadPendingTaskRunner : public blink::WebThread::Task {
public:
- MainThreadIdleTaskAdapter(const Scheduler::IdleTask& idleTask, double allottedTimeMs)
- : m_idleTask(idleTask)
- , m_allottedTimeMs(allottedTimeMs)
+ explicit MainThreadPendingTaskRunner(Scheduler* scheduler)
+ : m_scheduler(scheduler)
{
+ atomicIncrement(&m_scheduler->m_mainThreadTaskRunnerCount);
+ }
+
+ ~MainThreadPendingTaskRunner()
+ {
+ atomicDecrement(&m_scheduler->m_mainThreadTaskRunnerCount);
}
// WebThread::Task implementation.
virtual void run() OVERRIDE
{
- TRACE_EVENT1("blink", "MainThreadIdleTaskAdapter::run", "allottedTime", m_allottedTimeMs);
- m_idleTask(m_allottedTimeMs);
+ Scheduler::shared()->runPendingTasks();
}
-private:
- Scheduler::IdleTask m_idleTask;
- double m_allottedTimeMs;
+ Scheduler* m_scheduler; // Not owned.
};
-}
-
Scheduler* Scheduler::s_sharedScheduler = nullptr;
void Scheduler::initializeOnMainThread()
@@ -80,16 +81,15 @@ Scheduler* Scheduler::shared()
Scheduler::Scheduler()
: m_mainThread(blink::Platform::current()->currentThread())
, m_sharedTimerFunction(nullptr)
+ , m_mainThreadTaskRunnerCount(0)
{
}
Scheduler::~Scheduler()
{
-}
-
-void Scheduler::scheduleTask(const TraceLocation& location, const Task& task)
-{
- m_mainThread->postTask(new MainThreadTaskAdapter(location, task));
+ // We need to flush all posted tasks before exiting.
+ while (!m_pendingLowPriorityTasks.isEmpty())
+ runPendingTasks();
}
void Scheduler::scheduleIdleTask(const IdleTask& idleTask)
@@ -100,17 +100,20 @@ void Scheduler::scheduleIdleTask(const IdleTask& idleTask)
void Scheduler::postTask(const TraceLocation& location, const Task& task)
{
- scheduleTask(location, task);
+ m_pendingLowPriorityTasks.append(TracedTask(task, location));
+ maybePostMainThreadPendingTaskRunner();
}
void Scheduler::postInputTask(const TraceLocation& location, const Task& task)
{
- scheduleTask(location, task);
+ m_pendingInputTasks.append(TracedTask(task, location));
+ maybePostMainThreadPendingTaskRunner();
}
void Scheduler::postCompositorTask(const TraceLocation& location, const Task& task)
{
- scheduleTask(location, task);
+ m_pendingCompositorTasks.append(TracedTask(task, location));
+ maybePostMainThreadPendingTaskRunner();
}
void Scheduler::postIdleTask(const IdleTask& idleTask)
@@ -121,7 +124,49 @@ void Scheduler::postIdleTask(const IdleTask& idleTask)
void Scheduler::tickSharedTimer()
{
TRACE_EVENT0("blink", "Scheduler::tickSharedTimer");
+
+ runHighPriorityTasks();
m_sharedTimerFunction();
+ runHighPriorityTasks();
+}
+
+void Scheduler::runPendingTasks()
+{
+ TRACE_EVENT0("blink", "Scheduler::runPendingTasks");
+
+ // Execute high priority tasks first.
+ runHighPriorityTasks();
+
+ if (!m_pendingLowPriorityTasks.runFirstTaskIfNotEmpty())
+ return;
+
+ // To avoid staving main loop tasks that don't (yet?) go through the scheduler, we only
+ // execute one low priority task rather than all of them.
+ if (!m_pendingLowPriorityTasks.isEmpty())
+ m_mainThread->postTask(new MainThreadPendingTaskRunner(this));
+}
+
+void Scheduler::runHighPriorityTasks()
+{
+ TRACE_EVENT0("blink", "Scheduler::runHighPriorityTasks");
+
+ for (;;) {
+ if (m_pendingInputTasks.runFirstTaskIfNotEmpty())
+ continue;
+
+ if (!m_pendingCompositorTasks.runFirstTaskIfNotEmpty())
+ break;
+ }
+}
+
+void Scheduler::maybePostMainThreadPendingTaskRunner()
+{
+ if (acquireLoad(&m_mainThreadTaskRunnerCount))
+ return;
+
+ // Note this can potentially cause a sequence of MainThreadPendingTaskRunners to get posted
+ // if there are multiple low priority tasks to run.
+ m_mainThread->postTask(new MainThreadPendingTaskRunner(this));
}
void Scheduler::sharedTimerAdapter()
@@ -147,7 +192,41 @@ void Scheduler::stopSharedTimer()
bool Scheduler::shouldYieldForHighPriorityWork()
{
- return false;
+ return !m_pendingInputTasks.isEmpty() || !m_pendingCompositorTasks.isEmpty();
+}
+
+void Scheduler::TracedTask::run()
+{
+ TRACE_EVENT2("blink", "TracedTask::run",
+ "src_file", m_location.fileName(),
+ "src_func", m_location.functionName());
+ m_task();
+}
+
+void Scheduler::LockingTracedTaskDeque::append(const TracedTask& value)
+{
+ Locker<Mutex> lock(m_mutex);
+ m_queue.append(value);
+}
+
+bool Scheduler::LockingTracedTaskDeque::isEmpty()
+{
+ Locker<Mutex> lock(m_mutex);
+ return m_queue.isEmpty();
+}
+
+bool Scheduler::LockingTracedTaskDeque::runFirstTaskIfNotEmpty()
+{
+ m_mutex.lock();
+ if (m_queue.isEmpty()) {
+ m_mutex.unlock();
+ return false;
+ }
+ TracedTask task = m_queue.takeFirst();
+ m_mutex.unlock();
+
+ task.run();
+ return true;
}
} // namespace blink

Powered by Google App Engine
This is Rietveld 408576698