We want to give SkPixelRef a way to signal over to GrResourceCache that it's become pointless to keep around textures based on that SkPixelRef when its pixels change, so that it can be a good citizen and free those textures.

This adds an invalidation listener mechanism to SkPixelRef to let it send this message while still staying ignorant of who's listening.

These messages are tricky to deliver.  The SkPixelRefs they originates from and the GrResourceCaches they ultimately end up at may be on different threads; neither class is threadsafe; their object lifetimes are totally independent; it's a many-senders-to-many-receivers relation; and neither codebase should really know about the other.

So I've added a per-message-type global message bus to broadcast messages to threadsafe inboxes.  Anyone can post() a message, which will show up in all the inboxes of that type, read whenever the inbox's owner calls poll().  The implementation is _dumb_; it can be improved in several dimensions (inbox size limits, lock-free message delivery) if we find the need.

I took some care to make sure not to send the invalidation message for any SkPixelRef that's sharing a generation ID with another SkPixelRef.

BUG=
R=bsalomon@google.com, scroggo@google.com, reed@google.com

Author: mtklein@google.com

Review URL: https://codereview.chromium.org/26734003

git-svn-id: http://skia.googlecode.com/svn/trunk@11949 2bbb7eff-a529-9590-31e7-b0007b416f81
This commit is contained in:
commit-bot@chromium.org 2013-10-24 17:44:27 +00:00
parent 81c67001f2
commit 50a3043194
13 changed files with 381 additions and 17 deletions

View File

@ -114,6 +114,7 @@
'<(skia_src_path)/core/SkMaskGamma.h',
'<(skia_src_path)/core/SkMath.cpp',
'<(skia_src_path)/core/SkMatrix.cpp',
'<(skia_src_path)/core/SkMessageBus.h',
'<(skia_src_path)/core/SkMetaData.cpp',
'<(skia_src_path)/core/SkMipMap.cpp',
'<(skia_src_path)/core/SkOnce.h',

View File

@ -89,6 +89,7 @@
'../tests/Matrix44Test.cpp',
'../tests/MemoryTest.cpp',
'../tests/MemsetTest.cpp',
'../tests/MessageBusTest.cpp',
'../tests/MetaDataTest.cpp',
'../tests/MipMapTest.cpp',
'../tests/OnceTest.cpp',
@ -104,6 +105,7 @@
'../tests/PictureTest.cpp',
'../tests/PictureUtilsTest.cpp',
'../tests/PipeTest.cpp',
'../tests/PixelRefTest.cpp',
'../tests/PointTest.cpp',
'../tests/PremulAlphaRoundTripTest.cpp',
'../tests/QuickRejectTest.cpp',

View File

