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.
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 @@
{
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 @@
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 @@
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 @@
#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 @@
}
#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::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 @@
{
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 @@
}
}
-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 @@
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;
+ Vector<map_info_t>& infos = mMapInfo.editValueAt(index);
+ infos.add(info);
} else {
- info.callstacks.add(stack, 1);
- }
-
- if (index < 0) {
- info.count = 1;
- mMapInfo.add(handle, 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 @@
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 @@
// 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 @@
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 @@
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;