diff options
author | Mathias Agopian <mathias@google.com> | 2009-04-15 18:34:24 -0700 |
---|---|---|
committer | Mathias Agopian <mathias@google.com> | 2009-04-15 18:34:24 -0700 |
commit | a6b40ba521d5c2fc23da74015531bd4ed8657921 (patch) | |
tree | 148f806ad3253b6582f9287750535f3b3bfeb26a | |
parent | 5f105d38e2c0c45d1d997906a2fa220b001a8e75 (diff) | |
download | frameworks_base-a6b40ba521d5c2fc23da74015531bd4ed8657921.zip frameworks_base-a6b40ba521d5c2fc23da74015531bd4ed8657921.tar.gz frameworks_base-a6b40ba521d5c2fc23da74015531bd4ed8657921.tar.bz2 |
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.
-rw-r--r-- | include/ui/BufferMapper.h | 14 | ||||
-rw-r--r-- | include/utils/Singleton.h | 3 | ||||
-rw-r--r-- | libs/surfaceflinger/BufferAllocator.cpp | 19 | ||||
-rw-r--r-- | libs/ui/BufferMapper.cpp | 72 | ||||
-rw-r--r-- | libs/ui/Surface.cpp | 11 |
5 files changed, 64 insertions, 55 deletions
diff --git a/include/ui/BufferMapper.h b/include/ui/BufferMapper.h index 9e5c5d7..ff90033 100644 --- a/include/ui/BufferMapper.h +++ b/include/ui/BufferMapper.h @@ -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; }; // --------------------------------------------------------------------------- diff --git a/include/utils/Singleton.h b/include/utils/Singleton.h index 776f93b..ee07df1 100644 --- a/include/utils/Singleton.h +++ b/include/utils/Singleton.h @@ -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 diff --git a/libs/surfaceflinger/BufferAllocator.cpp b/libs/surfaceflinger/BufferAllocator.cpp index 96a2c32..28fe810 100644 --- a/libs/surfaceflinger/BufferAllocator.cpp +++ b/libs/surfaceflinger/BufferAllocator.cpp @@ -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); diff --git a/libs/ui/BufferMapper.cpp b/libs/ui/BufferMapper.cpp index c7e439c..e6ca239 100644 --- a/libs/ui/BufferMapper.cpp +++ b/libs/ui/BufferMapper.cpp @@ -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(); } } diff --git a/libs/ui/Surface.cpp b/libs/ui/Surface.cpp index 357e4d0..1ceece0 100644 --- a/libs/ui/Surface.cpp +++ b/libs/ui/Surface.cpp @@ -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; |