First pass at fix for ER-401: Fix object cache header on the viewer

2 files changed, 106 insertions, 65 deletions

diff --git a/indra/newview/llvocache.cpp b/indra/newview/llvocache.cpp
index 145ee31260..6ea88abab8 100644
--- a/indra/newview/llvocache.cpp
+++ b/indra/newview/llvocache.cpp
@@ -40,6 +40,7 @@ BOOL check_write(LLAPRFile* apr_file, void* src, S32 n_bytes)
 	return apr_file->write(src, n_bytes) == n_bytes ;
 }
 
+
 //---------------------------------------------------------------------------
 // LLVOCacheEntry
 //---------------------------------------------------------------------------
@@ -212,8 +213,8 @@ BOOL LLVOCacheEntry::writeToFile(LLAPRFile* apr_file) const
 		if(success)
 		{
 			success = check_write(apr_file, (void*)mBuffer, size);
+		}
 	}
-}
 
 	return success ;
 }
@@ -224,7 +225,7 @@ BOOL LLVOCacheEntry::writeToFile(LLAPRFile* apr_file) const
 // Format string used to construct filename for the object cache
 static const char OBJECT_CACHE_FILENAME[] = "objects_%d_%d.slc";
 
-const U32 NUM_ENTRIES_TO_PURGE = 16 ;
+const U32 NUM_ENTRIES_TO_PURGE = 50;
 const char* object_cache_dirname = "objectcache";
 const char* header_filename = "object.cache";
 
@@ -259,7 +260,6 @@ void LLVOCache::destroyClass()
 LLVOCache::LLVOCache():
 	mInitialized(FALSE),
 	mReadOnly(TRUE),
