changeset 147:fb6ccb367dd1

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.
author phoku@33b003aa-7bff-0310-803a-e67f0ece8222
date Thu, 09 Oct 2008 13:36:13 +0000
parents 54b3984e1afc
children 72c25cc27d8b
files engine/core/util/resource/pool.cpp engine/core/util/resource/pool.h engine/core/util/resource/resource_location.h engine/core/video/image_location.cpp
diffstat 4 files changed, 55 insertions(+), 6 deletions(-) [+]
line wrap: on
line diff
--- 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<ResourceLoader*>::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) );
+		}
+	}
+
 }
--- 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 {
--- 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
 
--- 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<const ImageLocation*>(&loc);
 		if (!r) {