From dace19ec17e85872df3fb35212e1b8bce72018b6 Mon Sep 17 00:00:00 2001 From: bsalomon Date: Mon, 17 Nov 2014 07:34:06 -0800 Subject: [PATCH] Correct accounting for wrapped resources BUG=skia:2889 Review URL: https://codereview.chromium.org/720033004 --- src/gpu/GrContext.cpp | 4 +- src/gpu/GrGpuResource.cpp | 9 +++ src/gpu/GrGpuResourceCacheAccess.h | 2 + src/gpu/GrResourceCache2.cpp | 85 ++++++++++++++++++++++++----- src/gpu/GrResourceCache2.h | 24 ++++++-- tests/ResourceCacheTest.cpp | 88 ++++++++++++++++++++++++++++++ 6 files changed, 192 insertions(+), 20 deletions(-) diff --git a/src/gpu/GrContext.cpp b/src/gpu/GrContext.cpp index 5e0755b229..7012988ab8 100755 --- a/src/gpu/GrContext.cpp +++ b/src/gpu/GrContext.cpp @@ -218,10 +218,10 @@ void GrContext::freeGpuResources() { void GrContext::getResourceCacheUsage(int* resourceCount, size_t* resourceBytes) const { if (resourceCount) { - *resourceCount = fResourceCache2->getResourceCount(); + *resourceCount = fResourceCache2->getBudgetedResourceCount(); } if (resourceBytes) { - *resourceBytes = fResourceCache2->getResourceBytes(); + *resourceBytes = fResourceCache2->getBudgetedResourceBytes(); } } diff --git a/src/gpu/GrGpuResource.cpp b/src/gpu/GrGpuResource.cpp index fcc5f4de50..ea3756b8dd 100644 --- a/src/gpu/GrGpuResource.cpp +++ b/src/gpu/GrGpuResource.cpp @@ -87,6 +87,11 @@ bool GrGpuResource::setContentKey(const GrResourceKey& contentKey) { // Currently this can only be called once and can't be called when the resource is scratch. SkASSERT(!contentKey.isScratch()); SkASSERT(this->internalHasRef()); + + // Wrapped resources can never have a key. + if (this->isWrapped()) { + return false; + } if (fContentKeySet || this->wasDestroyed()) { return false; @@ -116,6 +121,10 @@ void GrGpuResource::setScratchKey(const GrResourceKey& scratchKey) { SkASSERT(fScratchKey.isNullScratch()); SkASSERT(scratchKey.isScratch()); SkASSERT(!scratchKey.isNullScratch()); + // Wrapped resources can never have a key. + if (this->isWrapped()) { + return; + } fScratchKey = scratchKey; } diff --git a/src/gpu/GrGpuResourceCacheAccess.h b/src/gpu/GrGpuResourceCacheAccess.h index 3c3786d053..7d20fff473 100644 --- a/src/gpu/GrGpuResourceCacheAccess.h +++ b/src/gpu/GrGpuResourceCacheAccess.h @@ -54,6 +54,8 @@ public: return NULL; } + bool isWrapped() const { return fResource->isWrapped(); } + /** * Called by the cache to delete the resource under normal circumstances. */ diff --git a/src/gpu/GrResourceCache2.cpp b/src/gpu/GrResourceCache2.cpp index 4593b82a7b..d5590d0e20 100644 --- a/src/gpu/GrResourceCache2.cpp +++ b/src/gpu/GrResourceCache2.cpp @@ -66,9 +66,13 @@ GrResourceCache2::GrResourceCache2() #if GR_CACHE_STATS , fHighWaterCount(0) , fHighWaterBytes(0) + , fBudgetedHighWaterCount(0) + , fBudgetedHighWaterBytes(0) #endif , fCount(0) , fBytes(0) + , fBudgetedCount(0) + , fBudgetedBytes(0) , fPurging(false) , fNewlyPurgableResourceWhilePurging(false) , fOverBudgetCB(NULL) @@ -94,12 +98,21 @@ void GrResourceCache2::insertResource(GrGpuResource* resource) { SkASSERT(!fPurging); fResources.addToHead(resource); + size_t size = resource->gpuMemorySize(); ++fCount; fBytes += resource->gpuMemorySize(); #if GR_CACHE_STATS fHighWaterCount = SkTMax(fCount, fHighWaterCount); fHighWaterBytes = SkTMax(fBytes, fHighWaterBytes); #endif + if (!resource->cacheAccess().isWrapped()) { + ++fBudgetedCount; + fBudgetedBytes += size; +#if GR_CACHE_STATS + fBudgetedHighWaterCount = SkTMax(fBudgetedCount, fBudgetedHighWaterCount); + fBudgetedHighWaterBytes = SkTMax(fBudgetedBytes, fBudgetedHighWaterBytes); +#endif + } if (!resource->cacheAccess().getScratchKey().isNullScratch()) { // TODO(bsalomon): Make this assertion possible. // SkASSERT(!resource->isWrapped()); @@ -112,10 +125,17 @@ void GrResourceCache2::insertResource(GrGpuResource* resource) { void GrResourceCache2::removeResource(GrGpuResource* resource) { AutoValidate av(this); - --fCount; - fBytes -= resource->gpuMemorySize(); SkASSERT(this->isInCache(resource)); - fResources.remove(resource); + + size_t size = resource->gpuMemorySize(); + --fCount; + fBytes -= size; + if (!resource->cacheAccess().isWrapped()) { + --fBudgetedCount; + fBudgetedBytes -= size; + } + + fResources.remove(resource); if (!resource->cacheAccess().getScratchKey().isNullScratch()) { fScratchMap.remove(resource->cacheAccess().getScratchKey(), resource); } @@ -137,6 +157,9 @@ void GrResourceCache2::abandonAll() { SkASSERT(!fScratchMap.count()); SkASSERT(!fContentHash.count()); SkASSERT(!fCount); + SkASSERT(!fBytes); + SkASSERT(!fBudgetedCount); + SkASSERT(!fBudgetedBytes); } void GrResourceCache2::releaseAll() { @@ -151,6 +174,9 @@ void GrResourceCache2::releaseAll() { } SkASSERT(!fScratchMap.count()); SkASSERT(!fCount); + SkASSERT(!fBytes); + SkASSERT(!fBudgetedCount); + SkASSERT(!fBudgetedBytes); } class GrResourceCache2::AvailableForScratchUse { @@ -236,12 +262,16 @@ void GrResourceCache2::notifyPurgable(GrGpuResource* resource) { } // Purge the resource if we're over budget - bool overBudget = fCount > fMaxCount || fBytes > fMaxBytes; + bool overBudget = fBudgetedCount > fMaxCount || fBudgetedBytes > fMaxBytes; // Also purge if the resource has neither a valid scratch key nor a content key. bool noKey = !resource->cacheAccess().isScratch() && (NULL == resource->cacheAccess().getContentKey()); + // Wrapped resources should never have a key. + SkASSERT(noKey || !resource->cacheAccess().isWrapped()); + + // And purge if the resource is wrapped if (overBudget || noKey) { SkDEBUGCODE(int beforeCount = fCount;) resource->cacheAccess().release(); @@ -257,10 +287,18 @@ void GrResourceCache2::didChangeGpuMemorySize(const GrGpuResource* resource, siz SkASSERT(resource); SkASSERT(this->isInCache(resource)); - fBytes += resource->gpuMemorySize() - oldSize; + ptrdiff_t delta = resource->gpuMemorySize() - oldSize; + + fBytes += delta; #if GR_CACHE_STATS fHighWaterBytes = SkTMax(fBytes, fHighWaterBytes); #endif + if (!resource->cacheAccess().isWrapped()) { + fBudgetedBytes += delta; +#if GR_CACHE_STATS + fBudgetedHighWaterBytes = SkTMax(fBudgetedBytes, fBudgetedHighWaterBytes); +#endif + } this->purgeAsNeeded(); this->validate(); @@ -269,7 +307,7 @@ void GrResourceCache2::didChangeGpuMemorySize(const GrGpuResource* resource, siz void GrResourceCache2::internalPurgeAsNeeded() { SkASSERT(!fPurging); SkASSERT(!fNewlyPurgableResourceWhilePurging); - SkASSERT(fCount > fMaxCount || fBytes > fMaxBytes); + SkASSERT(fBudgetedCount > fMaxCount || fBudgetedBytes > fMaxBytes); fPurging = true; @@ -288,7 +326,7 @@ void GrResourceCache2::internalPurgeAsNeeded() { resource->cacheAccess().release(); } resource = prev; - if (fCount <= fMaxCount && fBytes <= fMaxBytes) { + if (fBudgetedCount <= fMaxCount && fBudgetedBytes <= fMaxBytes) { overBudget = false; resource = NULL; } @@ -337,6 +375,8 @@ void GrResourceCache2::purgeAllUnlocked() { void GrResourceCache2::validate() const { size_t bytes = 0; int count = 0; + int budgetedCount = 0; + size_t budgetedBytes = 0; int locked = 0; int scratch = 0; int couldBeScratch = 0; @@ -356,30 +396,46 @@ void GrResourceCache2::validate() const { SkASSERT(NULL == resource->cacheAccess().getContentKey()); ++scratch; SkASSERT(fScratchMap.countForKey(resource->cacheAccess().getScratchKey())); + SkASSERT(!resource->cacheAccess().isWrapped()); } else if (!resource->cacheAccess().getScratchKey().isNullScratch()) { SkASSERT(NULL != resource->cacheAccess().getContentKey()); ++couldBeScratch; SkASSERT(fScratchMap.countForKey(resource->cacheAccess().getScratchKey())); + SkASSERT(!resource->cacheAccess().isWrapped()); } if (const GrResourceKey* contentKey = resource->cacheAccess().getContentKey()) { ++content; SkASSERT(fContentHash.find(*contentKey) == resource); + SkASSERT(!resource->cacheAccess().isWrapped()); + } + + if (!resource->cacheAccess().isWrapped()) { + ++budgetedCount; + budgetedBytes += resource->gpuMemorySize(); } } + SkASSERT(fBudgetedCount <= fCount); + SkASSERT(fBudgetedBytes <= fBudgetedBytes); SkASSERT(bytes == fBytes); SkASSERT(count == fCount); + SkASSERT(budgetedBytes == fBudgetedBytes); + SkASSERT(budgetedCount == fBudgetedCount); #if GR_CACHE_STATS + SkASSERT(fBudgetedHighWaterCount <= fHighWaterCount); + SkASSERT(fBudgetedHighWaterBytes <= fHighWaterBytes); SkASSERT(bytes <= fHighWaterBytes); SkASSERT(count <= fHighWaterCount); + SkASSERT(budgetedBytes <= fBudgetedHighWaterBytes); + SkASSERT(budgetedCount <= fBudgetedHighWaterCount); #endif SkASSERT(content == fContentHash.count()); SkASSERT(scratch + couldBeScratch == fScratchMap.count()); // This assertion is not currently valid because we can be in recursive notifyIsPurgable() // calls. This will be fixed when subresource registration is explicit. - // bool overBudget = bytes > fMaxBytes || count > fMaxCount; + // bool overBudget = budgetedBytes > fMaxBytes || budgetedCount > fMaxCount; // SkASSERT(!overBudget || locked == count || fPurging); } #endif @@ -403,14 +459,15 @@ void GrResourceCache2::printStats() const { } } - float countUtilization = (100.f * fCount) / fMaxCount; - float byteUtilization = (100.f * fBytes) / fMaxBytes; + float countUtilization = (100.f * fBudgetedCount) / fMaxCount; + float byteUtilization = (100.f * fBudgetedBytes) / fMaxBytes; SkDebugf("Budget: %d items %d bytes\n", fMaxCount, fMaxBytes); - SkDebugf("\t\tEntry Count: current %d (%d locked, %d scratch %.2g%% full), high %d\n", - fCount, locked, scratch, countUtilization, fHighWaterCount); - SkDebugf("\t\tEntry Bytes: current %d (%.2g%% full) high %d\n", - fBytes, byteUtilization, fHighWaterBytes); + SkDebugf( + "\t\tEntry Count: current %d (%d budgeted, %d locked, %d scratch %.2g%% full), high %d\n", + fCount, fBudgetedCount, locked, scratch, countUtilization, fHighWaterCount); + SkDebugf("\t\tEntry Bytes: current %d (budgeted %d, %.2g%% full) high %d\n", + fBytes, fBudgetedBytes, byteUtilization, fHighWaterBytes); } #endif diff --git a/src/gpu/GrResourceCache2.h b/src/gpu/GrResourceCache2.h index 805d359eb4..b0394e3c63 100644 --- a/src/gpu/GrResourceCache2.h +++ b/src/gpu/GrResourceCache2.h @@ -47,15 +47,25 @@ public: void setLimits(int count, size_t bytes); /** - * Returns the number of cached resources. + * Returns the number of resources. */ int getResourceCount() const { return fCount; } /** - * Returns the number of bytes consumed by cached resources. + * Returns the number of resources that count against the budget. + */ + int getBudgetedResourceCount() const { return fBudgetedCount; } + + /** + * Returns the number of bytes consumed by resources. */ size_t getResourceBytes() const { return fBytes; } + /** + * Returns the number of bytes consumed by budgeted resources. + */ + size_t getBudgetedResourceBytes() const { return fBudgetedBytes; } + /** * Returns the cached resources count budget. */ @@ -155,7 +165,7 @@ private: /// @} void purgeAsNeeded() { - if (fPurging || (fCount <= fMaxCount && fBytes < fMaxBytes)) { + if (fPurging || (fBudgetedCount <= fMaxCount && fBudgetedBytes < fMaxBytes)) { return; } this->internalPurgeAsNeeded(); @@ -207,12 +217,18 @@ private: #if GR_CACHE_STATS int fHighWaterCount; size_t fHighWaterBytes; + int fBudgetedHighWaterCount; + size_t fBudgetedHighWaterBytes; #endif - // our current stats, related to our budget + // our current stats for all resources int fCount; size_t fBytes; + // our current stats for resources that count against the budget + int fBudgetedCount; + size_t fBudgetedBytes; + // prevents recursive purging bool fPurging; bool fNewlyPurgableResourceWhilePurging; diff --git a/tests/ResourceCacheTest.cpp b/tests/ResourceCacheTest.cpp index c760f0e47d..5fef3d0e75 100644 --- a/tests/ResourceCacheTest.cpp +++ b/tests/ResourceCacheTest.cpp @@ -66,6 +66,14 @@ class TestResource : public GrGpuResource { public: SK_DECLARE_INST_COUNT(TestResource); + TestResource(GrGpu* gpu, bool isWrapped) + : INHERITED(gpu, isWrapped) + , fToDelete(NULL) + , fSize(kDefaultSize) { + ++fNumAlive; + this->registerWithCache(); + } + TestResource(GrGpu* gpu) : INHERITED(gpu, false) , fToDelete(NULL) @@ -165,6 +173,85 @@ static void test_no_key(skiatest::Reporter* reporter) { REPORTER_ASSERT(reporter, 0 == cache2->getResourceBytes()); } +static void test_wrapped(skiatest::Reporter* reporter) { + SkAutoTUnref context(GrContext::CreateMockContext()); + REPORTER_ASSERT(reporter, SkToBool(context)); + if (NULL == context) { + return; + } + context->setResourceCacheLimits(10, 300); + GrResourceCache2* cache2 = context->getResourceCache2(); + cache2->purgeAllUnlocked(); + SkASSERT(0 == cache2->getResourceCount() && 0 == cache2->getResourceBytes()); + SkASSERT(0 == cache2->getBudgetedResourceCount() && 0 == cache2->getBudgetedResourceBytes()); + + GrCacheID::Key keyData; + memset(&keyData, 0, sizeof(keyData)); + GrResourceKey::ResourceType t = GrResourceKey::GenerateResourceType(); + GrResourceKey scratchKey(GrCacheID(GrResourceKey::ScratchDomain(), keyData), t, 0); + GrResourceKey contentKey(GrCacheID(GrCacheID::GenerateDomain(), keyData), t, 0); + + // Create a scratch, a content, and a wrapped resource + TestResource* scratch = new TestResource(context->getGpu(), scratchKey); + scratch->setSize(10); + TestResource* content = new TestResource(context->getGpu()); + scratch->setSize(11); + REPORTER_ASSERT(reporter, content->cacheAccess().setContentKey(contentKey)); + TestResource* wrapped = new TestResource(context->getGpu(), true); + scratch->setSize(12); + + // Make sure we can't add a content key to the wrapped resource + keyData.fData8[0] = 1; + GrResourceKey contentKey2(GrCacheID(GrCacheID::GenerateDomain(), keyData), t, 0); + REPORTER_ASSERT(reporter, !wrapped->cacheAccess().setContentKey(contentKey2)); + REPORTER_ASSERT(reporter, NULL == cache2->findAndRefContentResource(contentKey2)); + + // Make sure sizes are as we expect + REPORTER_ASSERT(reporter, 3 == cache2->getResourceCount()); + REPORTER_ASSERT(reporter, scratch->gpuMemorySize() + content->gpuMemorySize() + + wrapped->gpuMemorySize() == cache2->getResourceBytes()); + REPORTER_ASSERT(reporter, 2 == cache2->getBudgetedResourceCount()); + REPORTER_ASSERT(reporter, scratch->gpuMemorySize() + content->gpuMemorySize() == + cache2->getBudgetedResourceBytes()); + + // Our refs mean that the resources are non purgable. + cache2->purgeAllUnlocked(); + REPORTER_ASSERT(reporter, 3 == cache2->getResourceCount()); + REPORTER_ASSERT(reporter, scratch->gpuMemorySize() + content->gpuMemorySize() + + wrapped->gpuMemorySize() == cache2->getResourceBytes()); + REPORTER_ASSERT(reporter, 2 == cache2->getBudgetedResourceCount()); + REPORTER_ASSERT(reporter, scratch->gpuMemorySize() + content->gpuMemorySize() == + cache2->getBudgetedResourceBytes()); + + // Unreffing the wrapped resource should free it right away. + wrapped->unref(); + REPORTER_ASSERT(reporter, 2 == cache2->getResourceCount()); + REPORTER_ASSERT(reporter, scratch->gpuMemorySize() + content->gpuMemorySize() == + cache2->getResourceBytes()); + + // Now try freeing the other two resources first + wrapped = new TestResource(context->getGpu(), true); + scratch->setSize(12); + content->unref(); + cache2->purgeAllUnlocked(); + REPORTER_ASSERT(reporter, 2 == cache2->getResourceCount()); + REPORTER_ASSERT(reporter, scratch->gpuMemorySize() + wrapped->gpuMemorySize() == + cache2->getResourceBytes()); + REPORTER_ASSERT(reporter, 1 == cache2->getBudgetedResourceCount()); + REPORTER_ASSERT(reporter, scratch->gpuMemorySize() == cache2->getBudgetedResourceBytes()); + + scratch->unref(); + cache2->purgeAllUnlocked(); + REPORTER_ASSERT(reporter, 1 == cache2->getResourceCount()); + REPORTER_ASSERT(reporter, wrapped->gpuMemorySize() == cache2->getResourceBytes()); + REPORTER_ASSERT(reporter, 0 == cache2->getBudgetedResourceCount()); + REPORTER_ASSERT(reporter, 0 == cache2->getBudgetedResourceBytes()); + + wrapped->unref(); + REPORTER_ASSERT(reporter, 0 == cache2->getResourceCount()); + REPORTER_ASSERT(reporter, 0 == cache2->getResourceBytes()); +} + static void test_duplicate_scratch_key(skiatest::Reporter* reporter) { SkAutoTUnref context(GrContext::CreateMockContext()); REPORTER_ASSERT(reporter, SkToBool(context)); @@ -491,6 +578,7 @@ DEF_GPUTEST(ResourceCache, reporter, factory) { // The below tests create their own mock contexts. test_no_key(reporter); + test_wrapped(reporter); test_duplicate_content_key(reporter); test_duplicate_scratch_key(reporter); test_purge_invalidated(reporter);