SF: Cleanup EventThread Part 1
The cleanups in this CL are primarily about switching from
android::Thread to std::thread.
1) Convert from android::Thread to std::thread, along with using
std::mutex and std::condition_variable to keep consistency.
2) Switch the header to #pragma once.
3) Added Clang thread annotations and enabled the corresponding warning.
4) Added proper thread shutdown handling (invoked by dtor).
Test: No issues observed on Pixel XL
Bug: None
Change-Id: I2ef0ad18ef75e139f70d856031991f87d187efe6
diff --git a/services/surfaceflinger/EventThread.cpp b/services/surfaceflinger/EventThread.cpp
index 6b92cc8..2cbc59e 100644
--- a/services/surfaceflinger/EventThread.cpp
+++ b/services/surfaceflinger/EventThread.cpp
@@ -16,10 +16,14 @@
#define ATRACE_TAG ATRACE_TAG_GRAPHICS
-#include <stdint.h>
+#include <pthread.h>
+#include <sched.h>
#include <sys/types.h>
+#include <chrono>
+#include <cstdint>
#include <cutils/compiler.h>
+#include <cutils/sched_policy.h>
#include <gui/DisplayEventReceiver.h>
#include <gui/IDisplayEventConnection.h>
@@ -31,105 +35,126 @@
#include "EventThread.h"
#include "SurfaceFlinger.h"
-// ---------------------------------------------------------------------------
-namespace android {
+using namespace std::chrono_literals;
+
// ---------------------------------------------------------------------------
-EventThread::EventThread(const sp<VSyncSource>& src, SurfaceFlinger& flinger, bool interceptVSyncs)
- : mVSyncSource(src),
- mFlinger(flinger),
- mUseSoftwareVSync(false),
- mVsyncEnabled(false),
- mDebugVsyncEnabled(false),
- mInterceptVSyncs(interceptVSyncs) {
- for (int32_t i = 0; i < DisplayDevice::NUM_BUILTIN_DISPLAY_TYPES; i++) {
- mVSyncEvent[i].header.type = DisplayEventReceiver::DISPLAY_EVENT_VSYNC;
- mVSyncEvent[i].header.id = 0;
- mVSyncEvent[i].header.timestamp = 0;
- mVSyncEvent[i].vsync.count = 0;
+namespace android {
+
+// ---------------------------------------------------------------------------
+
+EventThread::EventThread(const sp<VSyncSource>& src, SurfaceFlinger& flinger, bool interceptVSyncs,
+ const char* threadName)
+ : mVSyncSource(src), mFlinger(flinger), mInterceptVSyncs(interceptVSyncs) {
+ for (auto& event : mVSyncEvent) {
+ event.header.type = DisplayEventReceiver::DISPLAY_EVENT_VSYNC;
+ event.header.id = 0;
+ event.header.timestamp = 0;
+ event.vsync.count = 0;
}
+
+ mThread = std::thread(&EventThread::threadMain, this);
+
+ pthread_setname_np(mThread.native_handle(), threadName);
+
+ pid_t tid = pthread_gettid_np(mThread.native_handle());
+
+ // Use SCHED_FIFO to minimize jitter
+ constexpr int EVENT_THREAD_PRIORITY = 2;
+ struct sched_param param = {0};
+ param.sched_priority = EVENT_THREAD_PRIORITY;
+ if (pthread_setschedparam(mThread.native_handle(), SCHED_FIFO, ¶m) != 0) {
+ ALOGE("Couldn't set SCHED_FIFO for EventThread");
+ }
+
+ set_sched_policy(tid, SP_FOREGROUND);
+}
+
+EventThread::~EventThread() {
+ {
+ std::lock_guard<std::mutex> lock(mMutex);
+ mKeepRunning = false;
+ mCondition.notify_all();
+ }
+ mThread.join();
}
void EventThread::setPhaseOffset(nsecs_t phaseOffset) {
- Mutex::Autolock _l(mLock);
+ std::lock_guard<std::mutex> lock(mMutex);
mVSyncSource->setPhaseOffset(phaseOffset);
}
-void EventThread::onFirstRef() {
- run("EventThread", PRIORITY_URGENT_DISPLAY + PRIORITY_MORE_FAVORABLE);
-}
-
sp<EventThread::Connection> EventThread::createEventConnection() const {
return new Connection(const_cast<EventThread*>(this));
}
status_t EventThread::registerDisplayEventConnection(
const sp<EventThread::Connection>& connection) {
- Mutex::Autolock _l(mLock);
+ std::lock_guard<std::mutex> lock(mMutex);
mDisplayEventConnections.add(connection);
- mCondition.broadcast();
+ mCondition.notify_all();
return NO_ERROR;
}
void EventThread::removeDisplayEventConnection(const wp<EventThread::Connection>& connection) {
- Mutex::Autolock _l(mLock);
+ std::lock_guard<std::mutex> lock(mMutex);
mDisplayEventConnections.remove(connection);
}
void EventThread::setVsyncRate(uint32_t count, const sp<EventThread::Connection>& connection) {
if (int32_t(count) >= 0) { // server must protect against bad params
- Mutex::Autolock _l(mLock);
+ std::lock_guard<std::mutex> lock(mMutex);
const int32_t new_count = (count == 0) ? -1 : count;
if (connection->count != new_count) {
connection->count = new_count;
- mCondition.broadcast();
+ mCondition.notify_all();
}
}
}
void EventThread::requestNextVsync(const sp<EventThread::Connection>& connection) {
- Mutex::Autolock _l(mLock);
+ std::lock_guard<std::mutex> lock(mMutex);
mFlinger.resyncWithRateLimit();
if (connection->count < 0) {
connection->count = 0;
- mCondition.broadcast();
+ mCondition.notify_all();
}
}
void EventThread::onScreenReleased() {
- Mutex::Autolock _l(mLock);
+ std::lock_guard<std::mutex> lock(mMutex);
if (!mUseSoftwareVSync) {
// disable reliance on h/w vsync
mUseSoftwareVSync = true;
- mCondition.broadcast();
+ mCondition.notify_all();
}
}
void EventThread::onScreenAcquired() {
- Mutex::Autolock _l(mLock);
+ std::lock_guard<std::mutex> lock(mMutex);
if (mUseSoftwareVSync) {
// resume use of h/w vsync
mUseSoftwareVSync = false;
- mCondition.broadcast();
+ mCondition.notify_all();
}
}
void EventThread::onVSyncEvent(nsecs_t timestamp) {
- Mutex::Autolock _l(mLock);
+ std::lock_guard<std::mutex> lock(mMutex);
mVSyncEvent[0].header.type = DisplayEventReceiver::DISPLAY_EVENT_VSYNC;
mVSyncEvent[0].header.id = 0;
mVSyncEvent[0].header.timestamp = timestamp;
mVSyncEvent[0].vsync.count++;
- mCondition.broadcast();
+ mCondition.notify_all();
}
void EventThread::onHotplugReceived(int type, bool connected) {
ALOGE_IF(type >= DisplayDevice::NUM_BUILTIN_DISPLAY_TYPES,
"received hotplug event for an invalid display (id=%d)", type);
- Mutex::Autolock _l(mLock);
+ std::lock_guard<std::mutex> lock(mMutex);
if (type < DisplayDevice::NUM_BUILTIN_DISPLAY_TYPES) {
DisplayEventReceiver::Event event;
event.header.type = DisplayEventReceiver::DISPLAY_EVENT_HOTPLUG;
@@ -137,46 +162,48 @@
event.header.timestamp = systemTime();
event.hotplug.connected = connected;
mPendingEvents.add(event);
- mCondition.broadcast();
+ mCondition.notify_all();
}
}
-bool EventThread::threadLoop() {
- DisplayEventReceiver::Event event;
- Vector<sp<EventThread::Connection> > signalConnections;
- signalConnections = waitForEvent(&event);
+void EventThread::threadMain() NO_THREAD_SAFETY_ANALYSIS {
+ std::unique_lock<std::mutex> lock(mMutex);
+ while (mKeepRunning) {
+ DisplayEventReceiver::Event event;
+ Vector<sp<EventThread::Connection> > signalConnections;
+ signalConnections = waitForEventLocked(&lock, &event);
- // dispatch events to listeners...
- const size_t count = signalConnections.size();
- for (size_t i = 0; i < count; i++) {
- const sp<Connection>& conn(signalConnections[i]);
- // now see if we still need to report this event
- status_t err = conn->postEvent(event);
- if (err == -EAGAIN || err == -EWOULDBLOCK) {
- // The destination doesn't accept events anymore, it's probably
- // full. For now, we just drop the events on the floor.
- // FIXME: Note that some events cannot be dropped and would have
- // to be re-sent later.
- // Right-now we don't have the ability to do this.
- ALOGW("EventThread: dropping event (%08x) for connection %p", event.header.type,
- conn.get());
- } else if (err < 0) {
- // handle any other error on the pipe as fatal. the only
- // reasonable thing to do is to clean-up this connection.
- // The most common error we'll get here is -EPIPE.
- removeDisplayEventConnection(signalConnections[i]);
+ // dispatch events to listeners...
+ const size_t count = signalConnections.size();
+ for (size_t i = 0; i < count; i++) {
+ const sp<Connection>& conn(signalConnections[i]);
+ // now see if we still need to report this event
+ status_t err = conn->postEvent(event);
+ if (err == -EAGAIN || err == -EWOULDBLOCK) {
+ // The destination doesn't accept events anymore, it's probably
+ // full. For now, we just drop the events on the floor.
+ // FIXME: Note that some events cannot be dropped and would have
+ // to be re-sent later.
+ // Right-now we don't have the ability to do this.
+ ALOGW("EventThread: dropping event (%08x) for connection %p", event.header.type,
+ conn.get());
+ } else if (err < 0) {
+ // handle any other error on the pipe as fatal. the only
+ // reasonable thing to do is to clean-up this connection.
+ // The most common error we'll get here is -EPIPE.
+ removeDisplayEventConnection(signalConnections[i]);
+ }
}
}
- return true;
}
// This will return when (1) a vsync event has been received, and (2) there was
// at least one connection interested in receiving it when we started waiting.
-Vector<sp<EventThread::Connection> > EventThread::waitForEvent(DisplayEventReceiver::Event* event) {
- Mutex::Autolock _l(mLock);
+Vector<sp<EventThread::Connection> > EventThread::waitForEventLocked(
+ std::unique_lock<std::mutex>* lock, DisplayEventReceiver::Event* event) {
Vector<sp<EventThread::Connection> > signalConnections;
- do {
+ while (signalConnections.isEmpty() && mKeepRunning) {
bool eventPending = false;
bool waitForVSync = false;
@@ -279,8 +306,8 @@
// use a (long) timeout when waiting for h/w vsync, and
// generate fake events when necessary.
bool softwareSync = mUseSoftwareVSync;
- nsecs_t timeout = softwareSync ? ms2ns(16) : ms2ns(1000);
- if (mCondition.waitRelative(mLock, timeout) == TIMED_OUT) {
+ auto timeout = softwareSync ? 16ms : 1000ms;
+ if (mCondition.wait_for(*lock, timeout) == std::cv_status::timeout) {
if (!softwareSync) {
ALOGW("Timed out waiting for hw vsync; faking it");
}
@@ -296,10 +323,10 @@
// h/w vsync should be disabled, so this will wait until we
// get a new connection, or an existing connection becomes
// interested in receiving vsync again.
- mCondition.wait(mLock);
+ mCondition.wait(*lock);
}
}
- } while (signalConnections.isEmpty());
+ }
// here we're guaranteed to have a timestamp and some connections to signal
// (The connections might have dropped out of mDisplayEventConnections
@@ -328,7 +355,7 @@
}
void EventThread::dump(String8& result) const {
- Mutex::Autolock _l(mLock);
+ std::lock_guard<std::mutex> lock(mMutex);
result.appendFormat("VSYNC state: %s\n", mDebugVsyncEnabled ? "enabled" : "disabled");
result.appendFormat(" soft-vsync: %s\n", mUseSoftwareVSync ? "enabled" : "disabled");
result.appendFormat(" numListeners=%zu,\n events-delivered: %u\n",
@@ -377,4 +404,4 @@
// ---------------------------------------------------------------------------
-}; // namespace android
+} // namespace android