Merge "Prevent waiting synchronous case if transaction applied first" into sc-dev (132bfbf5) · Commits · e / os / platform_frameworks_native

services/surfaceflinger/SurfaceFlinger.cpp

+58 −54

Original line number	Diff line number	Diff line
		@@ -3041,9 +3041,8 @@ void SurfaceFlinger::setVsyncConfig(const VsyncModulator::VsyncConfig& config,

		void SurfaceFlinger::commitTransaction() {
		commitTransactionLocked();
		mTransactionPending = false;
		signalSynchronousTransactions();
		mAnimTransactionPending = false;
		mTransactionCV.broadcast();
		}

		void SurfaceFlinger::commitTransactionLocked() {
		@@ -3312,7 +3311,7 @@ void SurfaceFlinger::flushTransactionQueues() {
		auto& [applyToken, transactionQueue] = *it;

		while (!transactionQueue.empty()) {
		const auto& transaction = transactionQueue.front();
		auto& transaction = transactionQueue.front();
		if (!transactionIsReadyToBeApplied(transaction.frameTimelineInfo,
		transaction.isAutoTimestamp,
		transaction.desiredPresentTime,
		@@ -3320,7 +3319,7 @@ void SurfaceFlinger::flushTransactionQueues() {
		setTransactionFlags(eTransactionFlushNeeded);
		break;
		}
		transactions.push_back(transaction);
		transactions.emplace_back(std::move(transaction));
		transactionQueue.pop();
		}

		@@ -3337,7 +3336,7 @@ void SurfaceFlinger::flushTransactionQueues() {
		// Case 2: push to pending when there exist a pending queue.
		// Case 3: others are ready to apply.
		while (!mTransactionQueue.empty()) {
		const auto& transaction = mTransactionQueue.front();
		auto& transaction = mTransactionQueue.front();
		bool pendingTransactions = mPendingTransactionQueues.find(transaction.applyToken) !=
		mPendingTransactionQueues.end();
		if (!transactionIsReadyToBeApplied(transaction.frameTimelineInfo,
		@@ -3345,9 +3344,9 @@ void SurfaceFlinger::flushTransactionQueues() {
		transaction.desiredPresentTime,
		transaction.states, pendingBuffers) \|\|
		pendingTransactions) {
		mPendingTransactionQueues[transaction.applyToken].push(transaction);
		mPendingTransactionQueues[transaction.applyToken].push(std::move(transaction));
		} else {
		transactions.push_back(transaction);
		transactions.emplace_back(std::move(transaction));
		}
		mTransactionQueue.pop();
		ATRACE_INT("TransactionQueue", mTransactionQueue.size());
		@@ -3363,6 +3362,10 @@ void SurfaceFlinger::flushTransactionQueues() {
		transaction.postTime, transaction.permissions,
		transaction.hasListenerCallbacks, transaction.listenerCallbacks,
		transaction.originPid, transaction.originUid, transaction.id);
		if (transaction.transactionCommittedSignal) {
		mTransactionCommittedSignals.emplace_back(
		std::move(transaction.transactionCommittedSignal));
		}
		}
		}
		}
		@@ -3430,7 +3433,7 @@ bool SurfaceFlinger::transactionIsReadyToBeApplied(
		return ready;
		}

		void SurfaceFlinger::queueTransaction(TransactionState state) {
		void SurfaceFlinger::queueTransaction(TransactionState& state) {
		Mutex::Autolock _l(mQueueLock);

		// If its TransactionQueue already has a pending TransactionState or if it is pending
		@@ -3450,19 +3453,14 @@ void SurfaceFlinger::queueTransaction(TransactionState state) {
		}
		}

		mTransactionQueue.emplace(state);
		ATRACE_INT("TransactionQueue", mTransactionQueue.size());

		// TODO(b/159125966): Remove eEarlyWakeup completely as no client should use this flag
		if (state.flags & eEarlyWakeup) {
		ALOGW("eEarlyWakeup is deprecated. Use eExplicitEarlyWakeup[Start\|End]");
		// Generate a CountDownLatch pending state if this is a synchronous transaction.
		if ((state.flags & eSynchronous) \|\| state.inputWindowCommands.syncInputWindows) {
		state.transactionCommittedSignal = std::make_shared<CountDownLatch>(
		(state.inputWindowCommands.syncInputWindows ? 2 : 1));
		}

		if (!(state.permissions & Permission::ACCESS_SURFACE_FLINGER) &&
		(state.flags & (eExplicitEarlyWakeupStart \| eExplicitEarlyWakeupEnd))) {
		ALOGE("Only WindowManager is allowed to use eExplicitEarlyWakeup[Start\|End] flags");
		state.flags &= ~(eExplicitEarlyWakeupStart \| eExplicitEarlyWakeupEnd);
		}
		mTransactionQueue.emplace(state);
		ATRACE_INT("TransactionQueue", mTransactionQueue.size());

		const auto schedule = [](uint32_t flags) {
		if (flags & eEarlyWakeup) return TransactionSchedule::Early;
		@@ -3474,28 +3472,23 @@ void SurfaceFlinger::queueTransaction(TransactionState state) {
		setTransactionFlags(eTransactionFlushNeeded, schedule);
		}

		void SurfaceFlinger::waitForSynchronousTransaction(bool synchronous, bool syncInput) {
		Mutex::Autolock _l(mStateLock);
		if (synchronous) {
		mTransactionPending = true;
		void SurfaceFlinger::waitForSynchronousTransaction(
		const CountDownLatch& transactionCommittedSignal) {
		// applyTransactionState is called on the main SF thread. While a given process may wish
		// to wait on synchronous transactions, the main SF thread should apply the transaction and
		// set the value to notify this after committed.
		if (!transactionCommittedSignal.wait_until(std::chrono::seconds(5))) {
		ALOGE("setTransactionState timed out!");
		}
		if (syncInput) {
		mPendingSyncInputWindows = true;
		}

		// applyTransactionState can be called by either the main SF thread or by
		// another process through setTransactionState. While a given process may wish
		// to wait on synchronous transactions, the main SF thread should never
		// be blocked. Therefore, we only wait if isMainThread is false.
		while (mTransactionPending \|\| mPendingSyncInputWindows) {
		status_t err = mTransactionCV.waitRelative(mStateLock, s2ns(5));
		if (CC_UNLIKELY(err != NO_ERROR)) {
		// just in case something goes wrong in SF, return to the
		// called after a few seconds.
		ALOGW_IF(err == TIMED_OUT, "setTransactionState timed out!");
		mTransactionPending = false;
		mPendingSyncInputWindows = false;
		break;
		void SurfaceFlinger::signalSynchronousTransactions() {
		for (auto it = mTransactionCommittedSignals.begin();
		it != mTransactionCommittedSignals.end();) {
		if ((*it)->countDown() == 0) {
		it = mTransactionCommittedSignals.erase(it);
		} else {
		it++;
		}
		}
		}
		@@ -3524,21 +3517,35 @@ status_t SurfaceFlinger::setTransactionState(
		permissions \|= Permission::ROTATE_SURFACE_FLINGER;
		}

		// TODO(b/159125966): Remove eEarlyWakeup completely as no client should use this flag
		if (flags & eEarlyWakeup) {
		ALOGW("eEarlyWakeup is deprecated. Use eExplicitEarlyWakeup[Start\|End]");
		}

		if (!(permissions & Permission::ACCESS_SURFACE_FLINGER) &&
		(flags & (eExplicitEarlyWakeupStart \| eExplicitEarlyWakeupEnd))) {
		ALOGE("Only WindowManager is allowed to use eExplicitEarlyWakeup[Start\|End] flags");
		flags &= ~(eExplicitEarlyWakeupStart \| eExplicitEarlyWakeupEnd);
		}

		const int64_t postTime = systemTime();

		IPCThreadState* ipc = IPCThreadState::self();
		const int originPid = ipc->getCallingPid();
		const int originUid = ipc->getCallingUid();
		TransactionState state{frameTimelineInfo, states,
		displays, flags,
		applyToken, inputWindowCommands,
		desiredPresentTime, isAutoTimestamp,
		uncacheBuffer, postTime,
		permissions, hasListenerCallbacks,
		listenerCallbacks, originPid,
		originUid, transactionId};
		queueTransaction(state);

		queueTransaction({frameTimelineInfo, states, displays, flags, applyToken, inputWindowCommands,
		desiredPresentTime, isAutoTimestamp, uncacheBuffer, postTime, permissions,
		hasListenerCallbacks, listenerCallbacks, originPid, originUid,
		transactionId});

		const bool synchronous = flags & eSynchronous;
		const bool syncInput = inputWindowCommands.syncInputWindows;
		if (synchronous \|\| syncInput) {
		waitForSynchronousTransaction(synchronous, syncInput);
		// Check the pending state to make sure the transaction is synchronous.
		if (state.transactionCommittedSignal) {
		waitForSynchronousTransaction(*state.transactionCommittedSignal);
		}

		return NO_ERROR;
		@@ -6078,10 +6085,7 @@ status_t SurfaceFlinger::renderScreenImplLocked(const RenderArea& renderArea,

		void SurfaceFlinger::setInputWindowsFinished() {
		Mutex::Autolock _l(mStateLock);

		mPendingSyncInputWindows = false;

		mTransactionCV.broadcast();
		signalSynchronousTransactions();
		}

		// ---------------------------------------------------------------------------

services/surfaceflinger/SurfaceFlinger.h

+40 −5

Original line number	Diff line number	Diff line
		@@ -474,6 +474,41 @@ private:
		hal::Connection connection = hal::Connection::INVALID;
		};

		class CountDownLatch {
		public:
		explicit CountDownLatch(int32_t count) : mCount(count) {}

		int32_t countDown() {
		std::unique_lock<std::mutex> lock(mMutex);
		if (mCount == 0) {
		return 0;
		}
		if (--mCount == 0) {
		mCountDownComplete.notify_all();
		}
		return mCount;
		}

		// Return true if triggered.
		bool wait_until(const std::chrono::seconds& timeout) const {
		std::unique_lock<std::mutex> lock(mMutex);
		const auto untilTime = std::chrono::system_clock::now() + timeout;
		while (mCount != 0) {
		// Conditional variables can be woken up sporadically, so we check count
		// to verify the wakeup was triggered by \|countDown\|.
		if (std::cv_status::timeout == mCountDownComplete.wait_until(lock, untilTime)) {
		return false;
		}
		}
		return true;
		}

		private:
		int32_t mCount;
		mutable std::condition_variable mCountDownComplete;
		mutable std::mutex mMutex;
		};

		struct TransactionState {
		TransactionState(const FrameTimelineInfo& frameTimelineInfo,
		const Vector<ComposerState>& composerStates,
		@@ -517,6 +552,7 @@ private:
		int originPid;
		int originUid;
		uint64_t id;
		std::shared_ptr<CountDownLatch> transactionCommittedSignal;
		};

		template <typename F, std::enable_if_t<!std::is_member_function_pointer_v<F>>* = nullptr>
		@@ -1080,8 +1116,9 @@ private:
		status_t CheckTransactCodeCredentials(uint32_t code);

		// Add transaction to the Transaction Queue
		void queueTransaction(TransactionState state) EXCLUDES(mQueueLock);
		void waitForSynchronousTransaction(bool synchronous, bool syncInput) EXCLUDES(mStateLock);
		void queueTransaction(TransactionState& state) EXCLUDES(mQueueLock);
		void waitForSynchronousTransaction(const CountDownLatch& transactionCommittedSignal);
		void signalSynchronousTransactions();

		/*
		* Generic Layer Metadata
		@@ -1111,8 +1148,7 @@ private:
		mutable Mutex mStateLock;
		State mCurrentState{LayerVector::StateSet::Current};
		std::atomic<int32_t> mTransactionFlags = 0;
		Condition mTransactionCV;
		bool mTransactionPending = false;
		std::vector<std::shared_ptr<CountDownLatch>> mTransactionCommittedSignals;
		bool mAnimTransactionPending = false;
		SortedVector<sp<Layer>> mLayersPendingRemoval;
		bool mForceTraversal = false;
		@@ -1324,7 +1360,6 @@ private:

		sp<SetInputWindowsListener> mSetInputWindowsListener;

		bool mPendingSyncInputWindows GUARDED_BY(mStateLock) = false;
		Hwc2::impl::PowerAdvisor mPowerAdvisor;

		// This should only be accessed on the main thread.

services/surfaceflinger/tests/utils/ScreenshotUtils.h

+1 −0

Original line number	Diff line number	Diff line
		@@ -85,6 +85,7 @@ public:

		void expectColor(const Rect& rect, const Color& color, uint8_t tolerance = 0) {
		ASSERT_NE(nullptr, mOutBuffer);
		ASSERT_NE(nullptr, mPixels);
		ASSERT_EQ(HAL_PIXEL_FORMAT_RGBA_8888, mOutBuffer->getPixelFormat());
		TransactionUtils::expectBufferColor(mOutBuffer, mPixels, rect, color, tolerance);
		}