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

Commit d600d57d authored by Marissa Wall's avatar Marissa Wall
Browse files

blast: in order no-op transaction callbacks 

Transactions callbacks were being sent as soon as they were ready
instead of in order. To fix this, keep a deque of Transaction
callbacks and do not send a callback until all the callbacks
before it have been sent.

Bug: 128519264
Test: Transaction_test

Change-Id: Ia363b3aca85bc1cd71d0fd915de79b44f786e09f
parent eccfc576

libs/gui/ITransactionCompletedListener.cpp

+14 −21
Original line number Diff line number Diff line
@@ -76,7 +76,11 @@ status_t SurfaceStats::readFromParcel(const Parcel* input) { 
}



status_t TransactionStats::writeToParcel(Parcel* output) const {

    status_t err = output->writeInt64(latchTime);

    status_t err = output->writeInt64Vector(callbackIds);

    if (err != NO_ERROR) {

        return err;

    }

    err = output->writeInt64(latchTime);

    if (err != NO_ERROR) {

        return err;

    }
@@ -96,7 +100,11 @@ status_t TransactionStats::writeToParcel(Parcel* output) const { 
}



status_t TransactionStats::readFromParcel(const Parcel* input) {

    status_t err = input->readInt64(&latchTime);

    status_t err = input->readInt64Vector(&callbackIds);

    if (err != NO_ERROR) {

        return err;

    }

    err = input->readInt64(&latchTime);

    if (err != NO_ERROR) {

        return err;

    }
@@ -120,16 +128,11 @@ status_t ListenerStats::writeToParcel(Parcel* output) const { 
    if (err != NO_ERROR) {

        return err;

    }



    for (const auto& [callbackIds, stats] : transactionStats) {

    for (const auto& stats : transactionStats) {

        err = output->writeParcelable(stats);

        if (err != NO_ERROR) {

            return err;

        }

        err = output->writeInt64Vector(callbackIds);

        if (err != NO_ERROR) {

            return err;

        }

    }

    return NO_ERROR;

}
@@ -139,18 +142,11 @@ status_t ListenerStats::readFromParcel(const Parcel* input) { 


    for (int i = 0; i < transactionStats_size; i++) {

        TransactionStats stats;

        std::vector<CallbackId> callbackIds;



        status_t err = input->readParcelable(&stats);

        if (err != NO_ERROR) {

            return err;

        }

        err = input->readInt64Vector(&callbackIds);

        if (err != NO_ERROR) {

            return err;

        }



        transactionStats.emplace(callbackIds, stats);

        transactionStats.push_back(stats);

    }

    return NO_ERROR;

}
@@ -159,11 +155,8 @@ ListenerStats ListenerStats::createEmpty(const sp<ITransactionCompletedListener> 
                                         const std::unordered_set<CallbackId>& callbackIds) {

    ListenerStats listenerStats;

    listenerStats.listener = listener;

    TransactionStats transactionStats;

    listenerStats.transactionStats.emplace(std::piecewise_construct,

                                           std::forward_as_tuple(callbackIds.begin(),

                                                                 callbackIds.end()),

                                           std::forward_as_tuple(transactionStats));

    listenerStats.transactionStats.emplace_back(callbackIds);



    return listenerStats;

}

libs/gui/SurfaceComposerClient.cpp

+4 −4
Original line number Diff line number Diff line
@@ -203,15 +203,15 @@ void TransactionCompletedListener::onTransactionCompleted(ListenerStats listener 
     * that could possibly exist for the callbacks.

     */

    std::unordered_map<sp<IBinder>, sp<SurfaceControl>, IBinderHash> surfaceControls;

    for (const auto& [callbackIds, transactionStats] : listenerStats.transactionStats) {

        for (auto callbackId : callbackIds) {

    for (const auto& transactionStats : listenerStats.transactionStats) {

        for (auto callbackId : transactionStats.callbackIds) {

            auto& [callbackFunction, callbackSurfaceControls] = mCallbacks[callbackId];

            surfaceControls.insert(callbackSurfaceControls.begin(), callbackSurfaceControls.end());

        }

    }



    for (const auto& [callbackIds, transactionStats] : listenerStats.transactionStats) {

        for (auto callbackId : callbackIds) {

    for (const auto& transactionStats : listenerStats.transactionStats) {

        for (auto callbackId : transactionStats.callbackIds) {

            auto& [callbackFunction, callbackSurfaceControls] = mCallbacks[callbackId];

            if (!callbackFunction) {

                ALOGE("cannot call null callback function, skipping");

libs/gui/include/gui/ITransactionCompletedListener.h

+10 −13
Original line number Diff line number Diff line
@@ -34,18 +34,6 @@ class ITransactionCompletedListener; 


using CallbackId = int64_t;



struct CallbackIdsHash {

    // CallbackId vectors have several properties that let us get away with this simple hash.

    // 1) CallbackIds are never 0 so if something has gone wrong and our CallbackId vector is

    // empty we can still hash 0.

    // 2) CallbackId vectors for the same listener either are identical or contain none of the

    // same members. It is sufficient to just check the first CallbackId in the vectors. If

    // they match, they are the same. If they do not match, they are not the same.

    std::size_t operator()(const std::vector<CallbackId> callbackIds) const {

        return std::hash<CallbackId>{}((callbackIds.size() == 0) ? 0 : callbackIds.front());

    }

};



class SurfaceStats : public Parcelable {

public:

    status_t writeToParcel(Parcel* output) const override;
@@ -65,6 +53,15 @@ public: 
    status_t writeToParcel(Parcel* output) const override;

    status_t readFromParcel(const Parcel* input) override;



    TransactionStats() = default;

    TransactionStats(const std::vector<CallbackId>& ids) : callbackIds(ids) {}

    TransactionStats(const std::unordered_set<CallbackId>& ids)

          : callbackIds(ids.begin(), ids.end()) {}

    TransactionStats(const std::vector<CallbackId>& ids, nsecs_t latch, const sp<Fence>& present,

                     const std::vector<SurfaceStats>& surfaces)

          : callbackIds(ids), latchTime(latch), presentFence(present), surfaceStats(surfaces) {}



    std::vector<CallbackId> callbackIds;

    nsecs_t latchTime = -1;

    sp<Fence> presentFence = nullptr;

    std::vector<SurfaceStats> surfaceStats;
@@ -79,7 +76,7 @@ public: 
                                     const std::unordered_set<CallbackId>& callbackIds);



    sp<ITransactionCompletedListener> listener;

    std::unordered_map<std::vector<CallbackId>, TransactionStats, CallbackIdsHash> transactionStats;

    std::vector<TransactionStats> transactionStats;

};



class ITransactionCompletedListener : public IInterface {

services/surfaceflinger/SurfaceFlinger.cpp

+10 −7
Original line number Diff line number Diff line
@@ -3651,18 +3651,22 @@ void SurfaceFlinger::applyTransactionState(const Vector<ComposerState>& states, 
        transactionFlags |= setDisplayStateLocked(display);

    }



    uint32_t clientStateFlags = 0;

    for (const ComposerState& state : states) {

        clientStateFlags |= setClientStateLocked(state, desiredPresentTime, listenerCallbacks,

                                                 postTime, privileged);

    }



    // In case the client has sent a Transaction that should receive callbacks but without any

    // SurfaceControls that should be included in the callback, send the listener and callbackIds

    // to the callback thread so it can send an empty callback

    if (!listenerCallbacks.empty()) {

        mTransactionCompletedThread.run();

    }

    for (const auto& [listener, callbackIds] : listenerCallbacks) {

        mTransactionCompletedThread.addCallback(listener, callbackIds);

    }



    uint32_t clientStateFlags = 0;

    for (const ComposerState& state : states) {

        clientStateFlags |= setClientStateLocked(state, desiredPresentTime, listenerCallbacks,

                                                 postTime, privileged);

    }



    // If the state doesn't require a traversal and there are callbacks, send them now

    if (!(clientStateFlags & eTraversalNeeded)) {

        mTransactionCompletedThread.sendCallbacks();
@@ -4019,7 +4023,6 @@ uint32_t SurfaceFlinger::setClientStateLocked( 
    }

    std::vector<sp<CallbackHandle>> callbackHandles;

    if ((what & layer_state_t::eHasListenerCallbacksChanged) && (!listenerCallbacks.empty())) {

        mTransactionCompletedThread.run();

        for (const auto& [listener, callbackIds] : listenerCallbacks) {

            callbackHandles.emplace_back(new CallbackHandle(listener, callbackIds, s.surface));

        }

services/surfaceflinger/TransactionCompletedThread.cpp

+129 −69
Original line number Diff line number Diff line
@@ -29,6 +29,18 @@ 


namespace android {



// Returns 0 if they are equal

//         <0 if the first id that doesn't match is lower in c2 or all ids match but c2 is shorter

//         >0 if the first id that doesn't match is greater in c2 or all ids match but c2 is longer

//

// See CallbackIdsHash for a explaniation of why this works

static int compareCallbackIds(const std::vector<CallbackId>& c1, const std::vector<CallbackId> c2) {

    if (c1.empty()) {

        return !c2.empty();

    }

    return c1.front() - c2.front();

}



TransactionCompletedThread::~TransactionCompletedThread() {

    std::lock_guard lockThread(mThreadMutex);
@@ -44,8 +56,8 @@ TransactionCompletedThread::~TransactionCompletedThread() { 


    {

        std::lock_guard lock(mMutex);

        for (const auto& [listener, listenerStats] : mListenerStats) {

            listener->unlinkToDeath(mDeathRecipient);

        for (const auto& [listener, transactionStats] : mCompletedTransactions) {

            IInterface::asBinder(listener)->unlinkToDeath(mDeathRecipient);

        }

    }

}
@@ -62,21 +74,59 @@ void TransactionCompletedThread::run() { 
    mThread = std::thread(&TransactionCompletedThread::threadMain, this);

}



void TransactionCompletedThread::registerPendingCallbackHandle(const sp<CallbackHandle>& handle) {

status_t TransactionCompletedThread::addCallback(const sp<ITransactionCompletedListener>& listener,

                                                 const std::vector<CallbackId>& callbackIds) {

    std::lock_guard lock(mMutex);

    if (!mRunning) {

        ALOGE("cannot add callback because the callback thread isn't running");

        return BAD_VALUE;

    }



    if (mCompletedTransactions.count(listener) == 0) {

        status_t err = IInterface::asBinder(listener)->linkToDeath(mDeathRecipient);

        if (err != NO_ERROR) {

            ALOGE("cannot add callback because linkToDeath failed, err: %d", err);

            return err;

        }

    }



    auto& transactionStatsDeque = mCompletedTransactions[listener];

    transactionStatsDeque.emplace_back(callbackIds);

    return NO_ERROR;

}



status_t TransactionCompletedThread::registerPendingCallbackHandle(

        const sp<CallbackHandle>& handle) {

    std::lock_guard lock(mMutex);

    if (!mRunning) {

        ALOGE("cannot register callback handle because the callback thread isn't running");

        return BAD_VALUE;

    }



    sp<IBinder> listener = IInterface::asBinder(handle->listener);

    const auto& callbackIds = handle->callbackIds;

    // If we can't find the transaction stats something has gone wrong. The client should call

    // addCallback before trying to register a pending callback handle.

    TransactionStats* transactionStats;

    status_t err = findTransactionStats(handle->listener, handle->callbackIds, &transactionStats);

    if (err != NO_ERROR) {

        ALOGE("cannot find transaction stats");

        return err;

    }



    mPendingTransactions[listener][callbackIds]++;

    mPendingTransactions[handle->listener][handle->callbackIds]++;

    return NO_ERROR;

}



void TransactionCompletedThread::addPresentedCallbackHandles(

status_t TransactionCompletedThread::addPresentedCallbackHandles(

        const std::deque<sp<CallbackHandle>>& handles) {

    std::lock_guard lock(mMutex);

    if (!mRunning) {

        ALOGE("cannot add presented callback handle because the callback thread isn't running");

        return BAD_VALUE;

    }



    for (const auto& handle : handles) {

        auto listener = mPendingTransactions.find(IInterface::asBinder(handle->listener));

        auto listener = mPendingTransactions.find(handle->listener);

        if (listener != mPendingTransactions.end()) {

            auto& pendingCallbacks = listener->second;

            auto pendingCallback = pendingCallbacks.find(handle->callbackIds);
@@ -88,59 +138,63 @@ void TransactionCompletedThread::addPresentedCallbackHandles( 
                    pendingCallbacks.erase(pendingCallback);

                }

            } else {

            ALOGE("there are more latched callbacks than there were registered callbacks");

                ALOGW("there are more latched callbacks than there were registered callbacks");

            }

        } else {

            ALOGW("cannot find listener in mPendingTransactions");

        }



        addCallbackHandle(handle);

        status_t err = addCallbackHandle(handle);

        if (err != NO_ERROR) {

            ALOGE("could not add callback handle");

            return err;

        }

    }



void TransactionCompletedThread::addUnpresentedCallbackHandle(const sp<CallbackHandle>& handle) {

    std::lock_guard lock(mMutex);

    addCallbackHandle(handle);

    return NO_ERROR;

}



void TransactionCompletedThread::addCallback(

        const sp<ITransactionCompletedListener>& transactionListener,

        const std::vector<CallbackId>& callbackIds) {

status_t TransactionCompletedThread::addUnpresentedCallbackHandle(

        const sp<CallbackHandle>& handle) {

    std::lock_guard lock(mMutex);

    addCallbackLocked(transactionListener, callbackIds);

    if (!mRunning) {

        ALOGE("cannot add unpresented callback handle because the callback thread isn't running");

        return BAD_VALUE;

    }



status_t TransactionCompletedThread::addCallbackHandle(const sp<CallbackHandle>& handle) {

    status_t err = addCallbackLocked(handle->listener, handle->callbackIds);

    if (err != NO_ERROR) {

        ALOGE("cannot add callback, err: %d", err);

        return err;

    return addCallbackHandle(handle);

}



    const sp<IBinder> listener = IInterface::asBinder(handle->listener);

    auto& listenerStats = mListenerStats[listener];

    auto& transactionStats = listenerStats.transactionStats[handle->callbackIds];

status_t TransactionCompletedThread::findTransactionStats(

        const sp<ITransactionCompletedListener>& listener,

        const std::vector<CallbackId>& callbackIds, TransactionStats** outTransactionStats) {

    auto& transactionStatsDeque = mCompletedTransactions[listener];



    transactionStats.latchTime = handle->latchTime;

    transactionStats.surfaceStats.emplace_back(handle->surfaceControl, handle->acquireTime,

                                               handle->previousReleaseFence);

    // Search back to front because the most recent transactions are at the back of the deque

    auto itr = transactionStatsDeque.rbegin();

    for (; itr != transactionStatsDeque.rend(); itr++) {

        if (compareCallbackIds(itr->callbackIds, callbackIds) == 0) {

            *outTransactionStats = &(*itr);

            return NO_ERROR;

        }

    }



status_t TransactionCompletedThread::addCallbackLocked(

        const sp<ITransactionCompletedListener>& transactionListener,

        const std::vector<CallbackId>& callbackIds) {

    const sp<IBinder> listener = IInterface::asBinder(transactionListener);

    // If we don't already have a reference to this listener, linkToDeath so we get a notification

    // if it dies.

    if (mListenerStats.count(listener) == 0) {

        status_t err = listener->linkToDeath(mDeathRecipient);

    ALOGE("could not find transaction stats");

    return BAD_VALUE;

}



status_t TransactionCompletedThread::addCallbackHandle(const sp<CallbackHandle>& handle) {

    // If we can't find the transaction stats something has gone wrong. The client should call

    // addCallback before trying to add a presnted callback handle.

    TransactionStats* transactionStats;

    status_t err = findTransactionStats(handle->listener, handle->callbackIds, &transactionStats);

    if (err != NO_ERROR) {

            ALOGE("cannot add callback because linkToDeath failed, err: %d", err);

        return err;

    }

    }



    auto& listenerStats = mListenerStats[listener];

    listenerStats.listener = transactionListener;

    listenerStats.transactionStats[callbackIds];

    transactionStats->latchTime = handle->latchTime;

    transactionStats->surfaceStats.emplace_back(handle->surfaceControl, handle->acquireTime,

                                                handle->previousReleaseFence);

    return NO_ERROR;

}
@@ -163,40 +217,46 @@ void TransactionCompletedThread::threadMain() { 
        mConditionVariable.wait(mMutex);



        // For each listener

        auto it = mListenerStats.begin();

        while (it != mListenerStats.end()) {

            auto& [listener, listenerStats] = *it;

        auto completedTransactionsItr = mCompletedTransactions.begin();

        while (completedTransactionsItr != mCompletedTransactions.end()) {

            auto& [listener, transactionStatsDeque] = *completedTransactionsItr;

            ListenerStats listenerStats;

            listenerStats.listener = listener;



            // For each transaction

            bool sendCallback = true;

            for (auto& [callbackIds, transactionStats] : listenerStats.transactionStats) {

                // If we are still waiting on the callback handles for this transaction, skip it

                if (mPendingTransactions[listener].count(callbackIds) != 0) {

                    sendCallback = false;

            auto transactionStatsItr = transactionStatsDeque.begin();

            while (transactionStatsItr != transactionStatsDeque.end()) {

                auto& transactionStats = *transactionStatsItr;



                // If we are still waiting on the callback handles for this transaction, stop

                // here because all transaction callbacks for the same listener must come in order

                if (mPendingTransactions[listener].count(transactionStats.callbackIds) != 0) {

                    break;

                }



                // If the transaction has been latched

                if (transactionStats.latchTime >= 0) {

                    if (!mPresentFence) {

                        sendCallback = false;

                        break;

                    }

                    transactionStats.presentFence = mPresentFence;

                }



                // Remove the transaction from completed to the callback

                listenerStats.transactionStats.push_back(std::move(transactionStats));

                transactionStatsItr = transactionStatsDeque.erase(transactionStatsItr);

            }

            // If the listener has no pending transactions and all latched transactions have been

            // presented

            if (sendCallback) {

            // If the listener has completed transactions

            if (!listenerStats.transactionStats.empty()) {

                // If the listener is still alive

                if (listener->isBinderAlive()) {

                if (IInterface::asBinder(listener)->isBinderAlive()) {

                    // Send callback

                    listenerStats.listener->onTransactionCompleted(listenerStats);

                    listener->unlinkToDeath(mDeathRecipient);

                    IInterface::asBinder(listener)->unlinkToDeath(mDeathRecipient);

                }

                it = mListenerStats.erase(it);

                completedTransactionsItr = mCompletedTransactions.erase(completedTransactionsItr);

            } else {

                it++;

                completedTransactionsItr++;

            }

        }