From bb58cde899dc331d0146faa8a85e7fe2e8f1f490 Mon Sep 17 00:00:00 2001 From: Sergio Giro Date: Fri, 25 Sep 2015 18:03:18 +0100 Subject: [PATCH] system/core: change LruCache to use unordered_set instead of BasicHashTable Towards deprecation of BasicHashTable Change-Id: I3f3d904636a85a794cccf972f6e4a94e7f3808f5 --- include/utils/LruCache.h | 153 +++++++++++++++++-------------- libutils/tests/LruCache_test.cpp | 2 +- 2 files changed, 83 insertions(+), 72 deletions(-) diff --git a/include/utils/LruCache.h b/include/utils/LruCache.h index cd9d7f94a..7818b4e2e 100644 --- a/include/utils/LruCache.h +++ b/include/utils/LruCache.h @@ -17,8 +17,11 @@ #ifndef ANDROID_UTILS_LRU_CACHE_H #define ANDROID_UTILS_LRU_CACHE_H +#include + #include -#include + +#include "utils/TypeHelpers.h" // hash_t namespace android { @@ -36,6 +39,7 @@ template class LruCache { public: explicit LruCache(uint32_t maxCapacity); + virtual ~LruCache(); enum Capacity { kUnlimitedCapacity, @@ -50,32 +54,6 @@ public: void clear(); const TValue& peekOldestValue(); - class Iterator { - public: - Iterator(const LruCache& cache): mCache(cache), mIndex(-1) { - } - - bool next() { - mIndex = mCache.mTable->next(mIndex); - return (ssize_t)mIndex != -1; - } - - size_t index() const { - return mIndex; - } - - const TValue& value() const { - return mCache.mTable->entryAt(mIndex).value; - } - - const TKey& key() const { - return mCache.mTable->entryAt(mIndex).key; - } - private: - const LruCache& mCache; - size_t mIndex; - }; - private: LruCache(const LruCache& that); // disallow copy constructor @@ -90,27 +68,79 @@ private: const TKey& getKey() const { return key; } }; + struct HashForEntry : public std::unary_function { + size_t operator() (const Entry* entry) const { + return hash_type(entry->key); + }; + }; + + struct EqualityForHashedEntries : public std::unary_function { + bool operator() (const Entry* lhs, const Entry* rhs) const { + return lhs->key == rhs->key; + }; + }; + + typedef std::unordered_set LruCacheSet; + void attachToCache(Entry& entry); void detachFromCache(Entry& entry); - void rehash(size_t newCapacity); - UniquePtr > mTable; + typename LruCacheSet::iterator findByKey(const TKey& key) { + Entry entryForSearch(key, mNullValue); + typename LruCacheSet::iterator result = mSet->find(&entryForSearch); + return result; + } + + UniquePtr mSet; OnEntryRemoved* mListener; Entry* mOldest; Entry* mYoungest; uint32_t mMaxCapacity; TValue mNullValue; + +public: + class Iterator { + public: + Iterator(const LruCache& cache): mCache(cache), mIterator(mCache.mSet->begin()) { + } + + bool next() { + if (mIterator == mCache.mSet->end()) { + return false; + } + std::advance(mIterator, 1); + return mIterator != mCache.mSet->end(); + } + + const TValue& value() const { + return (*mIterator)->value; + } + + const TKey& key() const { + return (*mIterator)->key; + } + private: + const LruCache& mCache; + typename LruCacheSet::iterator mIterator; + }; }; // Implementation is here, because it's fully templated template LruCache::LruCache(uint32_t maxCapacity) - : mTable(new BasicHashtable) + : mSet(new LruCacheSet()) , mListener(NULL) , mOldest(NULL) , mYoungest(NULL) , mMaxCapacity(maxCapacity) , mNullValue(NULL) { + mSet->max_load_factor(1.0); +}; + +template +LruCache::~LruCache() { + // Need to delete created entries. + clear(); }; template @@ -120,20 +150,19 @@ void LruCache::setOnEntryRemovedListener(OnEntryRemoved* listener) { template size_t LruCache::size() const { - return mTable->size(); + return mSet->size(); } template const TValue& LruCache::get(const TKey& key) { - hash_t hash = hash_type(key); - ssize_t index = mTable->find(-1, hash, key); - if (index == -1) { + typename LruCacheSet::const_iterator find_result = findByKey(key); + if (find_result == mSet->end()) { return mNullValue; } - Entry& entry = mTable->editEntryAt(index); - detachFromCache(entry); - attachToCache(entry); - return entry.value; + Entry *entry = *find_result; + detachFromCache(*entry); + attachToCache(*entry); + return entry->value; } template @@ -142,36 +171,29 @@ bool LruCache::put(const TKey& key, const TValue& value) { removeOldest(); } - hash_t hash = hash_type(key); - ssize_t index = mTable->find(-1, hash, key); - if (index >= 0) { + if (findByKey(key) != mSet->end()) { return false; } - if (!mTable->hasMoreRoom()) { - rehash(mTable->capacity() * 2); - } - // Would it be better to initialize a blank entry and assign key, value? - Entry initEntry(key, value); - index = mTable->add(hash, initEntry); - Entry& entry = mTable->editEntryAt(index); - attachToCache(entry); + Entry* newEntry = new Entry(key, value); + mSet->insert(newEntry); + attachToCache(*newEntry); return true; } template bool LruCache::remove(const TKey& key) { - hash_t hash = hash_type(key); - ssize_t index = mTable->find(-1, hash, key); - if (index < 0) { + typename LruCacheSet::const_iterator find_result = findByKey(key); + if (find_result == mSet->end()) { return false; } - Entry& entry = mTable->editEntryAt(index); + Entry* entry = *find_result; if (mListener) { - (*mListener)(entry.key, entry.value); + (*mListener)(entry->key, entry->value); } - detachFromCache(entry); - mTable->removeAt(index); + detachFromCache(*entry); + mSet->erase(entry); + delete entry; return true; } @@ -201,7 +223,10 @@ void LruCache::clear() { } mYoungest = NULL; mOldest = NULL; - mTable->clear(); + for (auto entry : *mSet.get()) { + delete entry; + } + mSet->clear(); } template @@ -232,19 +257,5 @@ void LruCache::detachFromCache(Entry& entry) { entry.child = NULL; } -template -void LruCache::rehash(size_t newCapacity) { - UniquePtr > oldTable(mTable.release()); - Entry* oldest = mOldest; - - mOldest = NULL; - mYoungest = NULL; - mTable.reset(new BasicHashtable(newCapacity)); - for (Entry* p = oldest; p != NULL; p = p->child) { - put(p->key, p->value); - } } - -} - #endif // ANDROID_UTILS_LRU_CACHE_H diff --git a/libutils/tests/LruCache_test.cpp b/libutils/tests/LruCache_test.cpp index 6155defd4..2ed84d700 100644 --- a/libutils/tests/LruCache_test.cpp +++ b/libutils/tests/LruCache_test.cpp @@ -221,7 +221,7 @@ TEST_F(LruCacheTest, NoLeak) { cache.put(ComplexKey(0), ComplexValue(0)); cache.put(ComplexKey(1), ComplexValue(1)); EXPECT_EQ(2U, cache.size()); - assertInstanceCount(2, 3); // the null value counts as an instance + assertInstanceCount(2, 3); // the member mNullValue counts as an instance } TEST_F(LruCacheTest, Clear) {