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

Commit a6b40ba5 authored by Mathias Agopian's avatar Mathias Agopian
Browse files

fix a rookie mistake causing Singleton<> to be a "multiton". Also improve the... 

fix a rookie mistake causing Singleton<> to be a "multiton". Also improve the BufferMapper's debugging, but turn it off.

Squashed commit of the following:

commit 04e9cae7f806bd65f2cfe35c011b47a36773bbe5
Author: Mathias Agopian <mathias@google.com>
Date:   Wed Apr 15 18:30:30 2009 -0700

    fix and improve BufferMapper's tracking of mapped buffers.

commit 1a8deaed15811092b2349cc3c40cafb5f722046c
Author: Mathias Agopian <mathias@google.com>
Date:   Wed Apr 15 00:52:02 2009 -0700

    fix some bugs with the Singleton<> class. untested.

commit ed01cc06ad70cf640ce1258f01189cb1a96fd3a8
Author: Mathias Agopian <mathias@google.com>
Date:   Tue Apr 14 19:29:25 2009 -0700

    some work to debug the Singleton<> template.
parent 5f105d38

include/ui/BufferMapper.h

+7 −7
Original line number Diff line number Diff line
@@ -40,8 +40,8 @@ class BufferMapper : public Singleton<BufferMapper> 
{

public:

    static inline BufferMapper& get() { return getInstance(); }

    status_t map(buffer_handle_t handle, void** addr);

    status_t unmap(buffer_handle_t handle);

    status_t map(buffer_handle_t handle, void** addr, const void* id);

    status_t unmap(buffer_handle_t handle, const void* id);

    status_t lock(buffer_handle_t handle, int usage, const Rect& bounds);

    status_t unlock(buffer_handle_t handle);
@@ -54,13 +54,13 @@ private: 
    mutable Mutex mLock;

    gralloc_module_t const *mAllocMod;

    

    void logMapLocked(buffer_handle_t handle, const void* id);

    void logUnmapLocked(buffer_handle_t handle, const void* id);

    struct map_info_t {

        int count;

        KeyedVector<CallStack, int> callstacks;

        const void* id;

        CallStack stack;

    };

    KeyedVector<buffer_handle_t, map_info_t> mMapInfo;

    void logMapLocked(buffer_handle_t handle);

    void logUnmapLocked(buffer_handle_t handle);

    KeyedVector<buffer_handle_t, Vector<map_info_t> > mMapInfo;

};



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

include/utils/Singleton.h

+0 −3
Original line number Diff line number Diff line
@@ -49,9 +49,6 @@ private: 
    static TYPE* sInstance;

};



template<class TYPE> Mutex Singleton<TYPE>::sLock; 

template<class TYPE> TYPE* Singleton<TYPE>::sInstance(0); 



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

}; // namespace android

libs/surfaceflinger/BufferAllocator.cpp

+14 −5
Original line number Diff line number Diff line
@@ -19,6 +19,8 @@ 
#include <utils/CallStack.h>

#include <cutils/ashmem.h>

#include <cutils/log.h>



#include <utils/Singleton.h>

#include <utils/String8.h>



#include <ui/BufferMapper.h>
@@ -32,6 +34,9 @@ 
namespace android {

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



template<class BufferAllocator> Mutex Singleton<BufferAllocator>::sLock; 

template<> BufferAllocator* Singleton<BufferAllocator>::sInstance(0); 



Mutex BufferAllocator::sLock;

KeyedVector<buffer_handle_t, BufferAllocator::alloc_rec_t> BufferAllocator::sAllocList;
@@ -106,7 +111,14 @@ status_t BufferAllocator::free(buffer_handle_t handle) 


#if ANDROID_GRALLOC_DEBUG

    void* base = (void*)(handle->data[2]);

#endif



    status_t err = mAllocDev->free(mAllocDev, handle);

    LOGW_IF(err, "free(...) failed %d (%s)", err, strerror(-err));

    

#if ANDROID_GRALLOC_DEBUG

