AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Mutexes in pixelrefs were done very sloppily initially. The code (a) assumes all

Browse Source 
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
This commit is contained in:
reed@google.com 2012-05-17 13:14:52 +00:00
parent fa66294c77
commit ff0da4ff48
8 changed files with 100 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
include/core/SkPixelRef.h
View File

@ -43,9 +43,10 @@ public:
*/
SkColorTable* colorTable() const { return fColorTable; }
/** Return the current lockcount (defaults to 0)
*/
int getLockCount() const { return fLockCount; }
/**
* Returns true if the lockcount > 0
*/
bool isLocked() const { return fLockCount > 0; }
/** Call to access the pixel memory, which is returned. Balance with a call
to unlockPixels().
@ -169,6 +170,18 @@ protected:
SkPixelRef(SkFlattenableReadBuffer&, SkBaseMutex*);
virtual void flatten(SkFlattenableWriteBuffer&) const SK_OVERRIDE;
// only call from constructor. Flags this to always be locked, removing
// the need to grab the mutex and call onLockPixels/onUnlockPixels.
// Performance tweak to avoid those calls (esp. in multi-thread use case).
void setPreLocked(void* pixels, SkColorTable* ctable);
/**
* If a subclass passed a particular mutex to the base constructor, it can
* override that to go back to the default mutex by calling this. However,
* this should only be called from within the subclass' constructor.
*/
void useDefaultMutex() { this->setMutex(NULL); }
private:
SkBaseMutex* fMutex; // must remain in scope for the life of this object
@ -182,6 +195,10 @@ private:
// can go from false to true, but never from true to false
bool fIsImmutable;
// only ever set in constructor, const after that
bool fPreLocked;
void setMutex(SkBaseMutex* mutex);
typedef SkFlattenable INHERITED;
};

7
include/core/SkThread_platform.h
View File

@ -140,6 +140,8 @@ struct SkBaseMutex {
// Special case used when the static mutex must be available globally.
#define SK_DECLARE_GLOBAL_MUTEX(name) SkBaseMutex name = { PTHREAD_MUTEX_INITIALIZER }
#define SK_DECLARE_MUTEX_ARRAY(name, count) SkBaseMutex name[count] = { PTHREAD_MUTEX_INITIALIZER }
// A normal mutex that requires to be initialized through normal C++ construction,
// i.e. when it's a member of another class, or allocated on the heap.
class SkMutex : public SkBaseMutex, SkNoncopyable {
@ -171,8 +173,9 @@ private:
typedef SkMutex SkBaseMutex;
#define SK_DECLARE_STATIC_MUTEX(name) static SkBaseMutex name
#define SK_DECLARE_GLOBAL_MUTEX(name) SkBaseMutex name
#define SK_DECLARE_STATIC_MUTEX(name) static SkBaseMutex name
#define SK_DECLARE_GLOBAL_MUTEX(name) SkBaseMutex name
#define SK_DECLARE_MUTEX_ARRAY(name, count) SkBaseMutex name[count]
#endif // !SK_USE_POSIX_THREADS

4
src/core/SkBitmap.cpp
View File

@ -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);

4
src/core/SkBitmapProcShader.cpp
View File

@ -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;

4
src/core/SkMallocPixelRef.cpp
View File

@ -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)

77
src/core/SkPixelRef.cpp
View File

@ -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(!fPreLocked || SKPIXELREF_PRELOCKED_LOCKCOUNT == fLockCount);
SkASSERT(fLockCount > 0);
if (0 == --fLockCount) {
this->onUnlockPixels();
fPixels = NULL;
fColorTable = NULL;
if (!fPreLocked) {
SkAutoMutexAcquire ac(*fMutex);
SkASSERT(fLockCount > 0);
if (0 == --fLockCount) {
this->onUnlockPixels();
fPixels = NULL;
fColorTable = NULL;
}
}
}

6
src/images/SkImageRefPool.cpp
View File

@ -59,7 +59,7 @@ void SkImageRefPool::setRAMUsed(size_t limit) {
while (NULL != ref && fRAMUsed > limit) {
// only purge it if its pixels are unlocked
if (0 == ref->getLockCount() && ref->fBitmap.getPixels()) {
if (!ref->isLocked() && ref->fBitmap.getPixels()) {
size_t size = ref->ramUsed();
SkASSERT(size <= fRAMUsed);
fRAMUsed -= size;
@ -181,10 +181,10 @@ void SkImageRefPool::dump() const {
SkImageRef* ref = fHead;
while (ref != NULL) {
SkDebugf(" [%3d %3d %d] ram=%d data=%d locks=%d %s\n", ref->fBitmap.width(),
SkDebugf(" [%3d %3d %d] ram=%d data=%d locked=%d %s\n", ref->fBitmap.width(),
ref->fBitmap.height(), ref->fBitmap.config(),
ref->ramUsed(), (int)ref->fStream->getLength(),
ref->getLockCount(), ref->getURI());
ref->isLocked(), ref->getURI());
ref = ref->fNext;
}

3
src/ports/SkImageRef_ashmem.cpp
View File

@ -41,6 +41,8 @@ SkImageRef_ashmem::SkImageRef_ashmem(SkStream* stream,
fRec.fPinned = false;
fCT = NULL;
this->useDefaultMutex(); // we don't need/want the shared imageref mutex
}
SkImageRef_ashmem::~SkImageRef_ashmem() {
@ -235,6 +237,7 @@ SkImageRef_ashmem::SkImageRef_ashmem(SkFlattenableReadBuffer& buffer)
buffer.read(buf, length);
setURI(buf, length);
}
this->useDefaultMutex(); // we don't need/want the shared imageref mutex
}
SkPixelRef* SkImageRef_ashmem::Create(SkFlattenableReadBuffer& buffer) {