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

Commit 1088bbf9 authored by Patrick Williams's avatar Patrick Williams Committed by Android (Google) Code Review
Browse files

Revert "Cache and uncache buffers in the same transaction" 

This reverts commit 75ce1ea0.

Reason for revert: b/266481887

Change-Id: I147947d55672c8f14d9bbe51df54eadfc43edeb2
parent 75ce1ea0

libs/gui/ISurfaceComposer.cpp

+8 −15
Original line number Diff line number Diff line
@@ -63,8 +63,7 @@ public: 
                                 Vector<ComposerState>& state, const Vector<DisplayState>& displays,

                                 uint32_t flags, const sp<IBinder>& applyToken,

                                 const InputWindowCommands& commands, int64_t desiredPresentTime,

                                 bool isAutoTimestamp,

                                 const std::vector<client_cache_t>& uncacheBuffers,

                                 bool isAutoTimestamp, const client_cache_t& uncacheBuffer,

                                 bool hasListenerCallbacks,

                                 const std::vector<ListenerCallbacks>& listenerCallbacks,

                                 uint64_t transactionId) override {
@@ -88,11 +87,8 @@ public: 
        SAFE_PARCEL(commands.write, data);

        SAFE_PARCEL(data.writeInt64, desiredPresentTime);

        SAFE_PARCEL(data.writeBool, isAutoTimestamp);

        SAFE_PARCEL(data.writeUint32, static_cast<uint32_t>(uncacheBuffers.size()));

        for (const client_cache_t& uncacheBuffer : uncacheBuffers) {

        SAFE_PARCEL(data.writeStrongBinder, uncacheBuffer.token.promote());

        SAFE_PARCEL(data.writeUint64, uncacheBuffer.id);

        }

        SAFE_PARCEL(data.writeBool, hasListenerCallbacks);



        SAFE_PARCEL(data.writeVectorSize, listenerCallbacks);
@@ -162,14 +158,11 @@ status_t BnSurfaceComposer::onTransact( 
            SAFE_PARCEL(data.readInt64, &desiredPresentTime);

            SAFE_PARCEL(data.readBool, &isAutoTimestamp);



            SAFE_PARCEL_READ_SIZE(data.readUint32, &count, data.dataSize());

            std::vector<client_cache_t> uncacheBuffers(count);

            client_cache_t uncachedBuffer;

            sp<IBinder> tmpBinder;

            for (size_t i = 0; i < count; i++) {

            SAFE_PARCEL(data.readNullableStrongBinder, &tmpBinder);

                uncacheBuffers[i].token = tmpBinder;

                SAFE_PARCEL(data.readUint64, &uncacheBuffers[i].id);

            }

            uncachedBuffer.token = tmpBinder;

            SAFE_PARCEL(data.readUint64, &uncachedBuffer.id);



            bool hasListenerCallbacks = false;

            SAFE_PARCEL(data.readBool, &hasListenerCallbacks);
@@ -189,7 +182,7 @@ status_t BnSurfaceComposer::onTransact( 


            return setTransactionState(frameTimelineInfo, state, displays, stateFlags, applyToken,

                                       inputWindowCommands, desiredPresentTime, isAutoTimestamp,

                                       uncacheBuffers, hasListenerCallbacks, listenerCallbacks,

                                       uncachedBuffer, hasListenerCallbacks, listenerCallbacks,

                                       transactionId);

        }

        default: {

libs/gui/SurfaceComposerClient.cpp

+17 −45
Original line number Diff line number Diff line
@@ -565,13 +565,11 @@ public: 
        return NO_ERROR;

    }



    uint64_t cache(const sp<GraphicBuffer>& buffer,

                   std::optional<client_cache_t>& outUncacheBuffer) {

    uint64_t cache(const sp<GraphicBuffer>& buffer) {

        std::lock_guard<std::mutex> lock(mMutex);



        if (mBuffers.size() >= BUFFER_CACHE_MAX_SIZE) {

            outUncacheBuffer = findLeastRecentlyUsedBuffer();

            mBuffers.erase(outUncacheBuffer->id);

            evictLeastRecentlyUsedBuffer();

        }



        buffer->addDeathCallback(removeDeadBufferCallback, nullptr);
@@ -582,13 +580,16 @@ public: 


    void uncache(uint64_t cacheId) {

        std::lock_guard<std::mutex> lock(mMutex);

        if (mBuffers.erase(cacheId)) {

            SurfaceComposerClient::doUncacheBufferTransaction(cacheId);

        uncacheLocked(cacheId);

    }



    void uncacheLocked(uint64_t cacheId) REQUIRES(mMutex) {

        mBuffers.erase(cacheId);

        SurfaceComposerClient::doUncacheBufferTransaction(cacheId);

    }



private:

    client_cache_t findLeastRecentlyUsedBuffer() REQUIRES(mMutex) {

    void evictLeastRecentlyUsedBuffer() REQUIRES(mMutex) {

        auto itr = mBuffers.begin();

        uint64_t minCounter = itr->second;

        auto minBuffer = itr;
@@ -602,8 +603,7 @@ private: 
            }

            itr++;

        }



        return {.token = getToken(), .id = minBuffer->first};

        uncacheLocked(minBuffer->first);

    }



    uint64_t getCounter() REQUIRES(mMutex) {
@@ -741,18 +741,6 @@ status_t SurfaceComposerClient::Transaction::readFromParcel(const Parcel* parcel 
    InputWindowCommands inputWindowCommands;

    inputWindowCommands.read(*parcel);



    count = static_cast<size_t>(parcel->readUint32());

    if (count > parcel->dataSize()) {

        return BAD_VALUE;

    }

    std::vector<client_cache_t> uncacheBuffers(count);

    for (size_t i = 0; i < count; i++) {

        sp<IBinder> tmpBinder;

        SAFE_PARCEL(parcel->readStrongBinder, &tmpBinder);

        uncacheBuffers[i].token = tmpBinder;

        SAFE_PARCEL(parcel->readUint64, &uncacheBuffers[i].id);

    }



    // Parsing was successful. Update the object.

    mId = transactionId;

    mTransactionNestCount = transactionNestCount;
@@ -767,7 +755,6 @@ status_t SurfaceComposerClient::Transaction::readFromParcel(const Parcel* parcel 
    mComposerStates = composerStates;

    mInputWindowCommands = inputWindowCommands;

    mApplyToken = applyToken;

    mUncacheBuffers = std::move(uncacheBuffers);

    return NO_ERROR;

}
@@ -819,13 +806,6 @@ status_t SurfaceComposerClient::Transaction::writeToParcel(Parcel* parcel) const 
    }



    mInputWindowCommands.write(*parcel);



    SAFE_PARCEL(parcel->writeUint32, static_cast<uint32_t>(mUncacheBuffers.size()));

    for (const client_cache_t& uncacheBuffer : mUncacheBuffers) {

        SAFE_PARCEL(parcel->writeStrongBinder, uncacheBuffer.token.promote());

        SAFE_PARCEL(parcel->writeUint64, uncacheBuffer.id);

    }



    return NO_ERROR;

}
@@ -893,10 +873,6 @@ SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::merge(Tr 
        }

    }



    for (const auto& cacheId : other.mUncacheBuffers) {

        mUncacheBuffers.push_back(cacheId);

    }



    mInputWindowCommands.merge(other.mInputWindowCommands);



    mMayContainBuffer |= other.mMayContainBuffer;
@@ -915,7 +891,6 @@ void SurfaceComposerClient::Transaction::clear() { 
    mDisplayStates.clear();

    mListenerCallbacks.clear();

    mInputWindowCommands.clear();

    mUncacheBuffers.clear();

    mMayContainBuffer = false;

    mTransactionNestCount = 0;

    mAnimation = false;
@@ -938,10 +913,10 @@ void SurfaceComposerClient::doUncacheBufferTransaction(uint64_t cacheId) { 
    uncacheBuffer.token = BufferCache::getInstance().getToken();

    uncacheBuffer.id = cacheId;

    Vector<ComposerState> composerStates;

    status_t status = sf->setTransactionState(FrameTimelineInfo{}, composerStates, {},

                                              ISurfaceComposer::eOneWay,

                                              Transaction::getDefaultApplyToken(), {}, systemTime(),

                                              true, {uncacheBuffer}, false, {}, generateId());

    status_t status =

            sf->setTransactionState(FrameTimelineInfo{}, composerStates, {},

                                    ISurfaceComposer::eOneWay, Transaction::getDefaultApplyToken(),

                                    {}, systemTime(), true, uncacheBuffer, false, {}, generateId());

    if (status != NO_ERROR) {

        ALOGE_AND_TRACE("SurfaceComposerClient::doUncacheBufferTransaction - %s",

                        strerror(-status));
@@ -979,11 +954,7 @@ void SurfaceComposerClient::Transaction::cacheBuffers() { 
            s->bufferData->buffer = nullptr;

        } else {

            // Cache-miss. Include the buffer and send the new cacheId.

            std::optional<client_cache_t> uncacheBuffer;

            cacheId = BufferCache::getInstance().cache(s->bufferData->buffer, uncacheBuffer);

            if (uncacheBuffer) {

                mUncacheBuffers.push_back(*uncacheBuffer);

            }

            cacheId = BufferCache::getInstance().cache(s->bufferData->buffer);

        }

        s->bufferData->flags |= BufferData::BufferDataChange::cachedBufferChanged;

        s->bufferData->cachedBuffer.token = BufferCache::getInstance().getToken();
@@ -1116,7 +1087,8 @@ status_t SurfaceComposerClient::Transaction::apply(bool synchronous, bool oneWay 
    sp<ISurfaceComposer> sf(ComposerService::getComposerService());

    sf->setTransactionState(mFrameTimelineInfo, composerStates, displayStates, flags, applyToken,

                            mInputWindowCommands, mDesiredPresentTime, mIsAutoTimestamp,

                            mUncacheBuffers, hasListenerCallbacks, listenerCallbacks, mId);

                            {} /*uncacheBuffer - only set in doUncacheBufferTransaction*/,

                            hasListenerCallbacks, listenerCallbacks, mId);

    mId = generateId();



    // Clear the current states and flags

libs/gui/include/gui/ISurfaceComposer.h

+2 −3
Original line number Diff line number Diff line
@@ -113,9 +113,8 @@ public: 
            const FrameTimelineInfo& frameTimelineInfo, Vector<ComposerState>& state,

            const Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,

            const InputWindowCommands& inputWindowCommands, int64_t desiredPresentTime,

            bool isAutoTimestamp, const std::vector<client_cache_t>& uncacheBuffer,

            bool hasListenerCallbacks, const std::vector<ListenerCallbacks>& listenerCallbacks,

            uint64_t transactionId) = 0;

            bool isAutoTimestamp, const client_cache_t& uncacheBuffer, bool hasListenerCallbacks,

            const std::vector<ListenerCallbacks>& listenerCallbacks, uint64_t transactionId) = 0;

};



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

libs/gui/include/gui/SurfaceComposerClient.h

+0 −1
Original line number Diff line number Diff line
@@ -402,7 +402,6 @@ public: 
        SortedVector<DisplayState> mDisplayStates;

        std::unordered_map<sp<ITransactionCompletedListener>, CallbackInfo, TCLHash>

                mListenerCallbacks;

        std::vector<client_cache_t> mUncacheBuffers;



        uint64_t mId;

libs/gui/tests/Surface_test.cpp

+1 −1
Original line number Diff line number Diff line
@@ -701,7 +701,7 @@ public: 
                                 const sp<IBinder>& /*applyToken*/,

                                 const InputWindowCommands& /*inputWindowCommands*/,

                                 int64_t /*desiredPresentTime*/, bool /*isAutoTimestamp*/,

                                 const std::vector<client_cache_t>& /*cachedBuffer*/,

                                 const client_cache_t& /*cachedBuffer*/,

                                 bool /*hasListenerCallbacks*/,

                                 const std::vector<ListenerCallbacks>& /*listenerCallbacks*/,

                                 uint64_t /*transactionId*/) override {