    if (base) {

        LOGD("freeing mapped handle %p from:", handle);

        CallStack s;

        s.update();

        s.dump("");
@@ -114,9 +126,6 @@ status_t BufferAllocator::free(buffer_handle_t handle) 
    }

#endif



    status_t err = mAllocDev->free(mAllocDev, handle);

    LOGW_IF(err, "free(...) failed %d (%s)", err, strerror(-err));



    if (err == NO_ERROR) {

        Mutex::Autolock _l(sLock);

        KeyedVector<buffer_handle_t, alloc_rec_t>& list(sAllocList);
@@ -129,7 +138,7 @@ status_t BufferAllocator::free(buffer_handle_t handle) 
status_t BufferAllocator::map(buffer_handle_t handle, void** addr)

{

    Mutex::Autolock _l(mLock);

    status_t err = BufferMapper::get().map(handle, addr);

    status_t err = BufferMapper::get().map(handle, addr, this);

    if (err == NO_ERROR) {

        Mutex::Autolock _l(sLock);

        KeyedVector<buffer_handle_t, alloc_rec_t>& list(sAllocList);
@@ -145,7 +154,7 @@ status_t BufferAllocator::unmap(buffer_handle_t handle) 
{

    Mutex::Autolock _l(mLock);

    gralloc_module_t* mod = (gralloc_module_t*)mAllocDev->common.module;

    status_t err = BufferMapper::get().unmap(handle);

    status_t err = BufferMapper::get().unmap(handle, this);

    if (err == NO_ERROR) {

        Mutex::Autolock _l(sLock);

        KeyedVector<buffer_handle_t, alloc_rec_t>& list(sAllocList);

libs/ui/BufferMapper.cpp

+36 −36
Original line number Diff line number Diff line
@@ -36,14 +36,17 @@ 


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

// enable mapping debugging

#define DEBUG_MAPPINGS           1

#define DEBUG_MAPPINGS           0

// never remove mappings from the list

#define DEBUG_MAPPINGS_KEEP_ALL  1

#define DEBUG_MAPPINGS_KEEP_ALL  0

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



namespace android {

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



template<class BufferMapper> Mutex Singleton<BufferMapper>::sLock; 

template<> BufferMapper* Singleton<BufferMapper>::sInstance(0); 



BufferMapper::BufferMapper()

    : mAllocMod(0)

{
@@ -55,26 +58,26 @@ BufferMapper::BufferMapper() 
    }

}



status_t BufferMapper::map(buffer_handle_t handle, void** addr)

status_t BufferMapper::map(buffer_handle_t handle, void** addr, const void* id)

{

    Mutex::Autolock _l(mLock);

    status_t err = mAllocMod->map(mAllocMod, handle, addr);

    LOGW_IF(err, "map(...) failed %d (%s)", err, strerror(-err));

#if DEBUG_MAPPINGS

    if (err == NO_ERROR)

        logMapLocked(handle);

        logMapLocked(handle, id);

#endif

    return err;

}



status_t BufferMapper::unmap(buffer_handle_t handle)

status_t BufferMapper::unmap(buffer_handle_t handle, const void* id)

{

    Mutex::Autolock _l(mLock);

    status_t err = mAllocMod->unmap(mAllocMod, handle);

    LOGW_IF(err, "unmap(...) failed %d (%s)", err, strerror(-err));

#if DEBUG_MAPPINGS

    if (err == NO_ERROR)

        logUnmapLocked(handle);

        logUnmapLocked(handle, id);

#endif

    return err;

}
@@ -94,49 +97,46 @@ status_t BufferMapper::unlock(buffer_handle_t handle) 
    return err;

}



void BufferMapper::logMapLocked(buffer_handle_t handle)

void BufferMapper::logMapLocked(buffer_handle_t handle, const void* id)

{

    CallStack stack;

    stack.update(2);

    

    map_info_t info;

    info.id = id;

    info.stack = stack;

    

    ssize_t index = mMapInfo.indexOfKey(handle);

    if (index >= 0) {

        info = mMapInfo.valueAt(index);

    }

    

    ssize_t stackIndex = info.callstacks.indexOfKey(stack);

    if (stackIndex >= 0) {

        info.callstacks.editValueAt(stackIndex) += 1;

    } else {

        info.callstacks.add(stack, 1);

    }

    

    if (index < 0) {

        info.count = 1;

        mMapInfo.add(handle, info);

        Vector<map_info_t>& infos = mMapInfo.editValueAt(index);

        infos.add(info);

    } else {

        info.count++;

        mMapInfo.replaceValueAt(index, info);

        Vector<map_info_t> infos;

        infos.add(info);

        mMapInfo.add(handle, infos);

    }

}



void BufferMapper::logUnmapLocked(buffer_handle_t handle)

void BufferMapper::logUnmapLocked(buffer_handle_t handle, const void* id)

{    

    ssize_t index = mMapInfo.indexOfKey(handle);

    if (index < 0) {

        LOGE("unmapping %p which doesn't exist!", handle);

        LOGE("unmapping %p which doesn't exist in our map!", handle);

        return;

    }

    

    map_info_t& info = mMapInfo.editValueAt(index);

    info.count--;

    if (info.count == 0) {

#if DEBUG_MAPPINGS_KEEP_ALL

        info.callstacks.clear();

#else

    Vector<map_info_t>& infos = mMapInfo.editValueAt(index);

    ssize_t count = infos.size();

    for (int i=0 ; i<count ; ) {

        if (infos[i].id == id) {

            infos.removeAt(i);

            --count;

        } else {

            ++i;

        }

    }

    if (count == 0) {

        mMapInfo.removeItemsAt(index, 1);

#endif

    }

}
@@ -149,11 +149,11 @@ void BufferMapper::dump(buffer_handle_t handle) 
        return;

    }

    

    const map_info_t& info = mMapInfo.valueAt(index);

    LOGD("dumping buffer_handle_t %p mappings (count=%d)", handle, info.count);

    for (size_t i=0 ; i<info.callstacks.size() ; i++) {

        LOGD("#%d, count=%d", i, info.callstacks.valueAt(i));

        info.callstacks.keyAt(i).dump();

    const Vector<map_info_t>& infos = mMapInfo.valueAt(index);

    ssize_t count = infos.size();

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

        LOGD("#%d", i);

        infos[i].stack.dump();

    }

}

libs/ui/Surface.cpp

+7 −4
Original line number Diff line number Diff line
@@ -50,6 +50,9 @@ namespace android { 
//  SurfaceBuffer

// ============================================================================



template<class SurfaceBuffer> Mutex Singleton<SurfaceBuffer>::sLock; 

template<> SurfaceBuffer* Singleton<SurfaceBuffer>::sInstance(0); 



SurfaceBuffer::SurfaceBuffer() 

    : BASE(), handle(0), mOwner(false)

{
@@ -199,10 +202,10 @@ Surface::~Surface() 
    if (mOwner && mToken>=0 && mClient!=0) {

        mClient->destroySurface(mToken);

        if (mBuffers[0] != 0) {

            BufferMapper::get().unmap(mBuffers[0]->getHandle());

            BufferMapper::get().unmap(mBuffers[0]->getHandle(), this);

        }

        if (mBuffers[1] != 0) {

            BufferMapper::get().unmap(mBuffers[1]->getHandle());

            BufferMapper::get().unmap(mBuffers[1]->getHandle(), this);

        }

    }

    mClient.clear();
@@ -572,10 +575,10 @@ status_t Surface::getBufferLocked(int index) 
    if (buffer != 0) {

        sp<SurfaceBuffer>& currentBuffer(mBuffers[index]);

        if (currentBuffer != 0) {

            BufferMapper::get().unmap(currentBuffer->getHandle());

            BufferMapper::get().unmap(currentBuffer->getHandle(), this);

            currentBuffer.clear();

        }

        err = BufferMapper::get().map(buffer->getHandle(), &buffer->bits);

        err = BufferMapper::get().map(buffer->getHandle(), &buffer->bits, this);

        LOGW_IF(err, "map(...) failed %d (%s)", err, strerror(-err));

        if (err == NO_ERROR) {

            currentBuffer = buffer;