Merge "sf: avoid lock on main thread during luma calc" into qt-dev
diff --git a/services/surfaceflinger/RegionSamplingThread.cpp b/services/surfaceflinger/RegionSamplingThread.cpp
index 0d14267..3684260 100644
--- a/services/surfaceflinger/RegionSamplingThread.cpp
+++ b/services/surfaceflinger/RegionSamplingThread.cpp
@@ -168,11 +168,8 @@
mPhaseCallback(std::make_unique<SamplingOffsetCallback>(*this, mScheduler,
tunables.mSamplingOffset)),
lastSampleTime(0ns) {
- {
- std::lock_guard threadLock(mThreadMutex);
- mThread = std::thread([this]() { threadMain(); });
- pthread_setname_np(mThread.native_handle(), "RegionSamplingThread");
- }
+ mThread = std::thread([this]() { threadMain(); });
+ pthread_setname_np(mThread.native_handle(), "RegionSamplingThread");
mIdleTimer.start();
}
@@ -186,12 +183,11 @@
mIdleTimer.stop();
{
- std::lock_guard lock(mMutex);
+ std::lock_guard lock(mThreadControlMutex);
mRunning = false;
mCondition.notify_one();
}
- std::lock_guard threadLock(mThreadMutex);
if (mThread.joinable()) {
mThread.join();
}
@@ -205,17 +201,17 @@
sp<IBinder> asBinder = IInterface::asBinder(listener);
asBinder->linkToDeath(this);
- std::lock_guard lock(mMutex);
+ std::lock_guard lock(mSamplingMutex);
mDescriptors.emplace(wp<IBinder>(asBinder), Descriptor{samplingArea, stopLayer, listener});
}
void RegionSamplingThread::removeListener(const sp<IRegionSamplingListener>& listener) {
- std::lock_guard lock(mMutex);
+ std::lock_guard lock(mSamplingMutex);
mDescriptors.erase(wp<IBinder>(IInterface::asBinder(listener)));
}
void RegionSamplingThread::checkForStaleLuma() {
- std::lock_guard lock(mMutex);
+ std::lock_guard lock(mThreadControlMutex);
if (mDiscardedFrames) {
ATRACE_INT(lumaSamplingStepTag, static_cast<int>(samplingStep::waitForZeroPhase));
@@ -233,7 +229,7 @@
}
void RegionSamplingThread::doSample() {
- std::lock_guard lock(mMutex);
+ std::lock_guard lock(mThreadControlMutex);
auto now = std::chrono::nanoseconds(systemTime(SYSTEM_TIME_MONOTONIC));
if (lastSampleTime + mTunables.mSamplingPeriod > now) {
ATRACE_INT(lumaSamplingStepTag, static_cast<int>(samplingStep::idleTimerWaiting));
@@ -254,7 +250,7 @@
}
void RegionSamplingThread::binderDied(const wp<IBinder>& who) {
- std::lock_guard lock(mMutex);
+ std::lock_guard lock(mSamplingMutex);
mDescriptors.erase(who);
}
@@ -315,6 +311,7 @@
void RegionSamplingThread::captureSample() {
ATRACE_CALL();
+ std::lock_guard lock(mSamplingMutex);
if (mDescriptors.empty()) {
return;
@@ -387,19 +384,8 @@
PIXEL_FORMAT_RGBA_8888, 1, usage, "RegionSamplingThread");
}
- // When calling into SF, we post a message into the SF message queue (so the
- // screen capture runs on the main thread). This message blocks until the
- // screenshot is actually captured, but before the capture occurs, the main
- // thread may perform a normal refresh cycle. At the end of this cycle, it
- // can request another sample (because layers changed), which triggers a
- // call into sampleNow. When sampleNow attempts to grab the mutex, we can
- // deadlock.
- //
- // To avoid this, we drop the mutex while we call into SF.
- mMutex.unlock();
bool ignored;
mFlinger.captureScreenCommon(renderArea, traverseLayers, buffer, false, ignored);
- mMutex.lock();
std::vector<Descriptor> activeDescriptors;
for (const auto& descriptor : descriptors) {
@@ -429,15 +415,19 @@
ATRACE_INT(lumaSamplingStepTag, static_cast<int>(samplingStep::noWorkNeeded));
}
-void RegionSamplingThread::threadMain() {
- std::lock_guard lock(mMutex);
+// NO_THREAD_SAFETY_ANALYSIS is because std::unique_lock presently lacks thread safety annotations.
+void RegionSamplingThread::threadMain() NO_THREAD_SAFETY_ANALYSIS {
+ std::unique_lock<std::mutex> lock(mThreadControlMutex);
while (mRunning) {
if (mSampleRequested) {
mSampleRequested = false;
+ lock.unlock();
captureSample();
+ lock.lock();
}
- mCondition.wait(mMutex,
- [this]() REQUIRES(mMutex) { return mSampleRequested || !mRunning; });
+ mCondition.wait(lock, [this]() REQUIRES(mThreadControlMutex) {
+ return mSampleRequested || !mRunning;
+ });
}
}
diff --git a/services/surfaceflinger/RegionSamplingThread.h b/services/surfaceflinger/RegionSamplingThread.h
index 72b2042..08134e6 100644
--- a/services/surfaceflinger/RegionSamplingThread.h
+++ b/services/surfaceflinger/RegionSamplingThread.h
@@ -100,7 +100,7 @@
void binderDied(const wp<IBinder>& who) override;
void checkForStaleLuma();
- void captureSample() REQUIRES(mMutex);
+ void captureSample();
void threadMain();
SurfaceFlinger& mFlinger;
@@ -110,19 +110,18 @@
std::unique_ptr<SamplingOffsetCallback> const mPhaseCallback;
- std::mutex mThreadMutex;
- std::thread mThread GUARDED_BY(mThreadMutex);
+ std::thread mThread;
- std::mutex mMutex;
+ std::mutex mThreadControlMutex;
std::condition_variable_any mCondition;
- bool mRunning GUARDED_BY(mMutex) = true;
- bool mSampleRequested GUARDED_BY(mMutex) = false;
+ bool mRunning GUARDED_BY(mThreadControlMutex) = true;
+ bool mSampleRequested GUARDED_BY(mThreadControlMutex) = false;
+ bool mDiscardedFrames GUARDED_BY(mThreadControlMutex) = false;
+ std::chrono::nanoseconds lastSampleTime GUARDED_BY(mThreadControlMutex);
- std::unordered_map<wp<IBinder>, Descriptor, WpHash> mDescriptors GUARDED_BY(mMutex);
- std::chrono::nanoseconds lastSampleTime GUARDED_BY(mMutex);
- bool mDiscardedFrames GUARDED_BY(mMutex) = false;
-
- sp<GraphicBuffer> mCachedBuffer GUARDED_BY(mMutex) = nullptr;
+ std::mutex mSamplingMutex;
+ std::unordered_map<wp<IBinder>, Descriptor, WpHash> mDescriptors GUARDED_BY(mSamplingMutex);
+ sp<GraphicBuffer> mCachedBuffer GUARDED_BY(mSamplingMutex) = nullptr;
};
} // namespace android