Port SkResourceCache to SkTHashTable

We'd like to fix bugs and make performance improvements to our hash tables.  It's a lot easier if we can focus on one implementation, so I'd like to move users of SkTDynamicHash to SkTHashTable, SkTHashMap, or SkTHashSet.  This is roughly outlined in the attached Skia bug.

In this case, the conversion from SkTDynamicHash to SkTHashTable is pretty trivial.  The main change is that the values stored in the table are no longer assumed to be pointers, so we just need to sprinkle in a couple of * and ->.

SkResourceCache is particularly interesting as the locus of the attached Chromium bug.  Porting this now means SkResourceCache will get any fixes we make as soon as we make them.

BUG=skia:6053,chromium:429375

Change-Id: If5dc8d331c62f1d4449fb8f9a7f7e9c746070213
Reviewed-on: https://skia-review.googlesource.com/5984
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@chromium.org>
This commit is contained in:
Mike Klein 2016-12-13 16:26:32 -05:00 committed by Skia Commit-Bot
parent 5adaf8bf24
commit e4bf164225
2 changed files with 15 additions and 11 deletions

View File

@ -51,10 +51,19 @@ void SkResourceCache::Key::init(void* nameSpace, uint64_t sharedID, size_t dataS
(fCount32 - kUnhashedLocal32s) << 2);
}
#include "SkTDynamicHash.h"
#include "SkTHash.h"
namespace {
struct HashTraits {
static uint32_t Hash(const SkResourceCache::Key& key) { return key.hash(); }
static const SkResourceCache::Key& GetKey(const SkResourceCache::Rec* rec) {
return rec->getKey();
}
};
}
class SkResourceCache::Hash :
public SkTDynamicHash<SkResourceCache::Rec, SkResourceCache::Key> {};
public SkTHashTable<SkResourceCache::Rec*, SkResourceCache::Key, HashTraits> {};
///////////////////////////////////////////////////////////////////////////////
@ -224,8 +233,8 @@ SkResourceCache::~SkResourceCache() {
bool SkResourceCache::find(const Key& key, FindVisitor visitor, void* context) {
this->checkMessages();
Rec* rec = fHash->find(key);
if (rec) {
if (auto found = fHash->find(key)) {
Rec* rec = *found;
if (visitor(*rec, context)) {
this->moveToHead(rec); // for our LRU
return true;
@ -254,14 +263,13 @@ void SkResourceCache::add(Rec* rec) {
SkASSERT(rec);
// See if we already have this key (racy inserts, etc.)
Rec* existing = fHash->find(rec->getKey());
if (existing) {
if (nullptr != fHash->find(rec->getKey())) {
delete rec;
return;
}
this->addToHead(rec);
fHash->add(rec);
fHash->set(rec);
if (gDumpCacheTransactions) {
SkString bytesStr, totalStr;

View File

@ -86,10 +86,6 @@ public:
virtual const char* getCategory() const = 0;
virtual SkDiscardableMemory* diagnostic_only_getDiscardable() const { return nullptr; }
// for SkTDynamicHash::Traits
static uint32_t Hash(const Key& key) { return key.hash(); }
static const Key& GetKey(const Rec& rec) { return rec.getKey(); }
private:
Rec* fNext;
Rec* fPrev;