From 7372d4423f6c769f772d8508ab890de6d1a624bf Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Thu, 1 Nov 2012 07:14:45 -0700 Subject: [PATCH] Fix a memory leak. Better TaggedCache logging. --- src/Application.cpp | 4 ++-- src/HashedObject.cpp | 2 +- src/LedgerHistory.cpp | 2 +- src/TaggedCache.h | 29 ++++++++++++++++++++++------- src/TransactionMaster.cpp | 2 +- 5 files changed, 27 insertions(+), 12 deletions(-) diff --git a/src/Application.cpp b/src/Application.cpp index 758fae4ae..f4f1e99d1 100644 --- a/src/Application.cpp +++ b/src/Application.cpp @@ -18,7 +18,7 @@ #include SETUP_LOG(); - +LogPartition TaggedCachePartition("TaggedCache"); Application* theApp = NULL; DatabaseCon::DatabaseCon(const std::string& strName, const char *initStrings[], int initCount) @@ -39,7 +39,7 @@ DatabaseCon::~DatabaseCon() Application::Application() : mIOWork(mIOService), mAuxWork(mAuxService), mUNL(mIOService), - mNetOps(mIOService, &mMasterLedger), mTempNodeCache(16384, 90), mHashedObjectStore(16384, 300), + mNetOps(mIOService, &mMasterLedger), mTempNodeCache("NodeCache", 16384, 90), mHashedObjectStore(16384, 300), mSNTPClient(mAuxService), mRpcDB(NULL), mTxnDB(NULL), mLedgerDB(NULL), mWalletDB(NULL), mHashNodeDB(NULL), mNetNodeDB(NULL), mConnectionPool(mIOService), mPeerDoor(NULL), mRPCDoor(NULL), mSweepTimer(mAuxService) diff --git a/src/HashedObject.cpp b/src/HashedObject.cpp index bff9c34c6..9fc95a825 100644 --- a/src/HashedObject.cpp +++ b/src/HashedObject.cpp @@ -12,7 +12,7 @@ SETUP_LOG(); DECLARE_INSTANCE(HashedObject); HashedObjectStore::HashedObjectStore(int cacheSize, int cacheAge) : - mCache(cacheSize, cacheAge), mWritePending(false) + mCache("HashedObjectStore", cacheSize, cacheAge), mWritePending(false) { mWriteSet.reserve(128); } diff --git a/src/LedgerHistory.cpp b/src/LedgerHistory.cpp index 854fc011f..dcecab9bf 100644 --- a/src/LedgerHistory.cpp +++ b/src/LedgerHistory.cpp @@ -19,7 +19,7 @@ // FIXME: Need to clean up ledgers by index, probably should switch to just mapping sequence to hash -LedgerHistory::LedgerHistory() : mLedgersByHash(CACHED_LEDGER_NUM, CACHED_LEDGER_AGE) +LedgerHistory::LedgerHistory() : mLedgersByHash("LedgerCache", CACHED_LEDGER_NUM, CACHED_LEDGER_AGE) { ; } void LedgerHistory::addLedger(Ledger::pointer ledger) diff --git a/src/TaggedCache.h b/src/TaggedCache.h index fe7e03d24..abfd27dca 100644 --- a/src/TaggedCache.h +++ b/src/TaggedCache.h @@ -1,12 +1,17 @@ #ifndef __TAGGEDCACHE__ #define __TAGGEDCACHE__ +#include + #include #include #include #include #include +#include "Log.h" +extern LogPartition TaggedCachePartition; + // This class implemented a cache and a map. The cache keeps objects alive // in the map. The map allows multiple code paths that reference objects // with the same tag to get the same actual object. @@ -30,6 +35,7 @@ public: protected: mutable boost::recursive_mutex mLock; + std::string mName; int mTargetSize, mTargetAge; boost::unordered_map mCache; // Hold strong reference to recent objects @@ -38,7 +44,8 @@ protected: boost::unordered_map mMap; // Track stored objects public: - TaggedCache(int size, int age) : mTargetSize(size), mTargetAge(age), mLastSweep(time(NULL)) { ; } + TaggedCache(const char *name, int size, int age) + : mName(name), mTargetSize(size), mTargetAge(age), mLastSweep(time(NULL)) { ; } int getTargetSize() const; int getTargetAge() const; @@ -92,32 +99,40 @@ template void TaggedCache::sweep if (mCache.size() < mTargetSize) return; - time_t now = time(NULL); - if ((mLastSweep + 10) < now) - return; - - mLastSweep = now; - time_t target = now - mTargetAge; + mLastSweep = time(NULL); + time_t target = mLastSweep - mTargetAge; // Pass 1, remove old objects from cache + int cacheRemovals = 0; typename boost::unordered_map::iterator cit = mCache.begin(); while (cit != mCache.end()) { if (cit->second.first < target) + { + ++cacheRemovals; mCache.erase(cit++); + } else ++cit; } // Pass 2, remove dead objects from map + int mapRemovals = 0; typename boost::unordered_map::iterator mit = mMap.begin(); while (mit != mMap.end()) { if (mit->second.expired()) + { + ++mapRemovals; mMap.erase(mit++); + } else ++mit; } + + if (TaggedCachePartition.doLog(lsTRACE) && (mapRemovals || cacheRemovals)) + Log(lsTRACE, TaggedCachePartition) << mName << ": cache = " << mCache.size() << "-" << cacheRemovals << + ", map = " << mMap.size() << "-" << mapRemovals; } template bool TaggedCache::touch(const key_type& key) diff --git a/src/TransactionMaster.cpp b/src/TransactionMaster.cpp index eabf17622..e260ccde2 100644 --- a/src/TransactionMaster.cpp +++ b/src/TransactionMaster.cpp @@ -13,7 +13,7 @@ #define CACHED_TRANSACTION_AGE 1800 #endif -TransactionMaster::TransactionMaster() : mCache(CACHED_TRANSACTION_NUM, CACHED_TRANSACTION_AGE) +TransactionMaster::TransactionMaster() : mCache("TransactionCache", CACHED_TRANSACTION_NUM, CACHED_TRANSACTION_AGE) { ; }