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

@@ -31,30 +31,35 @@ public:
 	typedef boost::weak_ptr<data_type>		weak_data_ptr;
 	typedef boost::shared_ptr<data_type>	data_ptr;
 
-	typedef bool (*visitor_func)(const c_Key&, c_Data&);
-
 protected:
 
-	typedef std::pair<time_t, data_ptr>						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<key_type, cache_entry>				cache_pair;
 	typedef boost::unordered_map<key_type, cache_entry>		cache_type;
 	typedef typename cache_type::iterator					cache_iterator;
-	typedef boost::unordered_map<key_type, weak_data_ptr> 	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<typename c_Key, typename c_Data> void TaggedCache<c_Key, c_Data>::setTa
 template<typename c_Key, typename c_Data> int TaggedCache<c_Key, c_Data>::getCacheSize()
 {
 	boost::recursive_mutex::scoped_lock sl(mLock);
-	return mCache.size();
+	return mCacheCount;
 }
 
 template<typename c_Key, typename c_Data> int TaggedCache<c_Key, c_Data>::getTrackSize()
 {
 	boost::recursive_mutex::scoped_lock sl(mLock);
-	return mMap.size();
+	return mCache.size();
 }
 
 template<typename c_Key, typename c_Data> void TaggedCache<c_Key, c_Data>::sweep()
@@ -123,134 +126,106 @@ template<typename c_Key, typename c_Data> void TaggedCache<c_Key, c_Data>::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<typename c_Key, typename c_Data> void TaggedCache<c_Key, c_Data>::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<typename c_Key, typename c_Data> void TaggedCache<c_Key, c_Data>::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<typename c_Key, typename c_Data> bool TaggedCache<c_Key, c_Data>::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<typename c_Key, typename c_Data> bool TaggedCache<c_Key, c_Data>::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<typename c_Key, typename c_Data>
@@ -259,40 +234,50 @@ bool TaggedCache<c_Key, c_Data>::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<typename c_Key, typename c_Data>
@@ -300,29 +285,22 @@ boost::shared_ptr<c_Data> TaggedCache<c_Key, c_Data>::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<typename c_Key, typename c_Data>