# HG changeset patch # User phoku@33b003aa-7bff-0310-803a-e67f0ece8222 # Date 1223559373 0 # Node ID fb6ccb367dd19baaf8765bdfda7527ce22988e70 # Parent 54b3984e1afcce3f62b98099a00e0f033ec8b9d8 Added just some docs and a sanityCheck against the difficult to find bugs of the location classes. So if memory usage/load time goes through the roof, try logging 'pool' messages. diff -r 54b3984e1afc -r fb6ccb367dd1 engine/core/util/resource/pool.cpp --- a/engine/core/util/resource/pool.cpp Thu Oct 09 12:36:21 2008 +0000 +++ b/engine/core/util/resource/pool.cpp Thu Oct 09 13:36:13 2008 +0000 @@ -48,6 +48,7 @@ Pool::~Pool() { FL_LOG(_log, LMsg("Pool destroyed: ") << m_name); printStatistics(); + sanityCheck(); clear(); std::vector::iterator loader; for (loader = m_loaders.begin(); loader != m_loaders.end(); loader++) { @@ -225,4 +226,24 @@ FL_LOG(_log, LMsg("Pool locked =") << amount); FL_LOG(_log, LMsg("Pool total size =") << m_entries.size()); } + + void Pool::sanityCheck() { + // It is easy to mess up the important consistent + // less-than operator for the location classes. + // This will check if according to the '==' operator + // entries are duplicate (=memory leaks). + // Slow and inaccurate. But should barf at real messups. + for(unsigned i = 0; i != m_entries.size(); ++i) { + int count = 0; + for(unsigned j = i+1; j < m_entries.size(); ++j) { + if( *m_entries[i]->location == *m_entries[j]->location ) + count ++; + } + if( 0 == count ) + continue; + FL_WARN(_log, LMsg("Multiple entries: ") << m_entries[i]->location->getFilename() + << " #entries = " << (count+1) ); + } + } + } diff -r 54b3984e1afc -r fb6ccb367dd1 engine/core/util/resource/pool.h --- a/engine/core/util/resource/pool.h Thu Oct 09 12:36:21 2008 +0000 +++ b/engine/core/util/resource/pool.h Thu Oct 09 13:36:13 2008 +0000 @@ -127,10 +127,14 @@ */ virtual void removePoolListener(IPoolListener* listener); - /** Prints the cache statistics to the standard output + /** Prints the cache statistics to the log */ virtual void printStatistics(); + /** Performs a sanity check for the location map. + */ + virtual void sanityCheck(); + protected: private: class PoolEntry { diff -r 54b3984e1afc -r fb6ccb367dd1 engine/core/util/resource/resource_location.h --- a/engine/core/util/resource/resource_location.h Thu Oct 09 12:36:21 2008 +0000 +++ b/engine/core/util/resource/resource_location.h Thu Oct 09 13:36:13 2008 +0000 @@ -34,10 +34,17 @@ namespace FIFE { +#define RES_TYPE_FILE 0 +#define RES_TYPE_IMAGE 1 + /** Contains information about the Location of a Resource * * This class is used to give ResoureProvider the information - * where to find the data. + * where to find the data. + * + * WARNING: It is \b very important that the comparison operators work correctly, + * otherwise the pools will silently consume more and more memory. So before you change + * something there, think about the implications. Please. */ class ResourceLocation { public: @@ -45,7 +52,7 @@ // LIFECYCLE /** Default constructor. */ - ResourceLocation(const std::string& filename): m_filename(filename) {} + ResourceLocation(const std::string& filename): m_filename(filename),m_type(RES_TYPE_FILE) {} /** Destructor. */ @@ -59,6 +66,10 @@ /** Compares two ResourceLocations for equality. */ virtual bool operator ==(const ResourceLocation& loc) const { + if( m_type != loc.m_type ) { + return false; + } + if (m_filename.length() != loc.m_filename.length()) { return false; } @@ -72,6 +83,10 @@ * This is needed as the locations should be stored in a \c std::map */ virtual bool operator <(const ResourceLocation& loc) const { + if( m_type < loc.m_type ) + return true; + if( m_type > loc.m_type ) + return false; return m_filename < loc.m_filename; } @@ -82,8 +97,11 @@ return new ResourceLocation(m_filename); } - private: + int getType() const { return m_type; } + + protected: std::string m_filename; + int m_type; }; } //FIFE diff -r 54b3984e1afc -r fb6ccb367dd1 engine/core/video/image_location.cpp --- a/engine/core/video/image_location.cpp Thu Oct 09 12:36:21 2008 +0000 +++ b/engine/core/video/image_location.cpp Thu Oct 09 13:36:13 2008 +0000 @@ -37,6 +37,7 @@ m_width(0), m_height(0), m_parent_image(NULL) { + m_type = RES_TYPE_IMAGE; } bool ImageLocation::operator ==(const ResourceLocation& loc) const { @@ -67,9 +68,14 @@ } bool ImageLocation::operator <(const ResourceLocation& loc) const { - if (!ResourceLocation::operator <(loc)) { + if( m_type < loc.getType() ) + return true; + if( m_type > loc.getType() ) return false; - } + if( m_filename < loc.getFilename() ) + return true; + if( m_filename > loc.getFilename() ) + return false; const ImageLocation* r = dynamic_cast(&loc); if (!r) {