Every pixel ref gets its own mutex.

Seems like a memory-saving flourish follow up would be to use SkSpinlock.

BUG=Florin's email.

Review URL: https://codereview.chromium.org/1289623004
This commit is contained in:
mtklein 2015-08-13 14:02:06 -07:00 committed by Commit bot
parent c7993d747f
commit 7e6d9c0326
3 changed files with 8 additions and 88 deletions

View File

@ -36,7 +36,6 @@ class GrTexture;
class SK_API SkPixelRef : public SkRefCnt {
public:
explicit SkPixelRef(const SkImageInfo&);
SkPixelRef(const SkImageInfo&, SkBaseMutex* mutex);
virtual ~SkPixelRef();
const SkImageInfo& info() const {
@ -319,7 +318,7 @@ protected:
/** Return the mutex associated with this pixelref. This value is assigned
in the constructor, and cannot change during the lifetime of the object.
*/
SkBaseMutex* mutex() const { return fMutex; }
SkBaseMutex* mutex() const { return &fMutex; }
// only call from constructor. Flags this to always be locked, removing
// the need to grab the mutex and call onLockPixels/onUnlockPixels.
@ -327,7 +326,7 @@ protected:
void setPreLocked(void*, size_t rowBytes, SkColorTable*);
private:
SkBaseMutex* fMutex; // must remain in scope for the life of this object
mutable SkMutex fMutex;
// mostly const. fInfo.fAlpahType can be changed at runtime.
const SkImageInfo fInfo;
@ -365,8 +364,6 @@ private:
void needsNewGenID();
void callGenIDChangeListeners();
void setMutex(SkBaseMutex* mutex);
void setTemporarilyImmutable();
void restoreMutability();
friend class SkSurface_Raster; // For the two methods above.

View File

@ -13,52 +13,6 @@
//#define SK_SUPPORT_LEGACY_UNBALANCED_PIXELREF_LOCKCOUNT
//#define SK_TRACE_PIXELREF_LIFETIME
#ifdef SK_BUILD_FOR_WIN32
// We don't have SK_BASE_MUTEX_INIT on Windows.
// must be a power-of-2. undef to just use 1 mutex
#define PIXELREF_MUTEX_RING_COUNT 32
static SkBaseMutex gPixelRefMutexRing[PIXELREF_MUTEX_RING_COUNT];
#else
static SkBaseMutex gPixelRefMutexRing[] = {
SK_BASE_MUTEX_INIT, SK_BASE_MUTEX_INIT,
SK_BASE_MUTEX_INIT, SK_BASE_MUTEX_INIT,
SK_BASE_MUTEX_INIT, SK_BASE_MUTEX_INIT,
SK_BASE_MUTEX_INIT, SK_BASE_MUTEX_INIT,
SK_BASE_MUTEX_INIT, SK_BASE_MUTEX_INIT,
SK_BASE_MUTEX_INIT, SK_BASE_MUTEX_INIT,
SK_BASE_MUTEX_INIT, SK_BASE_MUTEX_INIT,
SK_BASE_MUTEX_INIT, SK_BASE_MUTEX_INIT,
SK_BASE_MUTEX_INIT, SK_BASE_MUTEX_INIT,
SK_BASE_MUTEX_INIT, SK_BASE_MUTEX_INIT,
SK_BASE_MUTEX_INIT, SK_BASE_MUTEX_INIT,
SK_BASE_MUTEX_INIT, SK_BASE_MUTEX_INIT,
SK_BASE_MUTEX_INIT, SK_BASE_MUTEX_INIT,
SK_BASE_MUTEX_INIT, SK_BASE_MUTEX_INIT,
SK_BASE_MUTEX_INIT, SK_BASE_MUTEX_INIT,
SK_BASE_MUTEX_INIT, SK_BASE_MUTEX_INIT,
};
// must be a power-of-2. undef to just use 1 mutex
#define PIXELREF_MUTEX_RING_COUNT SK_ARRAY_COUNT(gPixelRefMutexRing)
#endif
static SkBaseMutex* get_default_mutex() {
static int32_t gPixelRefMutexRingIndex;
SkASSERT(SkIsPow2(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)];
}
///////////////////////////////////////////////////////////////////////////////
#include "SkNextID.h"
uint32_t SkNextID::ImageID() {
@ -73,13 +27,6 @@ uint32_t SkNextID::ImageID() {
///////////////////////////////////////////////////////////////////////////////
void SkPixelRef::setMutex(SkBaseMutex* mutex) {
if (NULL == mutex) {
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
@ -103,26 +50,6 @@ SkPixelRef::SkPixelRef(const SkImageInfo& info)
#ifdef SK_TRACE_PIXELREF_LIFETIME
SkDebugf(" pixelref %d\n", sk_atomic_inc(&gInstCounter));
#endif
this->setMutex(NULL);
fRec.zero();
fLockCount = 0;
this->needsNewGenID();
fMutability = kMutable;
fPreLocked = false;
fAddedToCache.store(false);
}
SkPixelRef::SkPixelRef(const SkImageInfo& info, SkBaseMutex* mutex)
: fInfo(validate_info(info))
#ifdef SK_BUILD_FOR_ANDROID_FRAMEWORK
, fStableID(SkNextID::ImageID())
#endif
{
#ifdef SK_TRACE_PIXELREF_LIFETIME
SkDebugf(" pixelref %d\n", sk_atomic_inc(&gInstCounter));
#endif
this->setMutex(mutex);
fRec.zero();
fLockCount = 0;
this->needsNewGenID();
@ -186,7 +113,7 @@ void SkPixelRef::setPreLocked(void* pixels, size_t rowBytes, SkColorTable* ctabl
// Increments fLockCount only on success
bool SkPixelRef::lockPixelsInsideMutex() {
fMutex->assertHeld();
fMutex.assertHeld();
if (1 == ++fLockCount) {
SkASSERT(fRec.isZero());
@ -212,7 +139,7 @@ bool SkPixelRef::lockPixels() {
if (!fPreLocked) {
TRACE_EVENT_BEGIN0("skia", "SkPixelRef::lockPixelsMutex");
SkAutoMutexAcquire ac(*fMutex);
SkAutoMutexAcquire ac(fMutex);
TRACE_EVENT_END0("skia", "SkPixelRef::lockPixelsMutex");
SkDEBUGCODE(int oldCount = fLockCount;)
bool success = this->lockPixelsInsideMutex();
@ -245,7 +172,7 @@ void SkPixelRef::unlockPixels() {
SkASSERT(!fPreLocked || SKPIXELREF_PRELOCKED_LOCKCOUNT == fLockCount);
if (!fPreLocked) {
SkAutoMutexAcquire ac(*fMutex);
SkAutoMutexAcquire ac(fMutex);
SkASSERT(fLockCount > 0);
if (0 == --fLockCount) {
@ -278,7 +205,7 @@ bool SkPixelRef::requestLock(const LockRequest& request, LockResult* result) {
result->fRowBytes = fRec.fRowBytes;
result->fSize.set(fInfo.width(), fInfo.height());
} else {
SkAutoMutexAcquire ac(*fMutex);
SkAutoMutexAcquire ac(fMutex);
if (!this->onRequestLock(request, result)) {
return false;
}

View File

@ -17,12 +17,8 @@
#include "SkGr.h"
#include "SkRect.h"
// since we call lockPixels recursively on fBitmap, we need a distinct mutex,
// to avoid deadlock with the default one provided by SkPixelRef.
SK_DECLARE_STATIC_MUTEX(gROLockPixelsPixelRefMutex);
SkROLockPixelsPixelRef::SkROLockPixelsPixelRef(const SkImageInfo& info)
: INHERITED(info, &gROLockPixelsPixelRefMutex) {}
: INHERITED(info) {}
SkROLockPixelsPixelRef::~SkROLockPixelsPixelRef() {}
@ -91,7 +87,7 @@ static SkGrPixelRef* copy_to_new_texture_pixelref(GrTexture* texture, SkColorTyp
// a larger TODO to remove SkGrPixelRef entirely.
context->copySurface(dst->asRenderTarget(), texture, srcRect, SkIPoint::Make(0,0),
GrContext::kFlushWrites_PixelOp);
SkImageInfo info = SkImageInfo::Make(desc.fWidth, desc.fHeight, dstCT, kPremul_SkAlphaType,
dstPT);
SkGrPixelRef* pixelRef = SkNEW_ARGS(SkGrPixelRef, (info, dst));