From 23526963135bd15737505bd560d41b0d5a41439e Mon Sep 17 00:00:00 2001
From: xidachen <xidachen@chromium.org>
Date: Mon, 1 Feb 2016 05:27:16 -0800
Subject: [PATCH] Replace the id<-->key hashmap in SkImageFilter by a SkTArray

In the current implementation, SkImageFilter::Cache maintains a hash map
that maps SkImageFilter's uniqueID to an array of keys, and its purpose
is to remove the values in Cache that are associated with this array of
keys that are indexed by uniqueID. However, maintaining this hash map
causes perf regression to smoothness.tough_filters_cases.

This CL removes the id<-->key hashmap. Instead, we maintain an array of
keys in SkImageFilter. Whenever there is a new key, we push it into the
array. In ~SkImageFilter(), we call Cache::purgeByKeys to remove all the
values that are associated with the keys that are maintained by SkImageFilter.

BUG=571655
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1651433002

Review URL: https://codereview.chromium.org/1651433002
---
 include/core/SkImageFilter.h |  6 +++++-
 src/core/SkImageFilter.cpp   | 27 +++++++--------------------
 2 files changed, 12 insertions(+), 21 deletions(-)

diff --git a/include/core/SkImageFilter.h b/include/core/SkImageFilter.h
index 351a959..44231cc 100644
--- a/include/core/SkImageFilter.h
+++ b/include/core/SkImageFilter.h
@@ -9,11 +9,13 @@
 #define SkImageFilter_DEFINED
 
 #include "../private/SkTemplates.h"
+#include "../private/SkMutex.h"
 #include "SkFilterQuality.h"
 #include "SkFlattenable.h"
 #include "SkMatrix.h"
 #include "SkRect.h"
 #include "SkSurfaceProps.h"
+#include "SkTArray.h"
 
 class GrFragmentProcessor;
 class GrTexture;
@@ -42,7 +44,7 @@ public:
         virtual bool get(const Key& key, SkBitmap* result, SkIPoint* offset) const = 0;
         virtual void set(const Key& key, const SkBitmap& result, const SkIPoint& offset) = 0;
         virtual void purge() {}
-        virtual void purgeByImageFilterId(uint32_t) {}
+        virtual void purgeByKeys(const Key[], int) {}
     };
 
     class Context {
@@ -450,6 +452,8 @@ private:
     bool fUsesSrcInput;
     CropRect fCropRect;
     uint32_t fUniqueID; // Globally unique
+    mutable SkTArray<Cache::Key> fCacheKeys;
+    mutable SkMutex fMutex;
 };
 
 /**
diff --git a/src/core/SkImageFilter.cpp b/src/core/SkImageFilter.cpp
index 9fca5cf..94d6ab6 100644
--- a/src/core/SkImageFilter.cpp
+++ b/src/core/SkImageFilter.cpp
@@ -13,12 +13,10 @@
 #include "SkDevice.h"
 #include "SkLocalMatrixImageFilter.h"
 #include "SkMatrixImageFilter.h"
-#include "SkMutex.h"
 #include "SkOncePtr.h"
 #include "SkReadBuffer.h"
 #include "SkRect.h"
 #include "SkTDynamicHash.h"
-#include "SkTHash.h"
 #include "SkTInternalLList.h"
 #include "SkValidationUtils.h"
 #include "SkWriteBuffer.h"
@@ -197,7 +195,7 @@ SkImageFilter::~SkImageFilter() {
         SkSafeUnref(fInputs[i]);
     }
     delete[] fInputs;
-    Cache::Get()->purgeByImageFilterId(fUniqueID);
+    Cache::Get()->purgeByKeys(fCacheKeys.begin(), fCacheKeys.count());
 }
 
 SkImageFilter::SkImageFilter(int inputCount, SkReadBuffer& buffer)
@@ -253,6 +251,8 @@ bool SkImageFilter::filterImage(Proxy* proxy, const SkBitmap& src,
         this->onFilterImage(proxy, src, context, result, offset)) {
         if (context.cache()) {
             context.cache()->set(key, *result, *offset);
+            SkAutoMutexAcquire mutex(fMutex);
+            fCacheKeys.push_back(key);
         }
         return true;
     }
@@ -547,7 +547,6 @@ public:
             ++iter;
             delete v;
         }
-        fIdToKeys.foreach([](uint32_t, SkTArray<Key>** array) { delete *array; });
     }
     struct Value {
         Value(const Key& key, const SkBitmap& bitmap, const SkIPoint& offset)
@@ -582,13 +581,6 @@ public:
             removeInternal(v);
         }
         Value* v = new Value(key, result, offset);
-        if (SkTArray<Key>** array = fIdToKeys.find(key.fUniqueID)) {
-            (*array)->push_back(key);
-        } else {
-            SkTArray<Key>* keyArray = new SkTArray<Key>();
-            keyArray->push_back(key);
-            fIdToKeys.set(key.fUniqueID, keyArray);
-        }
         fLookup.add(v);
         fLRU.addToHead(v);
         fCurrentBytes += result.getSize();
@@ -611,16 +603,12 @@ public:
         }
     }
 
-    void purgeByImageFilterId(uint32_t uniqueID) override {
+    void purgeByKeys(const Key keys[], int count) override {
         SkAutoMutexAcquire mutex(fMutex);
-        if (SkTArray<Key>** array = fIdToKeys.find(uniqueID)) {
-            for (auto& key : **array) {
-                if (Value* v = fLookup.find(key)) {
-                    this->removeInternal(v);
-                }
+        for (int i = 0; i < count; i++) {
+            if (Value* v = fLookup.find(keys[i])) {
+                this->removeInternal(v);
             }
-            fIdToKeys.remove(uniqueID);
-            delete *array; // This can be deleted outside the lock
         }
     }
 
@@ -633,7 +621,6 @@ private:
     }
 private:
     SkTDynamicHash<Value, Key>            fLookup;
-    SkTHashMap<uint32_t, SkTArray<Key>*>  fIdToKeys;
     mutable SkTInternalLList<Value>       fLRU;
     size_t                                fMaxBytes;
     size_t                                fCurrentBytes;
-- 
2.7.4