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

Commit 78b7220f authored by Marissa Wall's avatar Marissa Wall
Browse files

blast: drop buffer from SF's cache when destroyed 

When an app drops its reference to an AHardwareBuffer, the buffer
should be removed from the client and SurfaceFlinger caches.

Ideally, both caches would have wp to the buffer and the buffer
would automatically be removed from the cache.

Unfortunately, GraphicBuffers are refcounted per process. If SurfaceFlinger
just has a wp to the GraphicBuffer, the buffer's destructor will
be called and SurfaceFlinger will lose access to the buffer.
SurfaceFlinger can't just hold onto a sp to the buffer because
then the buffer wouldn't be destoryed when the app drops its reference.

Instead, when the app process drops its last strong reference,
GraphicBuffer will send a callback to the client side cache.
The cache will send a Transaction to SurfaceFlinger to drop its sp
to the buffer.

Bug: 127689853
Test: SurfaceFlinger_test

Change-Id: I2182578ed33d7c731945cb88cd1decb2892266b0
parent 4c6f904f

libs/gui/ISurfaceComposer.cpp

+10 −2
Original line number Diff line number Diff line
@@ -68,7 +68,8 @@ public: 
                                     const Vector<DisplayState>& displays, uint32_t flags,

                                     const sp<IBinder>& applyToken,

                                     const InputWindowCommands& commands,

                                     int64_t desiredPresentTime) {

                                     int64_t desiredPresentTime,

                                     const cached_buffer_t& uncacheBuffer) {

        Parcel data, reply;

        data.writeInterfaceToken(ISurfaceComposer::getInterfaceDescriptor());
@@ -86,6 +87,8 @@ public: 
        data.writeStrongBinder(applyToken);

        commands.write(data);

        data.writeInt64(desiredPresentTime);

        data.writeStrongBinder(uncacheBuffer.token);

        data.writeUint64(uncacheBuffer.cacheId);

        remote()->transact(BnSurfaceComposer::SET_TRANSACTION_STATE, data, &reply);

    }
@@ -970,8 +973,13 @@ status_t BnSurfaceComposer::onTransact( 
            inputWindowCommands.read(data);



            int64_t desiredPresentTime = data.readInt64();



            cached_buffer_t uncachedBuffer;

            uncachedBuffer.token = data.readStrongBinder();

            uncachedBuffer.cacheId = data.readUint64();



            setTransactionState(state, displays, stateFlags, applyToken, inputWindowCommands,

                                desiredPresentTime);

                                desiredPresentTime, uncachedBuffer);

            return NO_ERROR;

        }

        case BOOT_FINISHED: {

libs/gui/LayerState.cpp

+2 −2
Original line number Diff line number Diff line
@@ -95,7 +95,7 @@ status_t layer_state_t::write(Parcel& output) const 
    }



    output.writeStrongBinder(cachedBuffer.token);

    output.writeInt32(cachedBuffer.bufferId);

    output.writeUint64(cachedBuffer.cacheId);

    output.writeParcelable(metadata);



    output.writeFloat(bgColorAlpha);
@@ -173,7 +173,7 @@ status_t layer_state_t::read(const Parcel& input) 
    }



    cachedBuffer.token = input.readStrongBinder();

    cachedBuffer.bufferId = input.readInt32();

    cachedBuffer.cacheId = input.readUint64();

    input.readParcelable(&metadata);



    bgColorAlpha = input.readFloat();

libs/gui/SurfaceComposerClient.cpp