-	mNumEntries(0),
 	mCacheSize(1)
 {
 	mEnabled = gSavedSettings.getBOOL("ObjectCacheEnabled");
@@ -286,8 +286,15 @@ void LLVOCache::setDirNames(ELLPath location)
 
 void LLVOCache::initCache(ELLPath location, U32 size, U32 cache_version)
 {
-	if(mInitialized || !mEnabled)
+	if(!mEnabled)
+	{
+		llwarns << "Not initializing cache: Cache is currently disabled." << llendl;
+		return ;
+	}
+
+	if(mInitialized)
 	{
+		llwarns << "Cache already initialized." << llendl;
 		return ;
 	}
 
@@ -321,6 +328,7 @@ void LLVOCache::removeCache(ELLPath location)
 {
 	if(mReadOnly)
 	{
+		llwarns << "Not removing cache at " << location << ": Cache is currently in read-only mode." << llendl;
 		return ;
 	}
 
@@ -339,6 +347,7 @@ void LLVOCache::removeCache()
 	llassert_always(mInitialized) ;
 	if(mReadOnly)
 	{
+		llwarns << "Not clearing object cache: Cache is currently in read-only mode." << llendl;
 		return ;
 	}
 
@@ -352,16 +361,8 @@ void LLVOCache::removeCache()
 
 void LLVOCache::clearCacheInMemory()
 {
-	if(!mHeaderEntryQueue.empty()) 
-	{
-		for(header_entry_queue_t::iterator iter = mHeaderEntryQueue.begin(); iter != mHeaderEntryQueue.end(); ++iter)
-		{
-			delete *iter ;
-		}
-		mHeaderEntryQueue.clear();
-		mHandleEntryMap.clear();
-		mNumEntries = 0 ;
-	}
+	std::for_each(mHandleEntryMap.begin(), mHandleEntryMap.end(), DeletePairedPointer());
+	mHandleEntryMap.clear();
 }
 
 void LLVOCache::getObjectCacheFilename(U64 handle, std::string& filename) 
@@ -379,6 +380,7 @@ void LLVOCache::removeFromCache(U64 handle)
 {
 	if(mReadOnly)
 	{
+		llwarns << "Not removing cache for handle " << handle << ": Cache is currently in read-only mode." << llendl;
 		return ;
 	}
 
@@ -391,7 +393,6 @@ BOOL LLVOCache::checkRead(LLAPRFile* apr_file, void* src, S32 n_bytes)
 {
 	if(!check_read(apr_file, src, n_bytes))
 	{
-		delete apr_file ;
 		removeCache() ;
 		return FALSE ;
 	}
@@ -403,7 +404,6 @@ BOOL LLVOCache::checkWrite(LLAPRFile* apr_file, void* src, S32 n_bytes)
 {
 	if(!check_write(apr_file, src, n_bytes))
 	{
-		delete apr_file ;
 		removeCache() ;
 		return FALSE ;
 	}
@@ -415,7 +415,8 @@ void LLVOCache::readCacheHeader()
 {
 	if(!mEnabled)
 	{
-		return ;
+		llwarns << "Not reading cache header: Cache is currently disabled." << llendl;
+		return;
 	}
 
 	//clear stale info.
@@ -428,28 +429,29 @@ void LLVOCache::readCacheHeader()
 		//read the meta element
 		if(!checkRead(apr_file, &mMetaInfo, sizeof(HeaderMetaInfo)))
 		{
-			return ;
+			llwarns << "Error reading meta information from cache header." << llendl;
+			delete apr_file;
+			return;
 		}
 
 		HeaderEntryInfo* entry ;
-		mNumEntries = 0 ;
-		while(mNumEntries < mCacheSize)
+		for(U32 entry_index = 0; entry_index < mCacheSize; ++entry_index)
 		{
 			entry = new HeaderEntryInfo() ;
 			if(!checkRead(apr_file, entry, sizeof(HeaderEntryInfo)))
 			{
+				llwarns << "Error reading cache header entry. (entry_index=" << entry_index << ")" << llendl;
 				delete entry ;			
-				return ;
+				break;
 			}
 			else if(!entry->mTime) //end of the cache.
 			{
 				delete entry ;
-				return ;
+				break;
 			}
 
-			entry->mIndex = mNumEntries++ ;
-			mHeaderEntryQueue.insert(entry) ;
-			mHandleEntryMap[entry->mHandle] = entry ;
+			entry->mIndex = entry_index;
+			mHandleEntryMap[entry->mHandle] = entry;
 		}
 
 		delete apr_file ;
@@ -462,40 +464,57 @@ void LLVOCache::readCacheHeader()
 
 void LLVOCache::writeCacheHeader()
 {
-	if(mReadOnly || !mEnabled)
+	if (!mEnabled)
 	{
-		return ;
-	}	
+		llwarns << "Not writing cache header: Cache is currently disabled." << llendl;
+		return;
+	}
+
+	if(mReadOnly)
+	{
+		llwarns << "Not writing cache header: Cache is currently in read-only mode." << llendl;
+		return;
+	}
 
 	LLAPRFile* apr_file = new LLAPRFile(mHeaderFileName, APR_CREATE|APR_WRITE|APR_BINARY, mLocalAPRFilePoolp);
 
 	//write the meta element
 	if(!checkWrite(apr_file, &mMetaInfo, sizeof(HeaderMetaInfo)))
 	{
-		return ;
+		llwarns << "Error writing meta information to cache header." << llendl;
+		delete apr_file;
+		return;
 	}
 
-	mNumEntries = 0 ;
-	for(header_entry_queue_t::iterator iter = mHeaderEntryQueue.begin() ; iter != mHeaderEntryQueue.end(); ++iter)
+	U32 entry_index = 0;
+	handle_entry_map_t::iterator iter_end = mHandleEntryMap.end();
+	for(handle_entry_map_t::iterator iter = mHandleEntryMap.begin();
+		iter != iter_end;
+		++iter)
 	{
-		(*iter)->mIndex = mNumEntries++ ;
-		if(!checkWrite(apr_file, (void*)*iter, sizeof(HeaderEntryInfo)))
+		HeaderEntryInfo* entry = iter->second;
+		entry->mIndex = entry_index++;
+		if(!checkWrite(apr_file, (void*)entry, sizeof(HeaderEntryInfo)))
 		{
-			return ;
+			llwarns << "Failed to write cache header for entry " << entry->mHandle << " (entry_index = " << entry_index << ")" << llendl;
+			delete apr_file;
+			return;
 		}
 	}
 
-	mNumEntries = mHeaderEntryQueue.size() ;
-	if(mNumEntries < mCacheSize)
+	// Why do we need to fill the cache header with default entries?  DK 2010-12-14
+	// It looks like we currently rely on the file being pre-allocated so we can seek during updateEntry().
+	if(entry_index < mCacheSize)
 	{
 		HeaderEntryInfo* entry = new HeaderEntryInfo() ;
-		for(U32 i = mNumEntries ; i < mCacheSize; i++)
+		for(; entry_index < mCacheSize; ++entry_index)
 		{
 			//fill the cache with the default entry.
 			if(!checkWrite(apr_file, entry, sizeof(HeaderEntryInfo)))
 			{
+				llwarns << "Failed to fill cache header with default entries (entry_index = " << entry_index << ").  Switching to read-only mode." << llendl;
 				mReadOnly = TRUE ; //disable the cache.
-				return ;
+				break;
 			}
 		}
 		delete entry ;
@@ -508,13 +527,16 @@ BOOL LLVOCache::updateEntry(const HeaderEntryInfo* entry)
 	LLAPRFile* apr_file = new LLAPRFile(mHeaderFileName, APR_WRITE|APR_BINARY, mLocalAPRFilePoolp);
 	apr_file->seek(APR_SET, entry->mIndex * sizeof(HeaderEntryInfo) + sizeof(HeaderMetaInfo)) ;
 
-	return checkWrite(apr_file, (void*)entry, sizeof(HeaderEntryInfo)) ;
+	BOOL result = checkWrite(apr_file, (void*)entry, sizeof(HeaderEntryInfo)) ;
+	delete apr_file;
+	return result;
 }
 
 void LLVOCache::readFromCache(U64 handle, const LLUUID& id, LLVOCacheEntry::vocache_entry_map_t& cache_entry_map) 
 {
 	if(!mEnabled)
 	{
+		llwarns << "Not reading cache for handle " << handle << "): Cache is currently disabled." << llendl;
 		return ;
 	}
 	llassert_always(mInitialized);
@@ -522,6 +544,7 @@ void LLVOCache::readFromCache(U64 handle, const LLUUID& id, LLVOCacheEntry::voca
 	handle_entry_map_t::iterator iter = mHandleEntryMap.find(handle) ;
 	if(iter == mHandleEntryMap.end()) //no cache
 	{
+		llwarns << "No handle map entry for " << handle << llendl;
 		return ;
 	}
 
@@ -532,12 +555,13 @@ void LLVOCache::readFromCache(U64 handle, const LLUUID& id, LLVOCacheEntry::voca
 	LLUUID cache_id ;
 	if(!checkRead(apr_file, cache_id.mData, UUID_BYTES))
 	{
+		llwarns << "Error reading cache_id from " << filename << llendl;
+		delete apr_file;
 		return ;
 	}
 	if(cache_id != id)
 	{
-		llinfos << "Cache ID doesn't match for this region, discarding"<< llendl;
-
+		llwarns << "Cache ID (" << cache_id << ") doesn't match id for this region (" << id << "), discarding.  handle = " << handle << llendl;
 		delete apr_file ;
 		return ;
 	}
@@ -545,6 +569,8 @@ void LLVOCache::readFromCache(U64 handle, const LLUUID& id, LLVOCacheEntry::voca
 	S32 num_entries;
 	if(!checkRead(apr_file, &num_entries, sizeof(S32)))
 	{
+		llwarns << "Error reading num_entries from " << filename << llendl;
+		delete apr_file;
 		return ;
 	}
 	
@@ -553,13 +579,12 @@ void LLVOCache::readFromCache(U64 handle, const LLUUID& id, LLVOCacheEntry::voca
 		LLVOCacheEntry* entry = new LLVOCacheEntry(apr_file);
 		if (!entry->getLocalID())
 		{
-			llwarns << "Aborting cache file load for " << filename << ", cache file corruption!" << llendl;
+			llwarns << "Aborting cache file load for " << filename << ", cache file corruption! (entry number = " << i << ")" << llendl;
 			delete entry ;
 			break;
 		}
 		cache_entry_map[entry->getLocalID()] = entry;
 	}
-	num_entries = cache_entry_map.size() ;
 
 	delete apr_file ;
 	return ;
@@ -568,40 +593,53 @@ void LLVOCache::readFromCache(U64 handle, const LLUUID& id, LLVOCacheEntry::voca
 void LLVOCache::purgeEntries()
 {
 	U32 limit = mCacheSize - NUM_ENTRIES_TO_PURGE ;
-	while(mHeaderEntryQueue.size() > limit)
+	// Construct a vector of entries out of the map so we can sort by time.
+	std::vector<HeaderEntryInfo*> header_vector;
+	handle_entry_map_t::iterator iter_end = mHandleEntryMap.end();
+	for (handle_entry_map_t::iterator iter = mHandleEntryMap.begin();
+		iter != iter_end;
+		++iter)
+	{
+		header_vector.push_back(iter->second);
+	}
+	// Sort by time, oldest first.
+	std::sort(header_vector.begin(), header_vector.end(), header_entry_less());
+	while(header_vector.size() > limit)
 	{
-		header_entry_queue_t::iterator iter = mHeaderEntryQueue.begin() ;
-		HeaderEntryInfo* entry = *iter ;
+		HeaderEntryInfo* entry = header_vector.front();
 		
-		removeFromCache(entry->mHandle) ;
-		mHandleEntryMap.erase(entry->mHandle) ;		
-		mHeaderEntryQueue.erase(iter) ;
-		delete entry ;
+		removeFromCache(entry->mHandle);
+		mHandleEntryMap.erase(entry->mHandle);
+		header_vector.erase(header_vector.begin());
+		delete entry;
 	}
 
 	writeCacheHeader() ;
+	// *TODO: Verify that we can avoid re-reading the cache header.  DK 2010-12-14
 	readCacheHeader() ;
-	mNumEntries = mHandleEntryMap.size() ;
 }
 
 void LLVOCache::writeToCache(U64 handle, const LLUUID& id, const LLVOCacheEntry::vocache_entry_map_t& cache_entry_map, BOOL dirty_cache) 
 {
 	if(!mEnabled)
 	{
+		llwarns << "Not writing cache for handle " << handle << "): Cache is currently disabled." << llendl;
 		return ;
 	}
 	llassert_always(mInitialized);
 
 	if(mReadOnly)
 	{
+		llwarns << "Not writing cache for handle " << handle << "): Cache is currently in read-only mode." << llendl;
 		return ;
 	}
 
 	HeaderEntryInfo* entry;
 	handle_entry_map_t::iterator iter = mHandleEntryMap.find(handle) ;
+	U32 num_handle_entries = mHandleEntryMap.size();
 	if(iter == mHandleEntryMap.end()) //new entry
-	{		
-		if(mNumEntries >= mCacheSize)
+	{
+		if(num_handle_entries >= mCacheSize)
 		{
 			purgeEntries() ;
 		}
@@ -609,28 +647,26 @@ void LLVOCache::writeToCache(U64 handle, const LLUUID& id, const LLVOCacheEntry:
 		entry = new HeaderEntryInfo();
 		entry->mHandle = handle ;
 		entry->mTime = time(NULL) ;
-		entry->mIndex = mNumEntries++ ;
-		mHeaderEntryQueue.insert(entry) ;
+		entry->mIndex = num_handle_entries++;
 		mHandleEntryMap[handle] = entry ;
 	}
 	else
 	{
+		// Update access time.
 		entry = iter->second ;
 		entry->mTime = time(NULL) ;
-
-		//resort
-		mHeaderEntryQueue.erase(entry) ;
-		mHeaderEntryQueue.insert(entry) ;
 	}
 
 	//update cache header
 	if(!updateEntry(entry))
 	{
+		llwarns << "Failed to update cache header index " << entry->mIndex << ". handle = " << handle << llendl;
 		return ; //update failed.
 	}
 
 	if(!dirty_cache)
 	{
+		llwarns << "Skipping write to cache for handle " << handle << ": cache not dirty" << llendl;
 		return ; //nothing changed, no need to update.
 	}
 
@@ -641,12 +677,16 @@ void LLVOCache::writeToCache(U64 handle, const LLUUID& id, const LLVOCacheEntry:
 	
 	if(!checkWrite(apr_file, (void*)id.mData, UUID_BYTES))
 	{
+		llwarns << "Error writing id to " << filename << llendl;
+		delete apr_file;
 		return ;
 	}
 
 	S32 num_entries = cache_entry_map.size() ;
 	if(!checkWrite(apr_file, &num_entries, sizeof(S32)))
 	{
+		llwarns << "Error writing num_entries to " << filename << llendl;
+		delete apr_file;
 		return ;
 	}
 
@@ -654,10 +694,10 @@ void LLVOCache::writeToCache(U64 handle, const LLUUID& id, const LLVOCacheEntry:
 	{
 		if(!iter->second->writeToFile(apr_file))
 		{
+			llwarns << "Aborting cache file write for " << filename << ", error writing to file!" << llendl;
 			//failed
-			delete apr_file ;
 			removeCache() ;
-			return ;
+			break;
 		}
 	}
 
diff --git a/indra/newview/llvocache.h b/indra/newview/llvocache.h
index ed2bc8bafe..014112718e 100644
--- a/indra/newview/llvocache.h
+++ b/indra/newview/llvocache.h
@@ -95,10 +95,13 @@ private:
 	{
 		bool operator()(const HeaderEntryInfo* lhs, const HeaderEntryInfo* rhs) const
 		{
-			return lhs->mTime < rhs->mTime; // older entry in front of queue (set)
+			if (lhs->mTime == rhs->mTime)
+			{
+				return lhs->mHandle < rhs->mHandle;
+			}
+			return lhs->mTime < rhs->mTime; // older entry in front
 		}
 	};
-	typedef std::set<HeaderEntryInfo*, header_entry_less> header_entry_queue_t;
 	typedef std::map<U64, HeaderEntryInfo*> handle_entry_map_t;
 private:
 	LLVOCache() ;
@@ -134,11 +137,9 @@ private:
 	BOOL                 mReadOnly ;
 	HeaderMetaInfo       mMetaInfo;
 	U32                  mCacheSize;
-	U32                  mNumEntries;
 	std::string          mHeaderFileName ;
 	std::string          mObjectCacheDirName;
 	LLVolatileAPRPool*   mLocalAPRFilePoolp ; 	
-	header_entry_queue_t mHeaderEntryQueue;
 	handle_entry_map_t   mHandleEntryMap;	
 
 	static LLVOCache* sInstance ;