@ -14,6 +14,7 @@
#include "SkRefCnt.h"
#include "SkString.h"
#include "SkFlattenable.h"
#include "SkTDArray.h"
#ifdef SK_DEBUG
/**
@ -49,6 +50,7 @@ public:
SK_DECLARE_INST_COUNT(SkPixelRef)
explicit SkPixelRef(SkBaseMutex* mutex = NULL);
virtual ~SkPixelRef();
/** Return the pixel memory returned from lockPixels, or null if the
lockCount is 0.
@ -209,6 +211,22 @@ public:
SK_DEFINE_FLATTENABLE_TYPE(SkPixelRef)
// Register a listener that may be called the next time our generation ID changes.
//
// We'll only call the listener if we're confident that we are the only SkPixelRef with this
// generation ID. If our generation ID changes and we decide not to call the listener, we'll
// never call it: you must add a new listener for each generation ID change. We also won't call
// the listener when we're certain no one knows what our generation ID is.
//
// This can be used to invalidate caches keyed by SkPixelRef generation ID.
struct GenIDChangeListener {
virtual ~GenIDChangeListener() {}
virtual void onChange() = 0;
};
// Takes ownership of listener.
void addGenIDChangeListener(GenIDChangeListener* listener);
protected:
/** Called when the lockCount goes from 0 to 1. The caller will have already
acquire a mutex for thread safety, so this method need not do that.
@ -254,17 +272,15 @@ protected:
void setPreLocked(void* pixels, SkColorTable* ctable);
private:
SkBaseMutex* fMutex; // must remain in scope for the life of this object
void* fPixels;
SkColorTable* fColorTable; // we do not track ownership, subclass does
int fLockCount;
mutable uint32_t fGenerationID;
mutable bool fUniqueGenerationID;
// SkBitmap is only a friend so that when copying, it can modify the new SkPixelRef to have the
// same fGenerationID as the original.
friend class SkBitmap;
SkTDArray<GenIDChangeListener*> fGenIDChangeListeners; // pointers are owned
SkString fURI;
@ -273,8 +289,16 @@ private:
// only ever set in constructor, const after that
bool fPreLocked;
void needsNewGenID();
void callGenIDChangeListeners();
void setMutex(SkBaseMutex* mutex);
// When copying a bitmap to another with the same shape and config, we can safely
// clone the pixelref generation ID too, which makes them equivalent under caching.
friend class SkBitmap; // only for cloneGenID
void cloneGenID(const SkPixelRef&);
typedef SkFlattenable INHERITED;
};

View File

@ -131,11 +131,14 @@ public:
* @param srcData Pointer to the pixel values.
* @param rowBytes The number of bytes between rows of the texture. Zero
* implies tightly packed rows.
* @param cacheKey (optional) If non-NULL, we'll write the cache key we used to cacheKey.
*/
GrTexture* createTexture(const GrTextureParams* params,
const GrTextureDesc& desc,
const GrCacheID& cacheID,
void* srcData, size_t rowBytes);
void* srcData,
size_t rowBytes,
GrResourceKey* cacheKey = NULL);
/**
* Search for an entry based on key and dimensions. If found, ref it and return it. The return

View File

@ -1057,7 +1057,8 @@ bool SkBitmap::copyTo(SkBitmap* dst, Config dstConfig, Allocator* alloc) const {
if (tmpSrc.config() == dstConfig && NULL == alloc) {
dst->swap(tmpSrc);
if (dst->pixelRef() && this->config() == dstConfig) {
dst->pixelRef()->fGenerationID = fPixelRef->getGenerationID();
// TODO(scroggo): fix issue 1742
dst->pixelRef()->cloneGenID(*fPixelRef);
}
return true;
}
@ -1097,8 +1098,9 @@ bool SkBitmap::copyTo(SkBitmap* dst, Config dstConfig, Allocator* alloc) const {
if (tmpDst.getSize() == src->getSize()) {
memcpy(tmpDst.getPixels(), src->getPixels(), src->getSafeSize());
SkPixelRef* pixelRef = tmpDst.pixelRef();
if (pixelRef != NULL) {
pixelRef->fGenerationID = this->getGenerationID();
if (NULL != pixelRef && NULL != fPixelRef) {
// TODO(scroggo): fix issue 1742
pixelRef->cloneGenID(*fPixelRef);
}
} else {
const char* srcP = reinterpret_cast<const char*>(src->getPixels());
@ -1152,7 +1154,8 @@ bool SkBitmap::deepCopyTo(SkBitmap* dst, Config dstConfig) const {
if (pixelRef) {
uint32_t rowBytes;
if (dstConfig == fConfig) {
pixelRef->fGenerationID = fPixelRef->getGenerationID();
// TODO(scroggo): fix issue 1742
pixelRef->cloneGenID(*fPixelRef);
// Use the same rowBytes as the original.
rowBytes = fRowBytes;
} else {

115
src/core/SkMessageBus.h Normal file
View File

@ -0,0 +1,115 @@
/*
* Copyright 2013 Google Inc.
*
* Use of this source code is governed by a BSD-style license that can be
* found in the LICENSE file.
*/
#ifndef SkMessageBus_DEFINED
#define SkMessageBus_DEFINED
#include "SkOnce.h"
#include "SkTDArray.h"
#include "SkThread.h"
#include "SkTypes.h"
template <typename Message>
class SkMessageBus : SkNoncopyable {
public:
// Post a message to be received by all Inboxes for this Message type. Threadsafe.
static void Post(const Message& m);
class Inbox {
public:
Inbox();
~Inbox();
// Overwrite out with all the messages we've received since the last call. Threadsafe.
void poll(SkTDArray<Message>* out);
private:
SkTDArray<Message> fMessages;
SkMutex fMessagesMutex;
friend class SkMessageBus;
void receive(const Message& m); // SkMessageBus is a friend only to call this.
};
private:
SkMessageBus();
static SkMessageBus* Get();
static void New(SkMessageBus**);
SkTDArray<Inbox*> fInboxes;
SkMutex fInboxesMutex;
};
// ----------------------- Implementation of SkMessageBus::Inbox -----------------------
template<typename Message>
SkMessageBus<Message>::Inbox::Inbox() {
// Register ourselves with the corresponding message bus.
SkMessageBus<Message>* bus = SkMessageBus<Message>::Get();
SkAutoMutexAcquire lock(bus->fInboxesMutex);
bus->fInboxes.push(this);
}
template<typename Message>
SkMessageBus<Message>::Inbox::~Inbox() {
// Remove ourselves from the corresponding message bus.
SkMessageBus<Message>* bus = SkMessageBus<Message>::Get();
SkAutoMutexAcquire lock(bus->fInboxesMutex);
// This is a cheaper fInboxes.remove(fInboxes.find(this)) when order doesn't matter.
for (int i = 0; i < bus->fInboxes.count(); i++) {
if (this == bus->fInboxes[i]) {
bus->fInboxes.removeShuffle(i);
break;
}
}
}
template<typename Message>
void SkMessageBus<Message>::Inbox::receive(const Message& m) {
SkAutoMutexAcquire lock(fMessagesMutex);
fMessages.push(m);
}
template<typename Message>
void SkMessageBus<Message>::Inbox::poll(SkTDArray<Message>* messages) {
SkASSERT(NULL != messages);
messages->reset();
SkAutoMutexAcquire lock(fMessagesMutex);
messages->swap(fMessages);
}
// ----------------------- Implementation of SkMessageBus -----------------------
template <typename Message>
SkMessageBus<Message>::SkMessageBus() {}
template <typename Message>
/*static*/ void SkMessageBus<Message>::New(SkMessageBus<Message>** bus) {
*bus = new SkMessageBus<Message>();
}
template <typename Message>
/*static*/ SkMessageBus<Message>* SkMessageBus<Message>::Get() {
// The first time this method is called, create the singleton bus for this message type.
static SkMessageBus<Message>* bus = NULL;
SK_DECLARE_STATIC_ONCE(once);
SkOnce(&once, &New, &bus);
SkASSERT(bus != NULL);
return bus;
}
template <typename Message>
/*static*/ void SkMessageBus<Message>::Post(const Message& m) {
SkMessageBus<Message>* bus = SkMessageBus<Message>::Get();
SkAutoMutexAcquire lock(bus->fInboxesMutex);
for (int i = 0; i < bus->fInboxes.count(); i++) {
bus->fInboxes[i]->receive(m);
}
}
#endif // SkMessageBus_DEFINED

