Merge "Reduce the number of locks acquired by display lists" into jb-mr1-dev

4 files changed, 201 insertions, 78 deletions

diff --git a/libs/hwui/DisplayListRenderer.cpp b/libs/hwui/DisplayListRenderer.cpp
index 2de70d4..603b6df 100644
--- a/libs/hwui/DisplayListRenderer.cpp
+++ b/libs/hwui/DisplayListRenderer.cpp
@@ -149,6 +149,7 @@ void DisplayList::clearResources() {
     delete mTransformMatrix3D;
     delete mStaticMatrix;
     delete mAnimationMatrix;
+
     mTransformMatrix = NULL;
     mTransformCamera = NULL;
     mTransformMatrix3D = NULL;
@@ -156,50 +157,54 @@ void DisplayList::clearResources() {
     mAnimationMatrix = NULL;
 
     Caches& caches = Caches::getInstance();
+    caches.resourceCache.lock();
 
     for (size_t i = 0; i < mBitmapResources.size(); i++) {
-        caches.resourceCache.decrementRefcount(mBitmapResources.itemAt(i));
+        caches.resourceCache.decrementRefcountLocked(mBitmapResources.itemAt(i));
     }
-    mBitmapResources.clear();
 
     for (size_t i = 0; i < mOwnedBitmapResources.size(); i++) {
         SkBitmap* bitmap = mOwnedBitmapResources.itemAt(i);
-        caches.resourceCache.decrementRefcount(bitmap);
-        caches.resourceCache.destructor(bitmap);
+        caches.resourceCache.decrementRefcountLocked(bitmap);
+        caches.resourceCache.destructorLocked(bitmap);
     }
-    mOwnedBitmapResources.clear();
 
     for (size_t i = 0; i < mFilterResources.size(); i++) {
-        caches.resourceCache.decrementRefcount(mFilterResources.itemAt(i));
+        caches.resourceCache.decrementRefcountLocked(mFilterResources.itemAt(i));
     }
-    mFilterResources.clear();
 
     for (size_t i = 0; i < mShaders.size(); i++) {
-        caches.resourceCache.decrementRefcount(mShaders.itemAt(i));
-        caches.resourceCache.destructor(mShaders.itemAt(i));
+        caches.resourceCache.decrementRefcountLocked(mShaders.itemAt(i));
+        caches.resourceCache.destructorLocked(mShaders.itemAt(i));
     }
-    mShaders.clear();
+
+    for (size_t i = 0; i < mSourcePaths.size(); i++) {
+        caches.resourceCache.decrementRefcountLocked(mSourcePaths.itemAt(i));
+    }
+
+    caches.resourceCache.unlock();
 
     for (size_t i = 0; i < mPaints.size(); i++) {
         delete mPaints.itemAt(i);
     }
-    mPaints.clear();
 
     for (size_t i = 0; i < mPaths.size(); i++) {
         SkPath* path = mPaths.itemAt(i);
         caches.pathCache.remove(path);
         delete path;
     }
-    mPaths.clear();
-
-    for (size_t i = 0; i < mSourcePaths.size(); i++) {
-        caches.resourceCache.decrementRefcount(mSourcePaths.itemAt(i));
-    }
-    mSourcePaths.clear();
 
     for (size_t i = 0; i < mMatrices.size(); i++) {
         delete mMatrices.itemAt(i);
     }
+
+    mBitmapResources.clear();
+    mOwnedBitmapResources.clear();
+    mFilterResources.clear();
+    mShaders.clear();
+    mSourcePaths.clear();
+    mPaints.clear();
+    mPaths.clear();
     mMatrices.clear();
 }
 
@@ -223,35 +228,44 @@ void DisplayList::initFromDisplayListRenderer(const DisplayListRenderer& recorde
     mReader.setMemory(buffer, mSize);
 
     Caches& caches = Caches::getInstance();
+    caches.resourceCache.lock();
 
     const Vector<SkBitmap*>& bitmapResources = recorder.getBitmapResources();
     for (size_t i = 0; i < bitmapResources.size(); i++) {
         SkBitmap* resource = bitmapResources.itemAt(i);
         mBitmapResources.add(resource);
-        caches.resourceCache.incrementRefcount(resource);
+        caches.resourceCache.incrementRefcountLocked(resource);
     }
 
     const Vector<SkBitmap*> &ownedBitmapResources = recorder.getOwnedBitmapResources();
     for (size_t i = 0; i < ownedBitmapResources.size(); i++) {
         SkBitmap* resource = ownedBitmapResources.itemAt(i);
         mOwnedBitmapResources.add(resource);
-        caches.resourceCache.incrementRefcount(resource);
+        caches.resourceCache.incrementRefcountLocked(resource);
     }
 
     const Vector<SkiaColorFilter*>& filterResources = recorder.getFilterResources();
     for (size_t i = 0; i < filterResources.size(); i++) {
         SkiaColorFilter* resource = filterResources.itemAt(i);
         mFilterResources.add(resource);
-        caches.resourceCache.incrementRefcount(resource);
+        caches.resourceCache.incrementRefcountLocked(resource);
     }
 
     const Vector<SkiaShader*>& shaders = recorder.getShaders();
     for (size_t i = 0; i < shaders.size(); i++) {
         SkiaShader* resource = shaders.itemAt(i);
         mShaders.add(resource);
-        caches.resourceCache.incrementRefcount(resource);
+        caches.resourceCache.incrementRefcountLocked(resource);
+    }
+
+    const SortedVector<SkPath*>& sourcePaths = recorder.getSourcePaths();
+    for (size_t i = 0; i < sourcePaths.size(); i++) {
+        mSourcePaths.add(sourcePaths.itemAt(i));
+        caches.resourceCache.incrementRefcountLocked(sourcePaths.itemAt(i));
     }
 
+    caches.resourceCache.unlock();
+
     const Vector<SkPaint*>& paints = recorder.getPaints();
     for (size_t i = 0; i < paints.size(); i++) {
         mPaints.add(paints.itemAt(i));
@@ -262,12 +276,6 @@ void DisplayList::initFromDisplayListRenderer(const DisplayListRenderer& recorde
         mPaths.add(paths.itemAt(i));
     }
 
-    const SortedVector<SkPath*>& sourcePaths = recorder.getSourcePaths();
-    for (size_t i = 0; i < sourcePaths.size(); i++) {
-        mSourcePaths.add(sourcePaths.itemAt(i));
-        caches.resourceCache.incrementRefcount(sourcePaths.itemAt(i));
-    }
-
     const Vector<SkMatrix*>& matrices = recorder.getMatrices();
     for (size_t i = 0; i < matrices.size(); i++) {
         mMatrices.add(matrices.itemAt(i));
@@ -1309,7 +1317,8 @@ status_t DisplayList::replay(OpenGLRenderer& renderer, Rect& dirty, int32_t flag
 // Base structure
 ///////////////////////////////////////////////////////////////////////////////
 
-DisplayListRenderer::DisplayListRenderer() : mWriter(MIN_WRITER_SIZE),
+DisplayListRenderer::DisplayListRenderer():
+        mCaches(Caches::getInstance()), mWriter(MIN_WRITER_SIZE),
         mTranslateX(0.0f), mTranslateY(0.0f), mHasTranslate(false), mHasDrawOps(false) {
 }
 
@@ -1320,34 +1329,38 @@ DisplayListRenderer::~DisplayListRenderer() {
 void DisplayListRenderer::reset() {
     mWriter.reset();
 
-    Caches& caches = Caches::getInstance();
+    mCaches.resourceCache.lock();
+
     for (size_t i = 0; i < mBitmapResources.size(); i++) {
-        caches.resourceCache.decrementRefcount(mBitmapResources.itemAt(i));
+        mCaches.resourceCache.decrementRefcountLocked(mBitmapResources.itemAt(i));
     }
-    mBitmapResources.clear();
 
     for (size_t i = 0; i < mOwnedBitmapResources.size(); i++) {
-        SkBitmap* bitmap = mOwnedBitmapResources.itemAt(i);
-        caches.resourceCache.decrementRefcount(bitmap);
+        mCaches.resourceCache.decrementRefcountLocked(mOwnedBitmapResources.itemAt(i));
     }
-    mOwnedBitmapResources.clear();
 
     for (size_t i = 0; i < mFilterResources.size(); i++) {
-        caches.resourceCache.decrementRefcount(mFilterResources.itemAt(i));
+        mCaches.resourceCache.decrementRefcountLocked(mFilterResources.itemAt(i));
     }
-    mFilterResources.clear();
 
     for (size_t i = 0; i < mShaders.size(); i++) {
-        caches.resourceCache.decrementRefcount(mShaders.itemAt(i));
+        mCaches.resourceCache.decrementRefcountLocked(mShaders.itemAt(i));
     }
-    mShaders.clear();
-    mShaderMap.clear();
 
     for (size_t i = 0; i < mSourcePaths.size(); i++) {
-        caches.resourceCache.decrementRefcount(mSourcePaths.itemAt(i));
+        mCaches.resourceCache.decrementRefcountLocked(mSourcePaths.itemAt(i));
     }
+
+    mCaches.resourceCache.unlock();
+
+    mBitmapResources.clear();
+    mOwnedBitmapResources.clear();
+    mFilterResources.clear();
     mSourcePaths.clear();
 
+    mShaders.clear();
+    mShaderMap.clear();
+
     mPaints.clear();
     mPaintMap.clear();
 
diff --git a/libs/hwui/DisplayListRenderer.h b/libs/hwui/DisplayListRenderer.h
index c8b3e47..8e4f2d3 100644
--- a/libs/hwui/DisplayListRenderer.h
+++ b/libs/hwui/DisplayListRenderer.h
@@ -763,7 +763,7 @@ private:
             mPaths.add(pathCopy);
         }
         if (mSourcePaths.indexOf(path) < 0) {
-            Caches::getInstance().resourceCache.incrementRefcount(path);
+            mCaches.resourceCache.incrementRefcount(path);
             mSourcePaths.add(path);
         }
 
@@ -811,13 +811,13 @@ private:
         // which doesn't seem worth the extra cycles for this unlikely case.
         addInt((int) bitmap);
         mBitmapResources.add(bitmap);
-        Caches::getInstance().resourceCache.incrementRefcount(bitmap);
+        mCaches.resourceCache.incrementRefcount(bitmap);
     }
 
     void addBitmapData(SkBitmap* bitmap) {
         addInt((int) bitmap);
         mOwnedBitmapResources.add(bitmap);
-        Caches::getInstance().resourceCache.incrementRefcount(bitmap);
+        mCaches.resourceCache.incrementRefcount(bitmap);
     }
 
     inline void addShader(SkiaShader* shader) {
@@ -833,7 +833,7 @@ private:
             // replaceValueFor() performs an add if the entry doesn't exist
             mShaderMap.replaceValueFor(shader, shaderCopy);
             mShaders.add(shaderCopy);
-            Caches::getInstance().resourceCache.incrementRefcount(shaderCopy);
+            mCaches.resourceCache.incrementRefcount(shaderCopy);
         }
 
         addInt((int) shaderCopy);
@@ -842,7 +842,7 @@ private:
     inline void addColorFilter(SkiaColorFilter* colorFilter) {
         addInt((int) colorFilter);
         mFilterResources.add(colorFilter);
-        Caches::getInstance().resourceCache.incrementRefcount(colorFilter);
+        mCaches.resourceCache.incrementRefcount(colorFilter);
     }
 
     Vector<SkBitmap*> mBitmapResources;
@@ -862,15 +862,16 @@ private:
 
     Vector<SkMatrix*> mMatrices;
 
-    SkWriter32 mWriter;
     uint32_t mBufferSize;
 
     int mRestoreSaveCount;
 
+    Caches& mCaches;
+    SkWriter32 mWriter;
+
     float mTranslateX;
     float mTranslateY;
     bool mHasTranslate;
-
     bool mHasDrawOps;
 
     friend class DisplayList;
diff --git a/libs/hwui/ResourceCache.cpp b/libs/hwui/ResourceCache.cpp
index 2153a8b..b0c57d1 100644
--- a/libs/hwui/ResourceCache.cpp
+++ b/libs/hwui/ResourceCache.cpp
@@ -38,7 +38,7 @@ void ResourceCache::logCache() {
 
 ResourceCache::ResourceCache() {
     Mutex::Autolock _l(mLock);
-    mCache = new KeyedVector<void *, ResourceReference *>();
+    mCache = new KeyedVector<void*, ResourceReference*>();
 }
 
 ResourceCache::~ResourceCache() {
@@ -46,15 +46,17 @@ ResourceCache::~ResourceCache() {
     delete mCache;
 }
 
+void ResourceCache::lock() {
+    mLock.lock();
+}
+
+void ResourceCache::unlock() {
+    mLock.unlock();
+}
+
 void ResourceCache::incrementRefcount(void* resource, ResourceType resourceType) {
     Mutex::Autolock _l(mLock);
-    ssize_t index = mCache->indexOfKey(resource);
-    ResourceReference* ref = index >= 0 ? mCache->valueAt(index) : NULL;
-    if (ref == NULL || mCache->size() == 0) {
-        ref = new ResourceReference(resourceType);
-        mCache->add(resource, ref);
-    }
-    ref->refCount++;
+    incrementRefcountLocked(resource, resourceType);
 }
 
 void ResourceCache::incrementRefcount(SkBitmap* bitmapResource) {
@@ -77,18 +79,39 @@ void ResourceCache::incrementRefcount(SkiaColorFilter* filterResource) {
     incrementRefcount((void*) filterResource, kColorFilter);
 }
 
-void ResourceCache::decrementRefcount(void* resource) {
-    Mutex::Autolock _l(mLock);
+void ResourceCache::incrementRefcountLocked(void* resource, ResourceType resourceType) {
     ssize_t index = mCache->indexOfKey(resource);
     ResourceReference* ref = index >= 0 ? mCache->valueAt(index) : NULL;
-    if (ref == NULL) {
-        // Should not get here - shouldn't get a call to decrement if we're not yet tracking it
-        return;
-    }
-    ref->refCount--;
-    if (ref->refCount == 0) {
-        deleteResourceReference(resource, ref);
+    if (ref == NULL || mCache->size() == 0) {
+        ref = new ResourceReference(resourceType);
+        mCache->add(resource, ref);
     }
+    ref->refCount++;
+}
+
+void ResourceCache::incrementRefcountLocked(SkBitmap* bitmapResource) {
+    SkSafeRef(bitmapResource->pixelRef());
+    SkSafeRef(bitmapResource->getColorTable());
+    incrementRefcountLocked((void*) bitmapResource, kBitmap);
+}
+
+void ResourceCache::incrementRefcountLocked(SkPath* pathResource) {
+    incrementRefcountLocked((void*) pathResource, kPath);
+}
+
+void ResourceCache::incrementRefcountLocked(SkiaShader* shaderResource) {
+    SkSafeRef(shaderResource->getSkShader());
+    incrementRefcountLocked((void*) shaderResource, kShader);
+}
+
+void ResourceCache::incrementRefcountLocked(SkiaColorFilter* filterResource) {
+    SkSafeRef(filterResource->getSkColorFilter());
+    incrementRefcountLocked((void*) filterResource, kColorFilter);
+}
+
+void ResourceCache::decrementRefcount(void* resource) {
+    Mutex::Autolock _l(mLock);
+    decrementRefcountLocked(resource);
 }
 
 void ResourceCache::decrementRefcount(SkBitmap* bitmapResource) {
@@ -111,27 +134,45 @@ void ResourceCache::decrementRefcount(SkiaColorFilter* filterResource) {
     decrementRefcount((void*) filterResource);
 }
 
-void ResourceCache::recycle(SkBitmap* resource) {
-    Mutex::Autolock _l(mLock);
+void ResourceCache::decrementRefcountLocked(void* resource) {
     ssize_t index = mCache->indexOfKey(resource);
-    if (index < 0) {
-        // not tracking this resource; just recycle the pixel data
-        resource->setPixels(NULL, NULL);
-        return;
-    }
-    ResourceReference* ref = mCache->valueAt(index);
+    ResourceReference* ref = index >= 0 ? mCache->valueAt(index) : NULL;
     if (ref == NULL) {
-        // Should not get here - shouldn't get a call to recycle if we're not yet tracking it
+        // Should not get here - shouldn't get a call to decrement if we're not yet tracking it
         return;
     }
-    ref->recycled = true;
+    ref->refCount--;
     if (ref->refCount == 0) {
         deleteResourceReference(resource, ref);
     }
 }
 
+void ResourceCache::decrementRefcountLocked(SkBitmap* bitmapResource) {
+    SkSafeUnref(bitmapResource->pixelRef());
+    SkSafeUnref(bitmapResource->getColorTable());
+    decrementRefcountLocked((void*) bitmapResource);
+}
+
+void ResourceCache::decrementRefcountLocked(SkPath* pathResource) {
+    decrementRefcountLocked((void*) pathResource);
+}
+
+void ResourceCache::decrementRefcountLocked(SkiaShader* shaderResource) {
+    SkSafeUnref(shaderResource->getSkShader());
+    decrementRefcountLocked((void*) shaderResource);
+}
+
+void ResourceCache::decrementRefcountLocked(SkiaColorFilter* filterResource) {
+    SkSafeUnref(filterResource->getSkColorFilter());
+    decrementRefcountLocked((void*) filterResource);
+}
+
 void ResourceCache::destructor(SkPath* resource) {
     Mutex::Autolock _l(mLock);
+    destructorLocked(resource);
+}
+
+void ResourceCache::destructorLocked(SkPath* resource) {
     ssize_t index = mCache->indexOfKey(resource);
     ResourceReference* ref = index >= 0 ? mCache->valueAt(index) : NULL;
     if (ref == NULL) {
@@ -150,6 +191,10 @@ void ResourceCache::destructor(SkPath* resource) {
 
 void ResourceCache::destructor(SkBitmap* resource) {
     Mutex::Autolock _l(mLock);
+    destructorLocked(resource);
+}
+
+void ResourceCache::destructorLocked(SkBitmap* resource) {
     ssize_t index = mCache->indexOfKey(resource);
     ResourceReference* ref = index >= 0 ? mCache->valueAt(index) : NULL;
     if (ref == NULL) {
@@ -168,6 +213,10 @@ void ResourceCache::destructor(SkBitmap* resource) {
 
 void ResourceCache::destructor(SkiaShader* resource) {
     Mutex::Autolock _l(mLock);
+    destructorLocked(resource);
+}
+
+void ResourceCache::destructorLocked(SkiaShader* resource) {
     ssize_t index = mCache->indexOfKey(resource);
     ResourceReference* ref = index >= 0 ? mCache->valueAt(index) : NULL;
     if (ref == NULL) {
@@ -183,6 +232,10 @@ void ResourceCache::destructor(SkiaShader* resource) {
 
 void ResourceCache::destructor(SkiaColorFilter* resource) {
     Mutex::Autolock _l(mLock);
+    destructorLocked(resource);
+}
+
+void ResourceCache::destructorLocked(SkiaColorFilter* resource) {
     ssize_t index = mCache->indexOfKey(resource);
     ResourceReference* ref = index >= 0 ? mCache->valueAt(index) : NULL;
     if (ref == NULL) {
@@ -196,6 +249,29 @@ void ResourceCache::destructor(SkiaColorFilter* resource) {
     }
 }
 
+void ResourceCache::recycle(SkBitmap* resource) {
+    Mutex::Autolock _l(mLock);
+    recycleLocked(resource);
+}
+
+void ResourceCache::recycleLocked(SkBitmap* resource) {
+    ssize_t index = mCache->indexOfKey(resource);
+    if (index < 0) {
+        // not tracking this resource; just recycle the pixel data
+        resource->setPixels(NULL, NULL);
+        return;
+    }
+    ResourceReference* ref = mCache->valueAt(index);
+    if (ref == NULL) {
+        // Should not get here - shouldn't get a call to recycle if we're not yet tracking it
+        return;
+    }
+    ref->recycled = true;
+    if (ref->refCount == 0) {
+        deleteResourceReference(resource, ref);
+    }
+}
+
 /**
  * This method should only be called while the mLock mutex is held (that mutex is grabbed
  * by the various destructor() and recycle() methods which call this method).
diff --git a/libs/hwui/ResourceCache.h b/libs/hwui/ResourceCache.h
index 8cf466b..60ffa7d 100644
--- a/libs/hwui/ResourceCache.h
+++ b/libs/hwui/ResourceCache.h
@@ -52,28 +52,59 @@ public:
 };
 
 class ANDROID_API ResourceCache {
-    KeyedVector<void *, ResourceReference *>* mCache;
 public:
     ResourceCache();
     ~ResourceCache();
+
+    /**
+     * When using these two methods, make sure to only invoke the *Locked()
+     * variants of increment/decrementRefcount(), recyle() and destructor()
+     */
+    void lock();
+    void unlock();
+
     void incrementRefcount(SkPath* resource);
     void incrementRefcount(SkBitmap* resource);
     void incrementRefcount(SkiaShader* resource);
     void incrementRefcount(SkiaColorFilter* resource);
-    void incrementRefcount(const void* resource, ResourceType resourceType);
-    void decrementRefcount(void* resource);
+
+    void incrementRefcountLocked(SkPath* resource);
+    void incrementRefcountLocked(SkBitmap* resource);
+    void incrementRefcountLocked(SkiaShader* resource);
+    void incrementRefcountLocked(SkiaColorFilter* resource);
+
     void decrementRefcount(SkBitmap* resource);
     void decrementRefcount(SkPath* resource);
     void decrementRefcount(SkiaShader* resource);
     void decrementRefcount(SkiaColorFilter* resource);
-    void recycle(SkBitmap* resource);
+
+    void decrementRefcountLocked(SkBitmap* resource);
+    void decrementRefcountLocked(SkPath* resource);
+    void decrementRefcountLocked(SkiaShader* resource);
+    void decrementRefcountLocked(SkiaColorFilter* resource);
+
     void destructor(SkPath* resource);
     void destructor(SkBitmap* resource);
     void destructor(SkiaShader* resource);
     void destructor(SkiaColorFilter* resource);
+
+    void destructorLocked(SkPath* resource);
+    void destructorLocked(SkBitmap* resource);
+    void destructorLocked(SkiaShader* resource);
+    void destructorLocked(SkiaColorFilter* resource);
+
+    void recycle(SkBitmap* resource);
+    void recycleLocked(SkBitmap* resource);
+
 private:
     void deleteResourceReference(void* resource, ResourceReference* ref);
+
     void incrementRefcount(void* resource, ResourceType resourceType);
+    void incrementRefcountLocked(void* resource, ResourceType resourceType);
+
+    void decrementRefcount(void* resource);
+    void decrementRefcountLocked(void* resource);
+
     void logCache();
 
     /**
@@ -82,6 +113,8 @@ private:
      * or a reference queue finalization thread.
      */
     mutable Mutex mLock;
+
+    KeyedVector<void*, ResourceReference*>* mCache;
 };
 
 }; // namespace uirenderer