From 1f3479a87d6814745f4de6ed4125cbb0395ce0bb Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Thu, 31 Jan 2013 00:05:29 -0800 Subject: [PATCH] Rewrite the TaggedCache code to integrate the map and cache. Benchmarking showed the use of a separate map and cache was expensive. The 'visitor' code is gone, but nobody used it and it can be replaced if needed. --- src/cpp/ripple/TaggedCache.h | 276 ++++++++++++++++------------------- 1 file changed, 127 insertions(+), 149 deletions(-) diff --git a/src/cpp/ripple/TaggedCache.h b/src/cpp/ripple/TaggedCache.h index a5930494f..2a0e6a719 100644 --- a/src/cpp/ripple/TaggedCache.h +++ b/src/cpp/ripple/TaggedCache.h @@ -31,30 +31,35 @@ public: typedef boost::weak_ptr weak_data_ptr; typedef boost::shared_ptr data_ptr; - typedef bool (*visitor_func)(const c_Key&, c_Data&); - protected: - typedef std::pair cache_entry; + class cache_entry + { + public: + time_t last_use; + data_ptr ptr; + weak_data_ptr weak_ptr; + + cache_entry(time_t l, const data_ptr& d) : last_use(l), ptr(d), weak_ptr(d) { ; } + }; + typedef std::pair cache_pair; typedef boost::unordered_map cache_type; typedef typename cache_type::iterator cache_iterator; - typedef boost::unordered_map map_type; - typedef typename map_type::iterator map_iterator; mutable boost::recursive_mutex mLock; std::string mName; // Used for logging unsigned int mTargetSize; // Desired number of cache entries (0 = ignore) int mTargetAge; // Desired maximum cache age + int mCacheCount; // Number of items cached cache_type mCache; // Hold strong reference to recent objects - map_type mMap; // Track stored objects time_t mLastSweep; public: TaggedCache(const char *name, int size, int age) - : mName(name), mTargetSize(size), mTargetAge(age), mLastSweep(time(NULL)) { ; } + : mName(name), mTargetSize(size), mTargetAge(age), mCacheCount(0), mLastSweep(time(NULL)) { ; } int getTargetSize() const; int getTargetAge() const; @@ -66,8 +71,6 @@ public: void setTargetSize(int size); void setTargetAge(int age); void sweep(); - void visitAll(visitor_func); // Visits all tracked objects, removes selected objects - void visitCached(visitor_func); // Visits all cached objects, uncaches selected objects bool touch(const key_type& key); bool del(const key_type& key, bool valid); @@ -108,13 +111,13 @@ template void TaggedCache::setTa template int TaggedCache::getCacheSize() { boost::recursive_mutex::scoped_lock sl(mLock); - return mCache.size(); + return mCacheCount; } template int TaggedCache::getTrackSize() { boost::recursive_mutex::scoped_lock sl(mLock); - return mMap.size(); + return mCache.size(); } template void TaggedCache::sweep() @@ -123,134 +126,106 @@ template void TaggedCache::sweep time_t mLastSweep = time(NULL); time_t target = mLastSweep - mTargetAge; + int cacheRemovals = 0, mapRemovals = 0, cc = 0; - // Pass 1, remove old objects from cache - int cacheRemovals = 0; - if ((mTargetSize == 0) || (mCache.size() > mTargetSize)) + if ((mTargetSize != 0) && (mCache.size() > mTargetSize)) { - if (mTargetSize != 0) - { - target = mLastSweep - (mTargetAge * mTargetSize / mCache.size()); - if (target > (mLastSweep - 2)) - target = mLastSweep - 2; + target = mLastSweep - (mTargetAge * mTargetSize / mCache.size()); + if (target > (mLastSweep - 2)) + target = mLastSweep - 2; + Log(lsINFO, TaggedCachePartition) << mName << " is growing fast " << + mCache.size() << " of " << mTargetSize << + " aging at " << (mLastSweep - target) << " of " << mTargetAge; + } - Log(lsINFO, TaggedCachePartition) << mName << " is growing fast " << - mCache.size() << " of " << mTargetSize << - " aging at " << (mLastSweep - target) << " of " << mTargetAge; - } - else - target = mLastSweep - mTargetAge; - - cache_iterator cit = mCache.begin(); - while (cit != mCache.end()) - { - if (cit->second.first < target) + cache_iterator cit = mCache.begin(); + while (cit != mCache.end()) + { + if (!cit->second.ptr) + { // weak + if (cit->second.weak_ptr.expired()) { - ++cacheRemovals; + ++mapRemovals; mCache.erase(cit++); } else ++cit; } - } - - // Pass 2, remove dead objects from map - int mapRemovals = 0; - map_iterator mit = mMap.begin(); - while (mit != mMap.end()) - { - if (mit->second.expired()) - { - ++mapRemovals; - mMap.erase(mit++); - } else - ++mit; + { // strong + if (cit->second.last_use < target) + { // expired + --mCacheCount; + ++cacheRemovals; + cit->second.ptr.reset(); + if (cit->second.weak_ptr.expired()) + { + ++mapRemovals; + mCache.erase(cit++); + } + else + ++cit; + } + else + { + ++cc; + ++cit; + } + } } + assert(cc == mCacheCount); if (TaggedCachePartition.doLog(lsTRACE) && (mapRemovals || cacheRemovals)) Log(lsTRACE, TaggedCachePartition) << mName << ": cache = " << mCache.size() << "-" << cacheRemovals << - ", map = " << mMap.size() << "-" << mapRemovals; -} - -template void TaggedCache::visitAll(visitor_func func) -{ // Visits all tracked objects, removes selected objects - boost::recursive_mutex::scoped_lock sl(mLock); - - map_iterator mit = mMap.begin(); - while (mit != mMap.end()) - { - data_ptr cachedData = mit->second.lock(); - if (!cachedData) - mMap.erase(mit++); // dead reference found - else if (func(mit->first, mit->second)) - { - mCache.erase(mit->first); - mMap.erase(mit++); - } - else - ++mit; - } -} - -template void TaggedCache::visitCached(visitor_func func) -{ // Visits all cached objects, uncaches selected objects - boost::recursive_mutex::scoped_lock sl(mLock); - - cache_iterator cit = mCache.begin(); - while (cit != mCache.end()) - { - if (func(cit->first, cit->second.second)) - mCache.erase(cit++); - else - ++cit; - } + ", map-=" << mapRemovals; } template bool TaggedCache::touch(const key_type& key) { // If present, make current in cache boost::recursive_mutex::scoped_lock sl(mLock); - // Is the object in the map? - map_iterator mit = mMap.find(key); - if (mit == mMap.end()) - return false; - if (mit->second.expired()) - { // in map, but expired - mMap.erase(mit); - return false; - } - - // Is the object in the cache? cache_iterator cit = mCache.find(key); - if (cit != mCache.end()) - { // in both map and cache - cit->second.first = time(NULL); + if (cit == mCache.end()) // Don't have the object + return false; + + if (cit->second.ptr) + { // Have the object in cache + cit->second.last_use = time(NULL); return true; } - // In map but not cache, put in cache - mCache.insert(cache_pair(key, cache_entry(time(NULL), data_ptr(cit->second.second)))); - return true; + cit->second.ptr = cit->second.weak_ptr.lock(); + if (cit->second.ptr) + { // We just put the object back in cache + ++mCacheCount; + cit->second.last_use = time(NULL); + return true; + } + + // Object fell out + mCache.erase(cit); + return false; } template bool TaggedCache::del(const key_type& key, bool valid) { // Remove from cache, if !valid, remove from map too. Returns true if removed from cache boost::recursive_mutex::scoped_lock sl(mLock); - if (!valid) - { // remove from map too - map_iterator mit = mMap.find(key); - if (mit == mMap.end()) // not in map, cannot be in cache - return false; - mMap.erase(mit); - } - cache_iterator cit = mCache.find(key); if (cit == mCache.end()) return false; - mCache.erase(cit); - return true; + + bool ret = false; + if (cit->second.ptr) + { + --mCacheCount; + cit->second.reset(); + ret = true; + } + + if (!valid || cit->second.weak_ptr.expired()) + mCache.erase(cit); + return true; } template @@ -259,40 +234,50 @@ bool TaggedCache::canonicalize(const key_type& key, boost::shared // Return values: true=we had the data already boost::recursive_mutex::scoped_lock sl(mLock); - map_iterator mit = mMap.find(key); - if (mit == mMap.end()) - { // not in map + cache_iterator cit = mCache.find(key); + if (cit == mCache.end()) + { mCache.insert(cache_pair(key, cache_entry(time(NULL), data))); - mMap.insert(std::make_pair(key, data)); + ++mCacheCount; return false; } - data_ptr cachedData = mit->second.lock(); - if (!cachedData) - { // in map, but expired. Update in map, insert in cache - mit->second = data; - mCache.insert(cache_pair(key, cache_entry(time(NULL), data))); + cit->second.last_use = time(NULL); + + if (cit->second.ptr) + { + if (replace) + { + cit->second.ptr = data; + cit->second.weak_ptr = data; + } + else + data = cit->second.ptr; return true; } - // in map and cache, canonicalize - if (replace) - mit->second = data; - else - data = cachedData; - - // Valid in map, is it in cache? - cache_iterator cit = mCache.find(key); - if (cit != mCache.end()) + data_ptr cachedData = cit->second.weak_ptr.lock(); + if (cachedData) { - cit->second.first = time(NULL); // Yes, refesh if (replace) - cit->second.second = data; + { + cit->second.ptr = data; + cit->second.weak_ptr = data; + } + else + { + cit->second.ptr = cachedData; + data = cachedData; + } + ++mCacheCount; + return true; } - else // no, add to cache - mCache.insert(cache_pair(key, cache_entry(time(NULL), data))); - return true; + cit->second.ptr = data; + cit->second.weak_ptr = data; + ++mCacheCount; + + return false; } template @@ -300,29 +285,22 @@ boost::shared_ptr TaggedCache::fetch(const key_type& key) { // fetch us a shared pointer to the stored data object boost::recursive_mutex::scoped_lock sl(mLock); - // Is it in the cache? cache_iterator cit = mCache.find(key); - if (cit != mCache.end()) + if (cit == mCache.end()) + return data_ptr(); + cit->second.last_use = time(NULL); + + if (cit->second.ptr) + return cit->second.ptr; + + cit->second.ptr = cit->second.weak_ptr.lock(); + if (cit->second.ptr) { - cit->second.first = time(NULL); // Yes, refresh - return cit->second.second; + ++mCacheCount; + return cit->second.ptr; } - - // Is it in the map? - map_iterator mit = mMap.find(key); - if (mit == mMap.end()) - return data_ptr(); // No, we're done - - data_ptr cachedData = mit->second.lock(); - if (!cachedData) - { // in map, but expired. Sorry, we don't have it - mMap.erase(mit); - return cachedData; - } - - // Put it back in the cache - mCache.insert(cache_pair(key, cache_entry(time(NULL), cachedData))); - return cachedData; + mCache.erase(cit); + return data_ptr(); } template