View File

@ -85,12 +85,12 @@ void SkPixelRef::setMutex(SkBaseMutex* 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) {
SkPixelRef::SkPixelRef(SkBaseMutex* mutex) {
this->setMutex(mutex);
fPixels = NULL;
fColorTable = NULL; // we do not track ownership of this
fLockCount = 0;
fGenerationID = 0; // signal to rebuild
this->needsNewGenID();
fIsImmutable = false;
fPreLocked = false;
}
@ -103,9 +103,26 @@ SkPixelRef::SkPixelRef(SkFlattenableReadBuffer& buffer, SkBaseMutex* mutex)
fLockCount = 0;
fIsImmutable = buffer.readBool();
fGenerationID = buffer.readUInt();
fUniqueGenerationID = false; // Conservatively assuming the original still exists.
fPreLocked = false;
}
SkPixelRef::~SkPixelRef() {
this->callGenIDChangeListeners();
}
void SkPixelRef::needsNewGenID() {
fGenerationID = 0;
fUniqueGenerationID = false;
}
void SkPixelRef::cloneGenID(const SkPixelRef& that) {
// This is subtle. We must call that.getGenerationID() to make sure its genID isn't 0.
this->fGenerationID = that.getGenerationID();
this->fUniqueGenerationID = false;
that.fUniqueGenerationID = false;
}
void SkPixelRef::setPreLocked(void* pixels, SkColorTable* ctable) {
#ifndef SK_IGNORE_PIXELREF_SETPRELOCKED
// only call me in your constructor, otherwise fLockCount tracking can get
@ -129,6 +146,7 @@ void SkPixelRef::flatten(SkFlattenableWriteBuffer& buffer) const {
buffer.writeUInt(0);
} else {
buffer.writeUInt(fGenerationID);
fUniqueGenerationID = false; // Conservative, a copy is probably about to exist.
}
}
@ -178,18 +196,39 @@ bool SkPixelRef::onDecodeInto(int pow2, SkBitmap* bitmap) {
uint32_t SkPixelRef::getGenerationID() const {
if (0 == fGenerationID) {
fGenerationID = SkNextPixelRefGenerationID();
fUniqueGenerationID = true; // The only time we can be sure of this!
}
return fGenerationID;
}
void SkPixelRef::addGenIDChangeListener(GenIDChangeListener* listener) {
if (NULL == listener || !fUniqueGenerationID) {
// No point in tracking this if we're not going to call it.
SkDELETE(listener);
return;
}
*fGenIDChangeListeners.append() = listener;
}
void SkPixelRef::callGenIDChangeListeners() {
// We don't invalidate ourselves if we think another SkPixelRef is sharing our genID.
if (fUniqueGenerationID) {
for (int i = 0; i < fGenIDChangeListeners.count(); i++) {
fGenIDChangeListeners[i]->onChange();
}
}
// Listeners get at most one shot, so whether these triggered or not, blow them away.
fGenIDChangeListeners.deleteAll();
}
void SkPixelRef::notifyPixelsChanged() {
#ifdef SK_DEBUG
if (fIsImmutable) {
SkDebugf("========== notifyPixelsChanged called on immutable pixelref");
}
#endif
// this signals us to recompute this next time around
fGenerationID = 0;
this->callGenIDChangeListeners();
this->needsNewGenID();
}
void SkPixelRef::setImmutable() {

View File

@ -393,7 +393,8 @@ GrTexture* GrContext::createTexture(const GrTextureParams* params,
const GrTextureDesc& desc,
const GrCacheID& cacheID,
void* srcData,
size_t rowBytes) {
size_t rowBytes,
GrResourceKey* cacheKey) {
SK_TRACE_EVENT0("GrContext::createTexture");
GrResourceKey resourceKey = GrTexture::ComputeKey(fGpu, params, desc, cacheID);
@ -412,6 +413,10 @@ GrTexture* GrContext::createTexture(const GrTextureParams* params,
// necessary space before adding it.
fTextureCache->purgeAsNeeded(1, texture->sizeInBytes());
fTextureCache->addResource(resourceKey, texture);
if (NULL != cacheKey) {
*cacheKey = resourceKey;
}
}
return texture;

View File

@ -284,6 +284,8 @@ void GrResourceCache::purgeAsNeeded(int extraCount, size_t extraBytes) {
fPurging = true;
this->purgeInvalidated();
this->internalPurge(extraCount, extraBytes);
if (((fEntryCount+extraCount) > fMaxCount ||
(fEntryBytes+extraBytes) > fMaxBytes) &&
@ -298,6 +300,25 @@ void GrResourceCache::purgeAsNeeded(int extraCount, size_t extraBytes) {
fPurging = false;
}
void GrResourceCache::purgeInvalidated() {
SkTDArray<GrResourceInvalidatedMessage> invalidated;
fInvalidationInbox.poll(&invalidated);
for (int i = 0; i < invalidated.count(); i++) {
// We're somewhat missing an opportunity here. We could use the
// default find functor that gives us back resources whether we own
// them exclusively or not, and when they're not exclusively owned mark
// them for purging later when they do become exclusively owned.
//
// This is complicated and confusing. May try this in the future. For
// now, these resources are just LRU'd as if we never got the message.
GrResourceEntry* entry = fCache.find(invalidated[i].key, GrTFindUnreffedFunctor());
if (entry) {
this->deleteResource(entry);
}
}
}
void GrResourceCache::deleteResource(GrResourceEntry* entry) {
SkASSERT(1 == entry->fResource->getRefCnt());

View File

@ -15,6 +15,7 @@
#include "GrTypes.h"
#include "GrTHashTable.h"
#include "GrBinHashKey.h"
#include "SkMessageBus.h"
#include "SkTInternalLList.h"
class GrResource;
@ -141,6 +142,11 @@ private:
Key fKey;
};
// The cache listens for these messages to purge junk resources proactively.
struct GrResourceInvalidatedMessage {
GrResourceKey key;
};
///////////////////////////////////////////////////////////////////////////////
class GrResourceEntry {
@ -395,6 +401,10 @@ private:
void internalPurge(int extraCount, size_t extraBytes);
// Listen for messages that a resource has been invalidated and purge cached junk proactively.
SkMessageBus<GrResourceInvalidatedMessage>::Inbox fInvalidationInbox;
void purgeInvalidated();
#ifdef SK_DEBUG
static size_t countBytes(const SkTInternalLList<GrResourceEntry>& list);
#endif

View File

@ -7,6 +7,9 @@
#include "SkGr.h"
#include "SkConfig8888.h"
#include "SkMessageBus.h"
#include "SkPixelRef.h"
#include "GrResourceCache.h"
/* Fill out buffer with the compressed format Ganesh expects from a colortable
based bitmap. [palette (colortable) + indices].
@ -86,6 +89,28 @@ static void generate_bitmap_texture_desc(const SkBitmap& bitmap, GrTextureDesc*
desc->fSampleCnt = 0;
}
namespace {
// When the SkPixelRef genID changes, invalidate a corresponding GrResource described by key.
class GrResourceInvalidator : public SkPixelRef::GenIDChangeListener {
public:
explicit GrResourceInvalidator(GrResourceKey key) : fKey(key) {}
private:
GrResourceKey fKey;
virtual void onChange() SK_OVERRIDE {
const GrResourceInvalidatedMessage message = { fKey };
SkMessageBus<GrResourceInvalidatedMessage>::Post(message);
}
};
} // namespace
static void add_genID_listener(GrResourceKey key, SkPixelRef* pixelRef) {
SkASSERT(NULL != pixelRef);
pixelRef->addGenIDChangeListener(SkNEW_ARGS(GrResourceInvalidator, (key)));
}
static GrTexture* sk_gr_create_bitmap_texture(GrContext* ctx,
bool cache,
const GrTextureParams* params,
@ -112,7 +137,12 @@ static GrTexture* sk_gr_create_bitmap_texture(GrContext* ctx,
if (cache) {
GrCacheID cacheID;
generate_bitmap_cache_id(origBitmap, &cacheID);
return ctx->createTexture(params, desc, cacheID, storage.get(), bitmap->width());
GrResourceKey key;
GrTexture* result = ctx->createTexture(params, desc, cacheID,
storage.get(), bitmap->width(), &key);
add_genID_listener(key, origBitmap.pixelRef());
return result;
} else {
GrTexture* result = ctx->lockAndRefScratchTexture(desc,
GrContext::kExact_ScratchTexMatch);
@ -137,8 +167,13 @@ static GrTexture* sk_gr_create_bitmap_texture(GrContext* ctx,
// This texture is likely to be used again so leave it in the cache
GrCacheID cacheID;
generate_bitmap_cache_id(origBitmap, &cacheID);
return ctx->createTexture(params, desc, cacheID, bitmap->getPixels(), bitmap->rowBytes());
} else {
GrResourceKey key;
GrTexture* result = ctx->createTexture(params, desc, cacheID,
bitmap->getPixels(), bitmap->rowBytes(), &key);
add_genID_listener(key, origBitmap.pixelRef());
return result;
} else {
// This texture is unlikely to be used again (in its present form) so
// just use a scratch texture. This will remove the texture from the
// cache so no one else can find it. Additionally, once unlocked, the

57
tests/MessageBusTest.cpp Normal file
View File

@ -0,0 +1,57 @@
/*
* Copyright 2013 Google Inc.
*
* Use of this source code is governed by a BSD-style license that can be
* found in the LICENSE file.
*/
#include "SkMessageBus.h"
#include "Test.h"
#include "TestClassDef.h"
namespace {
struct TestMessage {
int x;
float y;
};
} // namespace
DEF_TEST(MessageBus, r) {
// Register two inboxes to receive all TestMessages.
SkMessageBus<TestMessage>::Inbox inbox1, inbox2;
// Send two messages.
const TestMessage m1 = { 5, 4.2f };
const TestMessage m2 = { 6, 4.3f };
SkMessageBus<TestMessage>::Post(m1);
SkMessageBus<TestMessage>::Post(m2);
// Make sure we got two.
SkTDArray<TestMessage> messages;
inbox1.poll(&messages);
REPORTER_ASSERT(r, 2 == messages.count());
REPORTER_ASSERT(r, 5 == messages[0].x);
REPORTER_ASSERT(r, 6 == messages[1].x);
// Send another; check we get just that one.
const TestMessage m3 = { 1, 0.3f };
SkMessageBus<TestMessage>::Post(m3);
inbox1.poll(&messages);
REPORTER_ASSERT(r, 1 == messages.count());
REPORTER_ASSERT(r, 1 == messages[0].x);
// Nothing was sent since the last read.
inbox1.poll(&messages);
REPORTER_ASSERT(r, 0 == messages.count());
// Over all this time, inbox2 should have piled up 3 messages.
inbox2.poll(&messages);
REPORTER_ASSERT(r, 3 == messages.count());
REPORTER_ASSERT(r, 5 == messages[0].x);
REPORTER_ASSERT(r, 6 == messages[1].x);
REPORTER_ASSERT(r, 1 == messages[2].x);
}
// Multithreaded tests tbd.

49
tests/PixelRefTest.cpp Normal file
View File

@ -0,0 +1,49 @@
#include "Test.h"
#include "TestClassDef.h"
#include "SkPixelRef.h"
#include "SkMallocPixelRef.h"
namespace {
class TestListener : public SkPixelRef::GenIDChangeListener {
public:
explicit TestListener(int* ptr) : fPtr(ptr) {}
void onChange() SK_OVERRIDE { (*fPtr)++; }
private:
int* fPtr;
};
} // namespace
DEF_TEST(PixelRef_GenIDChange, r) {
SkMallocPixelRef pixelRef(NULL, 0, NULL); // We don't really care about the pixels here.
// Register a listener.
int count = 0;
pixelRef.addGenIDChangeListener(SkNEW_ARGS(TestListener, (&count)));
REPORTER_ASSERT(r, 0 == count);
// No one has looked at our pixelRef's generation ID, so invalidating it doesn't make sense.
// (An SkPixelRef tree falls in the forest but there's nobody around to hear it. Do we care?)
pixelRef.notifyPixelsChanged();
REPORTER_ASSERT(r, 0 == count);
// Force the generation ID to be calculated.
REPORTER_ASSERT(r, 0 != pixelRef.getGenerationID());
// Our listener was dropped in the first call to notifyPixelsChanged(). This is a no-op.
pixelRef.notifyPixelsChanged();
REPORTER_ASSERT(r, 0 == count);
// Force the generation ID to be recalculated, then add a listener.
REPORTER_ASSERT(r, 0 != pixelRef.getGenerationID());
pixelRef.addGenIDChangeListener(SkNEW_ARGS(TestListener, (&count)));
pixelRef.notifyPixelsChanged();
REPORTER_ASSERT(r, 1 == count);
// Quick check that NULL is safe.
REPORTER_ASSERT(r, 0 != pixelRef.getGenerationID());
pixelRef.addGenIDChangeListener(NULL);
pixelRef.notifyPixelsChanged();
}