Free resources only from the GL context thread.
Bug #3179882
Resources were freed following garbage collections on a worker thread.
This worker thread had no EGL context, which would cause the renderer
to incorrectly assume that the memory was liberated.
Change-Id: Ifdb51f94ddf42641e8654522787bfac532976c7c
diff --git a/core/jni/android/graphics/Path.cpp b/core/jni/android/graphics/Path.cpp
index abe33f4..90c4dd4 100644
--- a/core/jni/android/graphics/Path.cpp
+++ b/core/jni/android/graphics/Path.cpp
@@ -36,7 +36,7 @@
static void finalizer(JNIEnv* env, jobject clazz, SkPath* obj) {
#ifdef USE_OPENGL_RENDERER
if (android::uirenderer::Caches::hasInstance()) {
- android::uirenderer::Caches::getInstance().pathCache.remove(obj);
+ android::uirenderer::Caches::getInstance().pathCache.removeDeferred(obj);
}
#endif
delete obj;
diff --git a/libs/hwui/Caches.cpp b/libs/hwui/Caches.cpp
index 2b498c6..40dd117 100644
--- a/libs/hwui/Caches.cpp
+++ b/libs/hwui/Caches.cpp
@@ -95,6 +95,16 @@
}
///////////////////////////////////////////////////////////////////////////////
+// Memory management
+///////////////////////////////////////////////////////////////////////////////
+
+void Caches::clearGarbage() {
+ textureCache.clearGarbage();
+ gradientCache.clearGarbage();
+ pathCache.clearGarbage();
+}
+
+///////////////////////////////////////////////////////////////////////////////
// VBO
///////////////////////////////////////////////////////////////////////////////
diff --git a/libs/hwui/Caches.h b/libs/hwui/Caches.h
index 2779dfd..e8ced9c 100644
--- a/libs/hwui/Caches.h
+++ b/libs/hwui/Caches.h
@@ -101,6 +101,12 @@
}
/**
+ * Call this on each frame to ensure that garbage is deleted from
+ * GPU memory.
+ */
+ void clearGarbage();
+
+ /**
* Binds the VBO used to render simple textured quads.
*/
void bindMeshBuffer();
diff --git a/libs/hwui/DisplayListRenderer.cpp b/libs/hwui/DisplayListRenderer.cpp
index d08df44..77d628a 100644
--- a/libs/hwui/DisplayListRenderer.cpp
+++ b/libs/hwui/DisplayListRenderer.cpp
@@ -153,13 +153,13 @@
mPaints.clear();
for (size_t i = 0; i < mMatrices.size(); i++) {
- delete mMatrices.itemAt(i);
+ delete mMatrices.itemAt(i);
}
mMatrices.clear();
if (mPathHeap) {
for (int i = 0; i < mPathHeap->count(); i++) {
- caches.pathCache.remove(&(*mPathHeap)[i]);
+ caches.pathCache.removeDeferred(&(*mPathHeap)[i]);
}
mPathHeap->safeUnref();
}
diff --git a/libs/hwui/GradientCache.cpp b/libs/hwui/GradientCache.cpp
index 97f4cb4..a37964e 100644
--- a/libs/hwui/GradientCache.cpp
+++ b/libs/hwui/GradientCache.cpp
@@ -54,7 +54,6 @@
}
GradientCache::~GradientCache() {
- Mutex::Autolock _l(mLock);
mCache.clear();
}
@@ -63,17 +62,14 @@
///////////////////////////////////////////////////////////////////////////////
uint32_t GradientCache::getSize() {
- Mutex::Autolock _l(mLock);
return mSize;
}
uint32_t GradientCache::getMaxSize() {
- Mutex::Autolock _l(mLock);
return mMaxSize;
}
void GradientCache::setMaxSize(uint32_t maxSize) {
- Mutex::Autolock _l(mLock);
mMaxSize = maxSize;
while (mSize > mMaxSize) {
mCache.removeOldest();
@@ -102,17 +98,28 @@
///////////////////////////////////////////////////////////////////////////////
Texture* GradientCache::get(SkShader* shader) {
- Mutex::Autolock _l(mLock);
return mCache.get(shader);
}
void GradientCache::remove(SkShader* shader) {
- Mutex::Autolock _l(mLock);
mCache.remove(shader);
}
-void GradientCache::clear() {
+void GradientCache::removeDeferred(SkShader* shader) {
Mutex::Autolock _l(mLock);
+ mGarbage.push(shader);
+}
+
+void GradientCache::clearGarbage() {
+ Mutex::Autolock _l(mLock);
+ size_t count = mGarbage.size();
+ for (size_t i = 0; i < count; i++) {
+ mCache.remove(mGarbage.itemAt(i));
+ }
+ mGarbage.clear();
+}
+
+void GradientCache::clear() {
mCache.clear();
}
@@ -138,21 +145,17 @@
canvas.drawRectCoords(0.0f, 0.0f, bitmap.width(), 1.0f, p);
- mLock.lock();
// Asume the cache is always big enough
const uint32_t size = bitmap.rowBytes() * bitmap.height();
while (mSize + size > mMaxSize) {
mCache.removeOldest();
}
- mLock.unlock();
Texture* texture = new Texture;
generateTexture(&bitmap, texture);
- mLock.lock();
mSize += size;
mCache.put(shader, texture);
- mLock.unlock();
return texture;
}
diff --git a/libs/hwui/GradientCache.h b/libs/hwui/GradientCache.h
index c9553f4..30da462 100644
--- a/libs/hwui/GradientCache.h
+++ b/libs/hwui/GradientCache.h
@@ -19,6 +19,8 @@
#include <SkShader.h>
+#include <utils/Vector.h>
+
#include "Texture.h"
#include "utils/GenerationCache.h"
@@ -53,11 +55,20 @@
*/
Texture* get(SkShader* shader);
/**
- * Removes the texture associated with the specified shader. Returns NULL
- * if the texture cannot be found. Upon remove the texture is freed.
+ * Removes the texture associated with the specified shader.
+ * Upon remove the texture is freed.
*/
void remove(SkShader* shader);
/**
+ * Removes the texture associated with the specified shader. This is meant
+ * to be called from threads that are not the EGL context thread.
+ */
+ void removeDeferred(SkShader* shader);
+ /**
+ * Process deferred removals.
+ */
+ void clearGarbage();
+ /**
* Clears the cache. This causes all textures to be deleted.
*/
void clear();
@@ -83,10 +94,7 @@
uint32_t mSize;
uint32_t mMaxSize;
- /**
- * Used to access mCache and mSize. All methods are accessed from a single
- * thread except for remove().
- */
+ Vector<SkShader*> mGarbage;
mutable Mutex mLock;
}; // class GradientCache
diff --git a/libs/hwui/OpenGLRenderer.cpp b/libs/hwui/OpenGLRenderer.cpp
index acce1a2..ad72584 100644
--- a/libs/hwui/OpenGLRenderer.cpp
+++ b/libs/hwui/OpenGLRenderer.cpp
@@ -135,6 +135,8 @@
}
void OpenGLRenderer::prepare(bool opaque) {
+ mCaches.clearGarbage();
+
mSnapshot = new Snapshot(mFirstSnapshot,
SkCanvas::kMatrix_SaveFlag | SkCanvas::kClip_SaveFlag);
mSaveCount = 1;
diff --git a/libs/hwui/PathCache.cpp b/libs/hwui/PathCache.cpp
index 7ff26dc..8740a64 100644
--- a/libs/hwui/PathCache.cpp
+++ b/libs/hwui/PathCache.cpp
@@ -53,7 +53,6 @@
}
PathCache::~PathCache() {
- Mutex::Autolock _l(mLock);
mCache.clear();
}
@@ -72,17 +71,14 @@
///////////////////////////////////////////////////////////////////////////////
uint32_t PathCache::getSize() {
- Mutex::Autolock _l(mLock);
return mSize;
}
uint32_t PathCache::getMaxSize() {
- Mutex::Autolock _l(mLock);
return mMaxSize;
}
void PathCache::setMaxSize(uint32_t maxSize) {
- Mutex::Autolock _l(mLock);
mMaxSize = maxSize;
while (mSize > mMaxSize) {
mCache.removeOldest();
@@ -94,6 +90,14 @@
///////////////////////////////////////////////////////////////////////////////
void PathCache::operator()(PathCacheEntry& path, PathTexture*& texture) {
+ removeTexture(texture);
+}
+
+///////////////////////////////////////////////////////////////////////////////
+// Caching
+///////////////////////////////////////////////////////////////////////////////
+
+void PathCache::removeTexture(PathTexture* texture) {
if (texture) {
const uint32_t size = texture->width * texture->height;
mSize -= size;
@@ -109,39 +113,46 @@
}
}
-///////////////////////////////////////////////////////////////////////////////
-// Caching
-///////////////////////////////////////////////////////////////////////////////
-
void PathCache::remove(SkPath* path) {
- Mutex::Autolock _l(mLock);
-
// TODO: Linear search...
Vector<uint32_t> pathsToRemove;
for (uint32_t i = 0; i < mCache.size(); i++) {
if (mCache.getKeyAt(i).path == path) {
pathsToRemove.push(i);
+ removeTexture(mCache.getValueAt(i));
}
}
+ mCache.setOnEntryRemovedListener(NULL);
for (size_t i = 0; i < pathsToRemove.size(); i++) {
mCache.removeAt(pathsToRemove.itemAt(i));
}
+ mCache.setOnEntryRemovedListener(this);
+}
+
+void PathCache::removeDeferred(SkPath* path) {
+ Mutex::Autolock _l(mLock);
+ mGarbage.push(path);
+}
+
+void PathCache::clearGarbage() {
+ Mutex::Autolock _l(mLock);
+ size_t count = mGarbage.size();
+ for (size_t i = 0; i < count; i++) {
+ remove(mGarbage.itemAt(i));
+ }
+ mGarbage.clear();
}
PathTexture* PathCache::get(SkPath* path, SkPaint* paint) {
PathCacheEntry entry(path, paint);
- mLock.lock();
PathTexture* texture = mCache.get(entry);
- mLock.unlock();
if (!texture) {
texture = addTexture(entry, path, paint);
} else if (path->getGenerationID() != texture->generation) {
- mLock.lock();
mCache.remove(entry);
- mLock.unlock();
texture = addTexture(entry, path, paint);
}
@@ -167,11 +178,9 @@
const uint32_t size = width * height;
// Don't even try to cache a bitmap that's bigger than the cache
if (size < mMaxSize) {
- mLock.lock();
while (mSize + size > mMaxSize) {
mCache.removeOldest();
}
- mLock.unlock();
}
PathTexture* texture = new PathTexture;
@@ -200,7 +209,6 @@
generateTexture(bitmap, texture);
if (size < mMaxSize) {
- mLock.lock();
mSize += size;
PATH_LOGD("PathCache::get: create path: name, size, mSize = %d, %d, %d",
texture->id, size, mSize);
@@ -208,7 +216,6 @@
LOGD("Path created, size = %d", size);
}
mCache.put(entry, texture);
- mLock.unlock();
} else {
texture->cleanup = true;
}
@@ -217,7 +224,6 @@
}
void PathCache::clear() {
- Mutex::Autolock _l(mLock);
mCache.clear();
}
diff --git a/libs/hwui/PathCache.h b/libs/hwui/PathCache.h
index 0193f43..ae2e55d 100644
--- a/libs/hwui/PathCache.h
+++ b/libs/hwui/PathCache.h
@@ -21,6 +21,8 @@
#include <SkPaint.h>
#include <SkPath.h>
+#include <utils/Vector.h>
+
#include "Debug.h"
#include "Texture.h"
#include "utils/Compare.h"
@@ -146,6 +148,15 @@
* Removes an entry.
*/
void remove(SkPath* path);
+ /**
+ * Removes the specified path. This is meant to be called from threads
+ * that are not the EGL context thread.
+ */
+ void removeDeferred(SkPath* path);
+ /**
+ * Process deferred removals.
+ */
+ void clearGarbage();
/**
* Sets the maximum size of the cache in bytes.
@@ -166,6 +177,8 @@
*/
void generateTexture(SkBitmap& bitmap, Texture* texture);
+ void removeTexture(PathTexture* texture);
+
PathTexture* addTexture(const PathCacheEntry& entry, const SkPath *path, const SkPaint* paint);
void init();
@@ -178,10 +191,7 @@
bool mDebugEnabled;
- /**
- * Used to access mCache and mSize. All methods are accessed from a single
- * thread except for remove().
- */
+ Vector<SkPath*> mGarbage;
mutable Mutex mLock;
}; // class PathCache
diff --git a/libs/hwui/ResourceCache.cpp b/libs/hwui/ResourceCache.cpp
index 47c5d48..9f18948 100644
--- a/libs/hwui/ResourceCache.cpp
+++ b/libs/hwui/ResourceCache.cpp
@@ -126,7 +126,7 @@
if (ref == NULL) {
// If we're not tracking this resource, just delete it
if (Caches::hasInstance()) {
- Caches::getInstance().textureCache.remove(resource);
+ Caches::getInstance().textureCache.removeDeferred(resource);
}
delete resource;
return;
@@ -143,7 +143,7 @@
if (ref == NULL) {
// If we're not tracking this resource, just delete it
if (Caches::hasInstance()) {
- Caches::getInstance().gradientCache.remove(resource->getSkShader());
+ Caches::getInstance().gradientCache.removeDeferred(resource->getSkShader());
}
delete resource;
return;
@@ -179,7 +179,7 @@
{
SkBitmap* bitmap = (SkBitmap*)resource;
if (Caches::hasInstance()) {
- Caches::getInstance().textureCache.remove(bitmap);
+ Caches::getInstance().textureCache.removeDeferred(bitmap);
}
delete bitmap;
}
@@ -188,7 +188,7 @@
{
SkiaShader* shader = (SkiaShader*)resource;
if (Caches::hasInstance()) {
- Caches::getInstance().gradientCache.remove(shader->getSkShader());
+ Caches::getInstance().gradientCache.removeDeferred(shader->getSkShader());
}
delete shader;
}
diff --git a/libs/hwui/TextureCache.cpp b/libs/hwui/TextureCache.cpp
index 1be6868..ebecebe 100644
--- a/libs/hwui/TextureCache.cpp
+++ b/libs/hwui/TextureCache.cpp
@@ -53,7 +53,6 @@
}
TextureCache::~TextureCache() {
- Mutex::Autolock _l(mLock);
mCache.clear();
}
@@ -71,17 +70,14 @@
///////////////////////////////////////////////////////////////////////////////
uint32_t TextureCache::getSize() {
- Mutex::Autolock _l(mLock);
return mSize;
}
uint32_t TextureCache::getMaxSize() {
- Mutex::Autolock _l(mLock);
return mMaxSize;
}
void TextureCache::setMaxSize(uint32_t maxSize) {
- Mutex::Autolock _l(mLock);
mMaxSize = maxSize;
while (mSize > mMaxSize) {
mCache.removeOldest();
@@ -111,9 +107,7 @@
///////////////////////////////////////////////////////////////////////////////
Texture* TextureCache::get(SkBitmap* bitmap) {
- mLock.lock();
Texture* texture = mCache.get(bitmap);
- mLock.unlock();
if (!texture) {
if (bitmap->width() > mMaxTextureSize || bitmap->height() > mMaxTextureSize) {
@@ -124,11 +118,9 @@
const uint32_t size = bitmap->rowBytes() * bitmap->height();
// Don't even try to cache a bitmap that's bigger than the cache
if (size < mMaxSize) {
- mLock.lock();
while (mSize + size > mMaxSize) {
mCache.removeOldest();
}
- mLock.unlock();
}
texture = new Texture;
@@ -136,7 +128,6 @@
generateTexture(bitmap, texture, false);
if (size < mMaxSize) {
- mLock.lock();
mSize += size;
TEXTURE_LOGD("TextureCache::get: create texture(%p): name, size, mSize = %d, %d, %d",
bitmap, texture->id, size, mSize);
@@ -144,7 +135,6 @@
LOGD("Texture created, size = %d", size);
}
mCache.put(bitmap, texture);
- mLock.unlock();
} else {
texture->cleanup = true;
}
@@ -156,12 +146,24 @@
}
void TextureCache::remove(SkBitmap* bitmap) {
- Mutex::Autolock _l(mLock);
mCache.remove(bitmap);
}
-void TextureCache::clear() {
+void TextureCache::removeDeferred(SkBitmap* bitmap) {
Mutex::Autolock _l(mLock);
+ mGarbage.push(bitmap);
+}
+
+void TextureCache::clearGarbage() {
+ Mutex::Autolock _l(mLock);
+ size_t count = mGarbage.size();
+ for (size_t i = 0; i < count; i++) {
+ mCache.remove(mGarbage.itemAt(i));
+ }
+ mGarbage.clear();
+}
+
+void TextureCache::clear() {
mCache.clear();
TEXTURE_LOGD("TextureCache:clear(), miSize = %d", mSize);
}
diff --git a/libs/hwui/TextureCache.h b/libs/hwui/TextureCache.h
index 27693df..f7707f7 100644
--- a/libs/hwui/TextureCache.h
+++ b/libs/hwui/TextureCache.h
@@ -19,6 +19,8 @@
#include <SkBitmap.h>
+#include <utils/Vector.h>
+
#include "Debug.h"
#include "Texture.h"
#include "utils/GenerationCache.h"
@@ -64,11 +66,21 @@
*/
Texture* get(SkBitmap* bitmap);
/**
- * Removes the texture associated with the specified bitmap. Returns NULL
- * if the texture cannot be found. Upon remove the texture is freed.
+ * Removes the texture associated with the specified bitmap.
+ * Upon remove the texture is freed.
*/
void remove(SkBitmap* bitmap);
/**
+ * Removes the texture associated with the specified bitmap. This is meant
+ * to be called from threads that are not the EGL context thread.
+ */
+ void removeDeferred(SkBitmap* bitmap);
+ /**
+ * Process deferred removals.
+ */
+ void clearGarbage();
+
+ /**
* Clears the cache. This causes all textures to be deleted.
*/
void clear();
@@ -109,10 +121,7 @@
bool mDebugEnabled;
- /**
- * Used to access mCache and mSize. All methods are accessed from a single
- * thread except for remove().
- */
+ Vector<SkBitmap*> mGarbage;
mutable Mutex mLock;
}; // class TextureCache
diff --git a/libs/hwui/utils/GenerationCache.h b/libs/hwui/utils/GenerationCache.h
index 2e76236..42e6d9b 100644
--- a/libs/hwui/utils/GenerationCache.h
+++ b/libs/hwui/utils/GenerationCache.h
@@ -130,7 +130,7 @@
template<typename K, typename V>
V GenerationCache<K, V>::getValueAt(uint32_t index) const {
- return mCache.valueAt(index);
+ return mCache.valueAt(index)->value;
}
template<typename K, typename V>