SF: Cleanups to use std::atomic/std::mutex
A few places in the code used "volatile". This patch removes the
volatile.
In most of the cases, the volatile value was converted to a std::atomic
value. The exception was Barrier where it was not actually necessary,
but there I converted the code to use std::mutex.
Bug: None
Test: atest SurfaceFlinger_test CtsViewTestCases
Change-Id: I7b4f5a7398a241db8201dc7f60d7c9cd41e32e1b
diff --git a/services/surfaceflinger/Barrier.h b/services/surfaceflinger/Barrier.h
index 3e9d443..97028a8 100644
--- a/services/surfaceflinger/Barrier.h
+++ b/services/surfaceflinger/Barrier.h
@@ -18,46 +18,40 @@
#define ANDROID_BARRIER_H
#include <stdint.h>
-#include <sys/types.h>
-#include <utils/threads.h>
+#include <condition_variable>
+#include <mutex>
namespace android {
class Barrier
{
public:
- inline Barrier() : state(CLOSED) { }
- inline ~Barrier() { }
-
// Release any threads waiting at the Barrier.
// Provides release semantics: preceding loads and stores will be visible
// to other threads before they wake up.
void open() {
- Mutex::Autolock _l(lock);
- state = OPENED;
- cv.broadcast();
+ std::lock_guard<std::mutex> lock(mMutex);
+ mIsOpen = true;
+ mCondition.notify_all();
}
// Reset the Barrier, so wait() will block until open() has been called.
void close() {
- Mutex::Autolock _l(lock);
- state = CLOSED;
+ std::lock_guard<std::mutex> lock(mMutex);
+ mIsOpen = false;
}
// Wait until the Barrier is OPEN.
// Provides acquire semantics: no subsequent loads or stores will occur
// until wait() returns.
void wait() const {
- Mutex::Autolock _l(lock);
- while (state == CLOSED) {
- cv.wait(lock);
- }
+ std::unique_lock<std::mutex> lock(mMutex);
+ mCondition.wait(lock, [this]() NO_THREAD_SAFETY_ANALYSIS { return mIsOpen; });
}
private:
- enum { OPENED, CLOSED };
- mutable Mutex lock;
- mutable Condition cv;
- volatile int state;
+ mutable std::mutex mMutex;
+ mutable std::condition_variable mCondition;
+ int mIsOpen GUARDED_BY(mMutex){false};
};
}; // namespace android
diff --git a/services/surfaceflinger/BufferQueueLayer.cpp b/services/surfaceflinger/BufferQueueLayer.cpp
index 4839bfb..521cdbf 100644
--- a/services/surfaceflinger/BufferQueueLayer.cpp
+++ b/services/surfaceflinger/BufferQueueLayer.cpp
@@ -207,8 +207,9 @@
}
std::optional<Region> BufferQueueLayer::latchSidebandStream(bool& recomputeVisibleRegions) {
- if (android_atomic_acquire_cas(true, false, &mSidebandStreamChanged) == 0) {
- // mSidebandStreamChanged was true
+ bool sidebandStreamChanged = true;
+ if (mSidebandStreamChanged.compare_exchange_strong(sidebandStreamChanged, false)) {
+ // mSidebandStreamChanged was changed to false
// replicated in LayerBE until FE/BE is ready to be synchronized
getBE().compositionInfo.hwc.sidebandStream = mConsumer->getSidebandStream();
if (getBE().compositionInfo.hwc.sidebandStream != nullptr) {
@@ -259,7 +260,7 @@
Mutex::Autolock lock(mQueueItemLock);
mTimeStats.removeTimeRecord(getName().c_str(), mQueueItems[0].mFrameNumber);
mQueueItems.removeAt(0);
- android_atomic_dec(&mQueuedFrames);
+ mQueuedFrames--;
}
return BAD_VALUE;
} else if (updateResult != NO_ERROR || mUpdateTexImageFailed) {
@@ -270,7 +271,7 @@
if (queuedBuffer) {
Mutex::Autolock lock(mQueueItemLock);
mQueueItems.clear();
- android_atomic_and(0, &mQueuedFrames);
+ mQueuedFrames = 0;
mTimeStats.clearLayerRecord(getName().c_str());
}
@@ -294,7 +295,7 @@
while (mQueueItems[0].mFrameNumber != currentFrameNumber) {
mTimeStats.removeTimeRecord(getName().c_str(), mQueueItems[0].mFrameNumber);
mQueueItems.removeAt(0);
- android_atomic_dec(&mQueuedFrames);
+ mQueuedFrames--;
}
const std::string layerName(getName().c_str());
@@ -306,7 +307,7 @@
// Decrement the queued-frames count. Signal another event if we
// have more frames pending.
- if ((queuedBuffer && android_atomic_dec(&mQueuedFrames) > 1) || mAutoRefresh) {
+ if ((queuedBuffer && mQueuedFrames.fetch_sub(1) > 1) || mAutoRefresh) {
mFlinger->signalLayerUpdate();
}
@@ -367,7 +368,7 @@
}
mQueueItems.push_back(item);
- android_atomic_inc(&mQueuedFrames);
+ mQueuedFrames++;
// Wake up any pending callbacks
mLastFrameNumberReceived = item.mFrameNumber;
@@ -404,8 +405,9 @@
}
void BufferQueueLayer::onSidebandStreamChanged() {
- if (android_atomic_release_cas(false, true, &mSidebandStreamChanged) == 0) {
- // mSidebandStreamChanged was false
+ bool sidebandStreamChanged = false;
+ if (mSidebandStreamChanged.compare_exchange_strong(sidebandStreamChanged, true)) {
+ // mSidebandStreamChanged was changed to true
mFlinger->signalLayerUpdate();
}
}
diff --git a/services/surfaceflinger/BufferQueueLayer.h b/services/surfaceflinger/BufferQueueLayer.h
index 579ed81..400f412 100644
--- a/services/surfaceflinger/BufferQueueLayer.h
+++ b/services/surfaceflinger/BufferQueueLayer.h
@@ -137,8 +137,8 @@
int mActiveBufferSlot;
// thread-safe
- volatile int32_t mQueuedFrames;
- volatile int32_t mSidebandStreamChanged; // used like an atomic boolean
+ std::atomic<int32_t> mQueuedFrames{0};
+ std::atomic<bool> mSidebandStreamChanged{false};
};
} // namespace android
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 03b63bd..19c84d0 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -62,12 +62,11 @@
namespace android {
-int32_t Layer::sSequence = 1;
+std::atomic<int32_t> Layer::sSequence{1};
Layer::Layer(SurfaceFlinger* flinger, const sp<Client>& client, const String8& name, uint32_t w,
uint32_t h, uint32_t flags)
: contentDirty(false),
- sequence(uint32_t(android_atomic_inc(&sSequence))),
mFlinger(flinger),
mPremultipliedAlpha(true),
mName(name),
@@ -1019,11 +1018,11 @@
}
uint32_t Layer::getTransactionFlags(uint32_t flags) {
- return android_atomic_and(~flags, &mTransactionFlags) & flags;
+ return mTransactionFlags.fetch_and(~flags) & flags;
}
uint32_t Layer::setTransactionFlags(uint32_t flags) {
- return android_atomic_or(flags, &mTransactionFlags);
+ return mTransactionFlags.fetch_or(flags);
}
bool Layer::setPosition(float x, float y, bool immediate) {
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index 3845b22..74f0a63 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -73,7 +73,7 @@
// ---------------------------------------------------------------------------
class Layer : public virtual RefBase {
- static int32_t sSequence;
+ static std::atomic<int32_t> sSequence;
public:
friend class LayerBE;
@@ -89,7 +89,7 @@
// Layer serial number. This gives layers an explicit ordering, so we
// have a stable sort order when their layer stack and Z-order are
// the same.
- int32_t sequence;
+ int32_t sequence{sSequence++};
enum { // flags for doTransaction()
eDontUpdateGeometryState = 0x00000001,
@@ -271,6 +271,7 @@
virtual void useSurfaceDamage() {}
virtual void useEmptyDamage() {}
+ uint32_t getTransactionFlags() const { return mTransactionFlags; }
uint32_t getTransactionFlags(uint32_t flags);
uint32_t setTransactionFlags(uint32_t flags);
@@ -696,7 +697,7 @@
// these are protected by an external lock
State mCurrentState;
State mDrawingState;
- volatile int32_t mTransactionFlags;
+ std::atomic<uint32_t> mTransactionFlags{0};
// Accessed from main thread and binder threads
Mutex mPendingStateMutex;
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index ad8a4dc..e2605b2 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -259,12 +259,10 @@
SurfaceFlinger::SurfaceFlinger(SurfaceFlinger::SkipInitializationTag)
: BnSurfaceComposer(),
- mTransactionFlags(0),
mTransactionPending(false),
mAnimTransactionPending(false),
mLayersRemoved(false),
mLayersAdded(false),
- mRepaintEverything(0),
mBootTime(systemTime()),
mDisplayTokens(),
mVisibleRegionsDirty(false),
@@ -1468,7 +1466,7 @@
resyncToHardwareVsync(false);
- android_atomic_or(1, &mRepaintEverything);
+ mRepaintEverything = true;
setTransactionFlags(eDisplayTransactionNeeded);
}
@@ -1527,7 +1525,7 @@
mRefreshPending = false;
- const bool repaintEverything = android_atomic_and(0, &mRepaintEverything);
+ const bool repaintEverything = mRepaintEverything.exchange(false);
preComposition();
rebuildLayerStacks();
calculateWorkingSet();
@@ -3355,11 +3353,11 @@
}
uint32_t SurfaceFlinger::peekTransactionFlags() {
- return android_atomic_release_load(&mTransactionFlags);
+ return mTransactionFlags;
}
uint32_t SurfaceFlinger::getTransactionFlags(uint32_t flags) {
- return android_atomic_and(~flags, &mTransactionFlags) & flags;
+ return mTransactionFlags.fetch_and(~flags) & flags;
}
uint32_t SurfaceFlinger::setTransactionFlags(uint32_t flags) {
@@ -3368,7 +3366,7 @@
uint32_t SurfaceFlinger::setTransactionFlags(uint32_t flags,
VSyncModulator::TransactionStart transactionStart) {
- uint32_t old = android_atomic_or(flags, &mTransactionFlags);
+ uint32_t old = mTransactionFlags.fetch_or(flags);
mVsyncModulator.setTransactionStart(transactionStart);
if ((old & flags)==0) { // wake the server up
signalTransaction();
@@ -4661,7 +4659,7 @@
}
result.appendFormat(" transaction-flags : %08x\n"
" gpu_to_cpu_unsupported : %d\n",
- mTransactionFlags, !mGpuToCpuSupported);
+ mTransactionFlags.load(), !mGpuToCpuSupported);
if (display) {
const auto activeConfig = getHwComposer().getActiveConfig(display->getId());
@@ -5172,7 +5170,7 @@
}
void SurfaceFlinger::repaintEverything() {
- android_atomic_or(1, &mRepaintEverything);
+ mRepaintEverything = true;
signalTransaction();
}
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index bfadf6e..a300857 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -794,7 +794,7 @@
// access must be protected by mStateLock
mutable Mutex mStateLock;
State mCurrentState{LayerVector::StateSet::Current};
- volatile int32_t mTransactionFlags;
+ std::atomic<int32_t> mTransactionFlags{0};
Condition mTransactionCV;
bool mTransactionPending;
bool mAnimTransactionPending;
@@ -813,8 +813,7 @@
bool mLayersRemoved;
bool mLayersAdded;
- // access must be protected by mInvalidateLock
- volatile int32_t mRepaintEverything;
+ std::atomic<bool> mRepaintEverything{false};
// helper methods
void configureHwcCommonData(const CompositionInfo& compositionInfo) const;
diff --git a/services/surfaceflinger/SurfaceInterceptor.cpp b/services/surfaceflinger/SurfaceInterceptor.cpp
index 0b4c6fc..57bda5a 100644
--- a/services/surfaceflinger/SurfaceInterceptor.cpp
+++ b/services/surfaceflinger/SurfaceInterceptor.cpp
@@ -96,8 +96,9 @@
const sp<const Layer>& layer)
{
Transaction* transaction(increment->mutable_transaction());
- transaction->set_synchronous(layer->mTransactionFlags & BnSurfaceComposer::eSynchronous);
- transaction->set_animation(layer->mTransactionFlags & BnSurfaceComposer::eAnimation);
+ const uint32_t layerFlags = layer->getTransactionFlags();
+ transaction->set_synchronous(layerFlags & BnSurfaceComposer::eSynchronous);
+ transaction->set_animation(layerFlags & BnSurfaceComposer::eAnimation);
const int32_t layerId(getLayerId(layer));
addPositionLocked(transaction, layerId, layer->mCurrentState.active_legacy.transform.tx(),