Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit f1b7c7be authored by arthurhung's avatar arthurhung
Browse files

Prevent waiting synchronous case if transaction applied first

Currently, transaction would be queued and apply in the main thread,
if a given transaction would wish to apply synchronously. We may wait
it in binder thread until the transaction has been applied, but if
'applyTransactionState' comes first, it would cause redundant waiting.

To prevent this, we use the CountDownLatch in transaction to wait the
synchronous state, it will wait until transactions applied and committed.

Bug: 181480169
Test: libsurfaceflinger_unittest SurfaceFlinger_test libgui_test
Change-Id: I1fe56296810cdeadeae5b37ae9b4bd00d9e689c3
parent 870da004
Loading
Loading
Loading
Loading
+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();
}

// ---------------------------------------------------------------------------
+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.
+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);
    }