+90 −72
Original line number Diff line number Diff line
@@ -233,6 +233,8 @@ void TransactionCompletedListener::onTransactionCompleted(ListenerStats listener 


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



void bufferCacheCallback(void* /*context*/, uint64_t graphicBufferId);



class BufferCache : public Singleton<BufferCache> {

public:

    BufferCache() : token(new BBinder()) {}
@@ -241,77 +243,57 @@ public: 
        return IInterface::asBinder(TransactionCompletedListener::getIInstance());

    }



    int32_t getId(const sp<GraphicBuffer>& buffer) {

    status_t getCacheId(const sp<GraphicBuffer>& buffer, uint64_t* cacheId) {

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



        auto itr = mBuffers.find(buffer);

        auto itr = mBuffers.find(buffer->getId());

        if (itr == mBuffers.end()) {

            return -1;

            return BAD_VALUE;

        }

        itr->second.counter = getCounter();

        return itr->second.id;

        itr->second = getCounter();

        *cacheId = buffer->getId();

        return NO_ERROR;

    }



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

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

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



        int32_t bufferId = getNextAvailableId();



        mBuffers[buffer].id = bufferId;

        mBuffers[buffer].counter = getCounter();

        return bufferId;

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

            evictLeastRecentlyUsedBuffer();

        }



private:

    int32_t evictDestroyedBuffer() REQUIRES(mMutex) {

        auto itr = mBuffers.begin();

        while (itr != mBuffers.end()) {

            auto& buffer = itr->first;

            if (buffer == nullptr || buffer.promote() == nullptr) {

                int32_t bufferId = itr->second.id;

                mBuffers.erase(itr);

                return bufferId;

            }

            itr++;

        buffer->addDeathCallback(bufferCacheCallback, nullptr);



        mBuffers[buffer->getId()] = getCounter();

        return buffer->getId();

    }

        return -1;



    void uncache(uint64_t cacheId) {

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

        uncacheLocked(cacheId);

    }



    int32_t evictLeastRecentlyUsedBuffer() REQUIRES(mMutex) {

        if (mBuffers.size() < 0) {

            return -1;

    void uncacheLocked(uint64_t cacheId) REQUIRES(mMutex) {

        mBuffers.erase(cacheId);

        SurfaceComposerClient::doUncacheBufferTransaction(cacheId);

    }



private:

    void evictLeastRecentlyUsedBuffer() REQUIRES(mMutex) {

        auto itr = mBuffers.begin();

        uint64_t minCounter = itr->second.counter;

        uint64_t minCounter = itr->second;

        auto minBuffer = itr;

        itr++;



        while (itr != mBuffers.end()) {

            uint64_t counter = itr->second.counter;

            uint64_t counter = itr->second;

            if (counter < minCounter) {

                minCounter = counter;

                minBuffer = itr;

            }

            itr++;

        }

        int32_t minBufferId = minBuffer->second.id;

        mBuffers.erase(minBuffer);

        return minBufferId;

    }



    int32_t getNextAvailableId() REQUIRES(mMutex) {

        static int32_t id = 0;

        if (id + 1 < BUFFER_CACHE_MAX_SIZE) {

            return id++;

        }



        // There are no more valid cache ids. To set additional buffers, evict existing buffers

        // and reuse their cache ids.

        int32_t bufferId = evictDestroyedBuffer();

        if (bufferId > 0) {

            return bufferId;

        }

        return evictLeastRecentlyUsedBuffer();

        uncacheLocked(minBuffer->first);

    }



    uint64_t getCounter() REQUIRES(mMutex) {
@@ -319,18 +301,8 @@ private: 
        return counter++;

    }



    struct Metadata {

        // The cache id of a buffer that can be set to ISurfaceComposer. When ISurfaceComposer

        // recieves this id, it can retrieve the buffer from its cache. Caching GraphicBuffers

        // is important because sending them across processes is expensive.

        int32_t id = 0;

        // When a buffer is set, a counter is incremented and stored in the cache's metadata.

        // When an buffer must be evicted, the entry with the lowest counter value is chosen.

        uint64_t counter = 0;

    };



    std::mutex mMutex;

    std::map<wp<GraphicBuffer>, Metadata> mBuffers GUARDED_BY(mMutex);

    std::map<uint64_t /*Cache id*/, uint64_t /*counter*/> mBuffers GUARDED_BY(mMutex);



    // Used by ISurfaceComposer to identify which process is sending the cached buffer.

    sp<IBinder> token;
@@ -338,6 +310,11 @@ private: 


ANDROID_SINGLETON_STATIC_INSTANCE(BufferCache);



void bufferCacheCallback(void* /*context*/, uint64_t graphicBufferId) {

    // GraphicBuffer id's are used as the cache ids.

    BufferCache::getInstance().uncache(graphicBufferId);

}



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



SurfaceComposerClient::Transaction::Transaction(const Transaction& other)
@@ -385,6 +362,9 @@ SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::merge(Tr 
    mInputWindowCommands.merge(other.mInputWindowCommands);

    other.mInputWindowCommands.clear();



    mContainsBuffer = other.mContainsBuffer;

    other.mContainsBuffer = false;



    return *this;

}
@@ -401,7 +381,50 @@ void SurfaceComposerClient::doDropReferenceTransaction(const sp<IBinder>& handle 
    s.state.parentHandleForChild = nullptr;



    composerStates.add(s);

    sf->setTransactionState(composerStates, displayStates, 0, nullptr, {}, -1);

    sf->setTransactionState(composerStates, displayStates, 0, nullptr, {}, -1, {});

}



void SurfaceComposerClient::doUncacheBufferTransaction(uint64_t cacheId) {

    sp<ISurfaceComposer> sf(ComposerService::getComposerService());



    cached_buffer_t uncacheBuffer;

    uncacheBuffer.token = BufferCache::getInstance().getToken();

    uncacheBuffer.cacheId = cacheId;



    sf->setTransactionState({}, {}, 0, nullptr, {}, -1, uncacheBuffer);

}



void SurfaceComposerClient::Transaction::cacheBuffers() {

    if (!mContainsBuffer) {

        return;

    }



    size_t count = 0;

    for (auto& [sc, cs] : mComposerStates) {

        layer_state_t* s = getLayerState(sc);

        if (!(s->what & layer_state_t::eBufferChanged)) {

            continue;

        }



        uint64_t cacheId = 0;

        status_t ret = BufferCache::getInstance().getCacheId(s->buffer, &cacheId);

        if (ret == NO_ERROR) {

            s->what &= ~static_cast<uint32_t>(layer_state_t::eBufferChanged);

            s->buffer = nullptr;

        } else {

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

        }

        s->what |= layer_state_t::eCachedBufferChanged;

        s->cachedBuffer.token = BufferCache::getInstance().getToken();

        s->cachedBuffer.cacheId = cacheId;



        // If we have more buffers than the size of the cache, we should stop caching so we don't

        // evict other buffers in this transaction

        count++;

        if (count >= BUFFER_CACHE_MAX_SIZE) {

            break;

        }

    }

}



status_t SurfaceComposerClient::Transaction::apply(bool synchronous) {
@@ -437,6 +460,8 @@ status_t SurfaceComposerClient::Transaction::apply(bool synchronous) { 
    }

    mListenerCallbacks.clear();



    cacheBuffers();



    Vector<ComposerState> composerStates;

    Vector<DisplayState> displayStates;

    uint32_t flags = 0;
@@ -468,7 +493,8 @@ status_t SurfaceComposerClient::Transaction::apply(bool synchronous) { 


    sp<IBinder> applyToken = IInterface::asBinder(TransactionCompletedListener::getIInstance());

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

                            mDesiredPresentTime);

                            mDesiredPresentTime,

                            {} /*uncacheBuffer - only set in doUncacheBufferTransaction*/);

    mInputWindowCommands.clear();

    mStatus = NO_ERROR;

    return NO_ERROR;
@@ -882,20 +908,12 @@ SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setBuffe 
        mStatus = BAD_INDEX;

        return *this;

    }



    int32_t bufferId = BufferCache::getInstance().getId(buffer);

    if (bufferId < 0) {

        bufferId = BufferCache::getInstance().cache(buffer);



    s->what |= layer_state_t::eBufferChanged;

    s->buffer = buffer;

    }



    s->what |= layer_state_t::eCachedBufferChanged;

    s->cachedBuffer.token = BufferCache::getInstance().getToken();

    s->cachedBuffer.bufferId = bufferId;



    registerSurfaceControlForCallback(sc);



    mContainsBuffer = true;

    return *this;

}

libs/gui/include/gui/ISurfaceComposer.h

+3 −1
Original line number Diff line number Diff line
@@ -40,6 +40,7 @@ 
namespace android {

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



struct cached_buffer_t;

struct ComposerState;

struct DisplayState;

struct DisplayInfo;
@@ -131,7 +132,8 @@ public: 
                                     const Vector<DisplayState>& displays, uint32_t flags,

                                     const sp<IBinder>& applyToken,

                                     const InputWindowCommands& inputWindowCommands,

                                     int64_t desiredPresentTime) = 0;

                                     int64_t desiredPresentTime,

                                     const cached_buffer_t& uncacheBuffer) = 0;



    /* signal that we're done booting.

     * Requires ACCESS_SURFACE_FLINGER permission

libs/gui/include/gui/LayerState.h

+5 −4
Original line number Diff line number Diff line
@@ -41,6 +41,11 @@ namespace android { 
class Parcel;

class ISurfaceComposerClient;



struct cached_buffer_t {

    sp<IBinder> token = nullptr;

    uint64_t cacheId;

};



/*

 * Used to communicate layer information between SurfaceFlinger and its clients.

 */
@@ -133,10 +138,6 @@ struct layer_state_t { 
        float dtdy{0};

        float dsdy{0};

    };

    struct cached_buffer_t {

        sp<IBinder> token = nullptr;

        int32_t bufferId = -1;

    };

    sp<IBinder> surface;

    uint64_t what;

    float x;