aboutsummaryrefslogtreecommitdiffhomepage
path: root/src/core
diff options
context:
space:
mode:
authorGravatar reed@google.com <reed@google.com@2bbb7eff-a529-9590-31e7-b0007b416f81>2012-05-17 13:14:52 +0000
committerGravatar reed@google.com <reed@google.com@2bbb7eff-a529-9590-31e7-b0007b416f81>2012-05-17 13:14:52 +0000
commitff0da4ff483ba7b4468b862949ffb3de505cba14 (patch)
treeb02ce2447d3a112e7c66c52be6d80e2ece7f70ef /src/core
parentfa66294c7705831808ce7772d4328fc626d45034 (diff)
Mutexes in pixelrefs were done very sloppily initially. The code (a) assumes all
pixelref subclasses want a mutex to guard their lock/unlock virtuals, and (b) most subclasses use the same mutex for *all* of their instances, even when there is no explicit need to guard modifying one instances with another. When we try drawing bitmaps from multiple threads, we are seeing a lot of slow- down from these mutexes. This CL has two changes to try to speed things up. 1. Add setPreLocked(), for pixelrefs who never need the onLockPixels virtual to be called. This speeds up those subclasses in multithreaded environs as it avoids the mutex lock all together (e.g. SkMallocPixelRef). 2. Add setMutex() to allow a subclass to change the mutex choice. ashmem wants this, since its unflattening constructor cannot pass down the null, it needs to cleanup afterwards. Review URL: https://codereview.appspot.com/6199075 git-svn-id: http://skia.googlecode.com/svn/trunk@3985 2bbb7eff-a529-9590-31e7-b0007b416f81
Diffstat (limited to 'src/core')
-rw-r--r--src/core/SkBitmap.cpp4
-rw-r--r--src/core/SkBitmapProcShader.cpp4
-rw-r--r--src/core/SkMallocPixelRef.cpp4
-rw-r--r--src/core/SkPixelRef.cpp79
4 files changed, 70 insertions, 21 deletions
diff --git a/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp
index da87e77159..6b9914535e 100644
--- a/src/core/SkBitmap.cpp
+++ b/src/core/SkBitmap.cpp
@@ -297,7 +297,7 @@ err:
void SkBitmap::updatePixelsFromRef() const {
if (NULL != fPixelRef) {
if (fPixelLockCount > 0) {
- SkASSERT(fPixelRef->getLockCount() > 0);
+ SkASSERT(fPixelRef->isLocked());
void* p = fPixelRef->pixels();
if (NULL != p) {
@@ -1525,7 +1525,7 @@ void SkBitmap::validate() const {
#if 0 // these asserts are not thread-correct, so disable for now
if (fPixelRef) {
if (fPixelLockCount > 0) {
- SkASSERT(fPixelRef->getLockCount() > 0);
+ SkASSERT(fPixelRef->isLocked());
} else {
SkASSERT(NULL == fPixels);
SkASSERT(NULL == fColorTable);
diff --git a/src/core/SkBitmapProcShader.cpp b/src/core/SkBitmapProcShader.cpp
index 3abfc8d230..f76bc09547 100644
--- a/src/core/SkBitmapProcShader.cpp
+++ b/src/core/SkBitmapProcShader.cpp
@@ -173,7 +173,7 @@ void SkBitmapProcShader::shadeSpan(int x, int y, SkPMColor dstC[], int count) {
SkASSERT(state.fBitmap->getPixels());
SkASSERT(state.fBitmap->pixelRef() == NULL ||
- state.fBitmap->pixelRef()->getLockCount());
+ state.fBitmap->pixelRef()->isLocked());
for (;;) {
int n = count;
@@ -217,7 +217,7 @@ void SkBitmapProcShader::shadeSpan16(int x, int y, uint16_t dstC[], int count) {
SkASSERT(state.fBitmap->getPixels());
SkASSERT(state.fBitmap->pixelRef() == NULL ||
- state.fBitmap->pixelRef()->getLockCount());
+ state.fBitmap->pixelRef()->isLocked());
for (;;) {
int n = count;
diff --git a/src/core/SkMallocPixelRef.cpp b/src/core/SkMallocPixelRef.cpp
index 313a26eeaa..14a064654f 100644
--- a/src/core/SkMallocPixelRef.cpp
+++ b/src/core/SkMallocPixelRef.cpp
@@ -18,6 +18,8 @@ SkMallocPixelRef::SkMallocPixelRef(void* storage, size_t size,
fSize = size;
fCTable = ctable;
SkSafeRef(ctable);
+
+ this->setPreLocked(fStorage, fCTable);
}
SkMallocPixelRef::~SkMallocPixelRef() {
@@ -57,6 +59,8 @@ SkMallocPixelRef::SkMallocPixelRef(SkFlattenableReadBuffer& buffer)
} else {
fCTable = NULL;
}
+
+ this->setPreLocked(fStorage, fCTable);
}
SK_DEFINE_FLATTENABLE_REGISTRAR(SkMallocPixelRef)
diff --git a/src/core/SkPixelRef.cpp b/src/core/SkPixelRef.cpp
index f01fbea81d..88ce76fb6a 100644
--- a/src/core/SkPixelRef.cpp
+++ b/src/core/SkPixelRef.cpp
@@ -9,7 +9,28 @@
#include "SkFlattenable.h"
#include "SkThread.h"
-SK_DECLARE_STATIC_MUTEX(gPixelRefMutex);
+// must be a power-of-2. undef to just use 1 mutex
+#define PIXELREF_MUTEX_RING_COUNT 32
+
+#ifdef PIXELREF_MUTEX_RING_COUNT
+ static int32_t gPixelRefMutexRingIndex;
+ static SK_DECLARE_MUTEX_ARRAY(gPixelRefMutexRing, PIXELREF_MUTEX_RING_COUNT);
+#else
+ SK_DECLARE_STATIC_MUTEX(gPixelRefMutex);
+#endif
+
+SkBaseMutex* get_default_mutex() {
+#ifdef PIXELREF_MUTEX_RING_COUNT
+ // atomic_inc might be overkill here. It may be fine if once in a while
+ // we hit a race-condition and two subsequent calls get the same index...
+ int index = sk_atomic_inc(&gPixelRefMutexRingIndex);
+ return &gPixelRefMutexRing[index & (PIXELREF_MUTEX_RING_COUNT - 1)];
+#else
+ return &gPixelRefMutex;
+#endif
+}
+
+///////////////////////////////////////////////////////////////////////////////
extern int32_t SkNextPixelRefGenerationID();
int32_t SkNextPixelRefGenerationID() {
@@ -23,30 +44,46 @@ int32_t SkNextPixelRefGenerationID() {
return genID;
}
+///////////////////////////////////////////////////////////////////////////////
-SkPixelRef::SkPixelRef(SkBaseMutex* mutex) {
+void SkPixelRef::setMutex(SkBaseMutex* mutex) {
if (NULL == mutex) {
- mutex = &gPixelRefMutex;
+ mutex = get_default_mutex();
}
fMutex = mutex;
+}
+
+// just need a > 0 value, so pick a funny one to aid in debugging
+#define SKPIXELREF_PRELOCKED_LOCKCOUNT 123456789
+
+SkPixelRef::SkPixelRef(SkBaseMutex* mutex) : fPreLocked(false) {
+ this->setMutex(mutex);
fPixels = NULL;
fColorTable = NULL; // we do not track ownership of this
fLockCount = 0;
fGenerationID = 0; // signal to rebuild
fIsImmutable = false;
+ fPreLocked = false;
}
SkPixelRef::SkPixelRef(SkFlattenableReadBuffer& buffer, SkBaseMutex* mutex)
: INHERITED(buffer) {
- if (NULL == mutex) {
- mutex = &gPixelRefMutex;
- }
- fMutex = mutex;
+ this->setMutex(mutex);
fPixels = NULL;
fColorTable = NULL; // we do not track ownership of this
fLockCount = 0;
fGenerationID = 0; // signal to rebuild
fIsImmutable = buffer.readBool();
+ fPreLocked = false;
+}
+
+void SkPixelRef::setPreLocked(void* pixels, SkColorTable* ctable) {
+ // only call me in your constructor, otherwise fLockCount tracking can get
+ // out of sync.
+ fPixels = pixels;
+ fColorTable = ctable;
+ fLockCount = SKPIXELREF_PRELOCKED_LOCKCOUNT;
+ fPreLocked = true;
}
void SkPixelRef::flatten(SkFlattenableWriteBuffer& buffer) const {
@@ -55,21 +92,29 @@ void SkPixelRef::flatten(SkFlattenableWriteBuffer& buffer) const {
}
void SkPixelRef::lockPixels() {
- SkAutoMutexAcquire ac(*fMutex);
+ SkASSERT(!fPreLocked || SKPIXELREF_PRELOCKED_LOCKCOUNT == fLockCount);
- if (1 == ++fLockCount) {
- fPixels = this->onLockPixels(&fColorTable);
+ if (!fPreLocked) {
+ SkAutoMutexAcquire ac(*fMutex);
+
+ if (1 == ++fLockCount) {
+ fPixels = this->onLockPixels(&fColorTable);
+ }
}
}
void SkPixelRef::unlockPixels() {
- SkAutoMutexAcquire ac(*fMutex);
-
- SkASSERT(fLockCount > 0);
- if (0 == --fLockCount) {
- this->onUnlockPixels();
- fPixels = NULL;
- fColorTable = NULL;
+ SkASSERT(!fPreLocked || SKPIXELREF_PRELOCKED_LOCKCOUNT == fLockCount);
+
+ if (!fPreLocked) {
+ SkAutoMutexAcquire ac(*fMutex);
+
+ SkASSERT(fLockCount > 0);
+ if (0 == --fLockCount) {
+ this->onUnlockPixels();
+ fPixels = NULL;
+ fColorTable = NULL;
+ }
}
}