SkResourceCache::Key namespace support.

Add a unique-per-subclass namespace tag to make Keys from different
domains comparable.

Also drop the SkPictureShader cache and convert to using the global
resource cache instead.

R=reed@google.com,mtklein@google.com,robertphillips@google.com

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

diff --git a/bench/ImageCacheBench.cpp b/bench/ImageCacheBench.cpp
index 0f8fdf2..b8f40c8 100644 (file)
--- a/bench/ImageCacheBench.cpp
+++ b/bench/ImageCacheBench.cpp
@@ -12,11 +12,10 @@ namespace {
 static void* gGlobalAddress;
 class TestKey : public SkResourceCache::Key {
 public:
-    void*    fPtr;
     intptr_t fValue;
 
-    TestKey(intptr_t value) : fPtr(&gGlobalAddress), fValue(value) {
-        this->init(sizeof(fPtr) + sizeof(fValue));
+    TestKey(intptr_t value) : fValue(value) {
+        this->init(&gGlobalAddress, sizeof(fValue));
     }
 };
 struct TestRec : public SkResourceCache::Rec {

diff --git a/src/core/SkBitmapCache.cpp b/src/core/SkBitmapCache.cpp
index 13ec5aa..193a5ae 100644 (file)
--- a/src/core/SkBitmapCache.cpp
+++ b/src/core/SkBitmapCache.cpp
@@ -28,6 +28,9 @@ static SkIRect get_bounds_from_bitmap(const SkBitmap& bm) {
     return SkIRect::MakeXYWH(origin.fX, origin.fY, bm.width(), bm.height());
 }
 
+namespace {
+static unsigned gBitmapKeyNamespaceLabel;
+
 struct BitmapKey : public SkResourceCache::Key {
 public:
     BitmapKey(uint32_t genID, SkScalar scaleX, SkScalar scaleY, const SkIRect& bounds)
@@ -36,7 +39,8 @@ public:
     , fScaleY(scaleY)
     , fBounds(bounds)
     {
-        this->init(sizeof(fGenID) + sizeof(fScaleX) + sizeof(fScaleY) + sizeof(fBounds));
+        this->init(&gBitmapKeyNamespaceLabel,
+                   sizeof(fGenID) + sizeof(fScaleX) + sizeof(fScaleY) + sizeof(fBounds));
     }
 
     uint32_t    fGenID;
@@ -69,6 +73,7 @@ struct BitmapRec : public SkResourceCache::Rec {
         return SkToBool(result->getPixels());
     }
 };
+} // namespace
 
 #define CHECK_LOCAL(localCache, localName, globalName, ...) \
     ((localCache) ? localCache->localName(__VA_ARGS__) : SkResourceCache::globalName(__VA_ARGS__))

diff --git a/src/core/SkPictureShader.cpp b/src/core/SkPictureShader.cpp
index a4928d4..1f32a7e 100644 (file)
--- a/src/core/SkPictureShader.cpp
+++ b/src/core/SkPictureShader.cpp
@@ -10,17 +10,18 @@
 #include "SkBitmap.h"
 #include "SkBitmapProcShader.h"
 #include "SkCanvas.h"
-#include "SkDiscardableMemory.h"
 #include "SkMatrixUtils.h"
 #include "SkPicture.h"
 #include "SkReadBuffer.h"
 #include "SkResourceCache.h"
-#include "SkThread.h"
 
 #if SK_SUPPORT_GPU
 #include "GrContext.h"
 #endif
 
+namespace {
+static unsigned gBitmapSkaderKeyNamespaceLabel;
+
 struct BitmapShaderKey : public SkResourceCache::Key {
 public:
     BitmapShaderKey(uint32_t pictureID,
@@ -43,7 +44,7 @@ public:
                                       sizeof(fLocalMatrix);
         // This better be packed.
         SkASSERT(sizeof(uint32_t) * (&fEndOfStruct - &fPictureID) == keySize);
-        this->init(keySize);
+        this->init(&gBitmapSkaderKeyNamespaceLabel, keySize);
     }
 
 private:
@@ -80,60 +81,16 @@ struct BitmapShaderRec : public SkResourceCache::Rec {
     }
 };
 
-// FIXME: there's considerable boilerplate/duplication here vs. the global resource cache.
-SK_DECLARE_STATIC_MUTEX(gBitmapShaderCacheMutex);
-static SkResourceCache* gBitmapShaderCache = NULL;
-
-#ifndef SK_DEFAULT_TILE_CACHE_LIMIT
-    #define SK_DEFAULT_TILE_CACHE_LIMIT     (2 * 1024 * 1024)
-#endif
-
-static void cleanup_cache() {
-    // We'll clean this up in our own tests, but disable for clients.
-    // Chrome seems to have funky multi-process things going on in unit tests that
-    // makes this unsafe to delete when the main process atexit()s.
-    // SkLazyPtr does the same sort of thing.
-#if SK_DEVELOPER
-    SkDELETE(gBitmapShaderCache);
-#endif
-}
-
-/** Must hold gBitmapShaderCacheMutex when calling. */
-static SkResourceCache* cache() {
-    // gTileCacheMutex is always held when this is called, so we don't need to be fancy in here.
-    gBitmapShaderCacheMutex.assertHeld();
-    if (NULL == gBitmapShaderCache) {
-#ifdef SK_USE_DISCARDABLE_SCALEDIMAGECACHE
-        gBitmapShaderCache = SkNEW_ARGS(SkResourceCache, (SkDiscardableMemory::Create));
-#else
-        gBitmapShaderCache = SkNEW_ARGS(SkResourceCache, (SK_DEFAULT_TILE_CACHE_LIMIT));
-#endif
-        atexit(cleanup_cache);
-    }
-    return gBitmapShaderCache;
-}
-
-static bool cache_find(const BitmapShaderKey& key, SkAutoTUnref<SkShader>* result) {
-    SkAutoMutexAcquire am(gBitmapShaderCacheMutex);
-    return cache()->find(key, BitmapShaderRec::Visitor, result);
-}
-
-static void cache_add(BitmapShaderRec* rec) {
-    SkAutoMutexAcquire am(gBitmapShaderCacheMutex);
-    cache()->add(rec);
-}
-
 static bool cache_try_alloc_pixels(SkBitmap* bitmap) {
-    SkAutoMutexAcquire am(gBitmapShaderCacheMutex);
-    SkBitmap::Allocator* allocator = cache()->allocator();
+    SkBitmap::Allocator* allocator = SkResourceCache::GetAllocator();
 
-    if (NULL != allocator) {
-        return allocator->allocPixelRef(bitmap, NULL);
-    } else {
-        return bitmap->tryAllocPixels();
-    }
+    return NULL != allocator
+        ? allocator->allocPixelRef(bitmap, NULL)
+        : bitmap->tryAllocPixels();
 }
 
+} // namespace
+
 SkPictureShader::SkPictureShader(const SkPicture* picture, TileMode tmx, TileMode tmy,
                                  const SkMatrix* localMatrix, const SkRect* tile)
     : INHERITED(localMatrix)
@@ -227,7 +184,7 @@ SkShader* SkPictureShader::refBitmapShader(const SkMatrix& matrix, const SkMatri
                         tileScale,
                         this->getLocalMatrix());
 
-    if (!cache_find(key, &tileShader)) {
+    if (!SkResourceCache::Find(key, BitmapShaderRec::Visitor, &tileShader)) {
         SkBitmap bm;
         bm.setInfo(SkImageInfo::MakeN32Premul(tileSize));
         if (!cache_try_alloc_pixels(&bm)) {
@@ -244,7 +201,7 @@ SkShader* SkPictureShader::refBitmapShader(const SkMatrix& matrix, const SkMatri
         shaderMatrix.preScale(1 / tileScale.width(), 1 / tileScale.height());
         tileShader.reset(CreateBitmapShader(bm, fTmx, fTmy, &shaderMatrix));
 
-        cache_add(SkNEW_ARGS(BitmapShaderRec, (key, tileShader.get(), bm.getSize())));
+        SkResourceCache::Add(SkNEW_ARGS(BitmapShaderRec, (key, tileShader.get(), bm.getSize())));
     }
 
     return tileShader.detach();

diff --git a/src/core/SkResourceCache.cpp b/src/core/SkResourceCache.cpp
index f7a810e..1eb53cd 100644 (file)
--- a/src/core/SkResourceCache.cpp
+++ b/src/core/SkResourceCache.cpp
@@ -10,6 +10,8 @@
 #include "SkMipMap.h"
 #include "SkPixelRef.h"
 
+#include <stddef.h>
+
 // This can be defined by the caller's build system
 //#define SK_USE_DISCARDABLE_SCALEDIMAGECACHE
 
@@ -21,12 +23,22 @@
     #define SK_DEFAULT_IMAGE_CACHE_LIMIT     (2 * 1024 * 1024)
 #endif
 
-void SkResourceCache::Key::init(size_t length) {
+void SkResourceCache::Key::init(void* nameSpace, size_t length) {
     SkASSERT(SkAlign4(length) == length);
-    // 2 is fCount32 and fHash
-    fCount32 = SkToS32(2 + (length >> 2));
-    // skip both of our fields whe computing the murmur
-    fHash = SkChecksum::Murmur3(this->as32() + 2, (fCount32 - 2) << 2);
+
+    // fCount32 and fHash are not hashed
+    static const int kUnhashedLocal32s = 2;
+    static const int kLocal32s = kUnhashedLocal32s + (sizeof(fNamespace) >> 2);
+
+    SK_COMPILE_ASSERT(sizeof(Key) == (kLocal32s << 2), unaccounted_key_locals);
+    SK_COMPILE_ASSERT(sizeof(Key) == offsetof(Key, fNamespace) + sizeof(fNamespace),
+                      namespace_field_must_be_last);
+
+    fCount32 = SkToS32(kLocal32s + (length >> 2));
+    fNamespace = nameSpace;
+    // skip unhashed fields when computing the murmur
+    fHash = SkChecksum::Murmur3(this->as32() + kUnhashedLocal32s,
+                                (fCount32 - kUnhashedLocal32s) << 2);
 }
 
 #include "SkTDynamicHash.h"

diff --git a/src/core/SkResourceCache.h b/src/core/SkResourceCache.h
index c50b370..c16913f 100644 (file)
--- a/src/core/SkResourceCache.h
+++ b/src/core/SkResourceCache.h
@@ -31,8 +31,9 @@ public:
         void* writableContents() { return this + 1; }
 
         // must call this after your private data has been written.
+        // nameSpace must be unique per Key subclass.
         // length must be a multiple of 4
-        void init(size_t length);
+        void init(void* nameSpace, size_t length);
 
         // This is only valid after having called init().
         uint32_t hash() const { return fHash; }
@@ -49,8 +50,9 @@ public:
         }
 
     private:
-        int32_t  fCount32;  // 2 + user contents count32
+        int32_t  fCount32;   // local + user contents count32
         uint32_t fHash;
+        void*    fNamespace; // A unique namespace tag. This is hashed.
         /* uint32_t fContents32[] */
 
         const uint32_t* as32() const { return (const uint32_t*)this; }

diff --git a/tests/ImageCacheTest.cpp b/tests/ImageCacheTest.cpp
index 9f893bb..eaf3f28 100644 (file)
--- a/tests/ImageCacheTest.cpp
+++ b/tests/ImageCacheTest.cpp
@@ -12,11 +12,10 @@
 namespace {
 static void* gGlobalAddress;
 struct TestingKey : public SkResourceCache::Key {
-    void*       fPtr;
     intptr_t    fValue;
 
-    TestingKey(intptr_t value) : fPtr(&gGlobalAddress), fValue(value) {
-        this->init(sizeof(fPtr) + sizeof(fValue));
+    TestingKey(intptr_t value) : fValue(value) {
+        this->init(&gGlobalAddress, sizeof(fValue));
     }
 };
 struct TestingRec : public SkResourceCache::Rec {