# housekeeping and bugfix in Loader queue

remove old instrumentation, fix warnings, add measured timing results
Loader: bugfix: no more memory leak whenever callback returns a warning
code. this was a loophole in the callback return value docs.

This was SVN commit r4048.

source/graphics/ObjectBase.cpp

@@ -214,11 +214,8 @@ bool CObjectBase::Load(const char* filename)
 	return true;
 }
 
-TIMER_ADD_CLIENT(tc_CalculateVariationKey)
-
 std::vector<u8> CObjectBase::CalculateVariationKey(const std::vector<std::set<CStrW> >& selections)
 {
-	TIMER_ACCRUE(tc_CalculateVariationKey);
 	// (TODO: see CObjectManager::FindObjectVariation for an opportunity to
 	// call this function a bit less frequently)
 

source/lib/app_hooks.cpp

@@ -115,7 +115,7 @@ static const wchar_t* translate(const wchar_t* text)
 }
 
 
-static void translate_free(const wchar_t* text)
+static void translate_free(const wchar_t* UNUSED(text))
 {
 	// no-op - translate() doesn't own the pointer.
 }

source/lib/res/graphics/unifont.cpp

@@ -10,17 +10,17 @@
 
 #include "precompiled.h"
 
-#include "lib/lib.h"
-#include "lib/res/res.h"
-#include "lib/ogl.h"
-#include "ogl_tex.h"
-
 #include <string>
 #include <sstream>
 #include <map>
 
 #include <stdio.h>
 
+#include "lib/lib.h"
+#include "lib/res/res.h"
+#include "lib/ogl.h"
+#include "ogl_tex.h"
+
 // This isn't particularly efficient - it can be improved if we
 // (a) care enough, and (b) know about fixed ranges of characters
 // that the fonts usually contain
@@ -60,6 +60,7 @@ static void UniFont_dtor(UniFont* f)
 	SAFE_DELETE(f->glyphs_size);
 }
 
+// [10..70ms]
 static LibError UniFont_reload(UniFont* f, const char* fn, Handle UNUSED(h))
 {
 	// already loaded
@@ -77,7 +78,7 @@ static LibError UniFont_reload(UniFont* f, const char* fn, Handle UNUSED(h))
 	const std::string FilenameFnt = FilenameBase+".fnt";
 	const char* fnt_fn = FilenameFnt.c_str();
 	FileIOBuf buf; size_t size;
-	RETURN_ERR(vfs_load(fnt_fn, buf, size));
+	RETURN_ERR(vfs_load(fnt_fn, buf, size));	// [cumulative for 12: 36ms]
 	std::istringstream FNTStream (std::string((const char*)buf, (int)size));
 	(void)file_buf_free(buf);
 
@@ -103,6 +104,7 @@ static LibError UniFont_reload(UniFont* f, const char* fn, Handle UNUSED(h))
 	if (f->ListBase == 0) // My Voodoo2 drivers didn't support display lists (although I'd be surprised if they got this far)
 		WARN_RETURN(ERR_FAIL);
 
+	// [cumulative for 12: 256ms]
 	for (int i = 0; i < NumGlyphs; ++i)
 	{
 		int          Codepoint, TextureX, TextureY, Width, Height, OffsetX, OffsetY, Advance;
@@ -126,7 +128,7 @@ static LibError UniFont_reload(UniFont* f, const char* fn, Handle UNUSED(h))
 
 		// It might be better to use vertex arrays instead of display lists,
 		// but this works well enough for now.
-
+		// [cumulative for 12: 180ms]
 		glNewList(f->ListBase+i, GL_COMPILE);
 			glBegin(GL_QUADS);
 				glTexCoord2f(u,   -v);   glVertex2i(OffsetX,       -OffsetY);
@@ -137,6 +139,7 @@ static LibError UniFont_reload(UniFont* f, const char* fn, Handle UNUSED(h))
 			glTranslatef((GLfloat)Advance, 0, 0);
 		glEndList();
 
+		// [cumulative for 12: 20ms]
 		(*f->glyphs_id)[(wchar_t)Codepoint] = (unsigned short)i;
 		(*f->glyphs_size)[(wchar_t)Codepoint] = Advance;
 	}
@@ -144,13 +147,12 @@ static LibError UniFont_reload(UniFont* f, const char* fn, Handle UNUSED(h))
 	debug_assert(f->Height); // Ensure the height has been found (which should always happen if the font includes an 'I')
 
 	// Load glyph texture
+	// [cumulative for 12: 20ms]
 	std::string FilenameTex = FilenameBase+".tga";  
 	const char* tex_fn = FilenameTex.c_str();   
 	Handle ht = ogl_tex_load(tex_fn);
 	RETURN_ERR(ht);
-
 	(void)ogl_tex_set_filter(ht, GL_NEAREST);
-
 	// override is necessary because the GL format is chosen as LUMINANCE,
 	// but we want ALPHA. there is no way of knowing what format
 	// 8bpp textures are in - we could adopt a naming convention and
@@ -163,6 +165,7 @@ static LibError UniFont_reload(UniFont* f, const char* fn, Handle UNUSED(h))
 	}
 
 	f->ht = ht;
+
 	return INFO_OK;
 }
 

source/ps/GameSetup/GameSetup.cpp

@@ -489,9 +489,11 @@ void Render()
 static void InitScripting()
 {
 	TIMER("InitScripting");
-	// Create the scripting host.  This needs to be done before the GUI is created.
-	new ScriptingHost;
 
+	// Create the scripting host.  This needs to be done before the GUI is created.
+	// [7ms]
+	new ScriptingHost;
+	
 	// It would be nice for onLoad code to be able to access the setTimeout() calls.
 	new CScheduler;
 
@@ -515,7 +517,7 @@ static void InitScripting()
 	CScriptEvent::ScriptingInit();
 	CJSProgressTimer::ScriptingInit();
 	CProjectile::ScriptingInit();
-	
+
 	g_ScriptingHost.DefineConstant( "FORMATION_ENTER", CFormationEvent::FORMATION_ENTER );
 	g_ScriptingHost.DefineConstant( "FORMATION_LEAVE", CFormationEvent::FORMATION_LEAVE );
 	g_ScriptingHost.DefineConstant( "FORMATION_DAMAGE", CFormationEvent::FORMATION_DAMAGE );
@@ -543,7 +545,6 @@ static void InitScripting()
 	g_ScriptingHost.DefineConstant( "ORDER_PRODUCE", CEntityOrder::ORDER_PRODUCE );
 	g_ScriptingHost.DefineConstant( "ORDER_START_CONSTRUCTION", CEntityOrder::ORDER_START_CONSTRUCTION );
 
-
 #define REG_JS_CONSTANT(_name) g_ScriptingHost.DefineConstant(#_name, _name)
 	REG_JS_CONSTANT(SDL_BUTTON_LEFT);
 	REG_JS_CONSTANT(SDL_BUTTON_MIDDLE);
@@ -581,6 +582,7 @@ static void InitVfs(const char* argv0)
 	(void)file_set_root_dir(argv0, "../data");
 
 	vfs_init();
+
 	vfs_mount("screenshots/", "screenshots");
 	vfs_mount("profiles/", "profiles", VFS_MOUNT_RECURSIVE);
 
@@ -590,9 +592,12 @@ static void InitVfs(const char* argv0)
 	// - we mount as archivable so that all files will be added to archive.
 	//   even though we write out XMBs here, they will eventually be read,
 	//   so putting them in an archive boosts performance.
+	//
+	// [hot: 16ms]
 	vfs_mount("cache/", "cache", VFS_MOUNT_RECURSIVE|VFS_MOUNT_ARCHIVES|VFS_MOUNT_ARCHIVABLE);
 
 	// --- this belongs in a LoadMod function
+	// [hot: 150ms]
 	vfs_mount("", "mods/official", VFS_MOUNT_RECURSIVE|VFS_MOUNT_ARCHIVES|VFS_MOUNT_WATCH|VFS_MOUNT_ARCHIVABLE);
 	// ---
 
@@ -869,16 +874,10 @@ void Shutdown()
 }
 
 
-// workaround for VC7 EBP-trashing bug, which confuses the stack trace code.
-#if MSC_VERSION
-# pragma optimize("", off)
-#endif
-
 void Init(int argc, char* argv[], uint flags)
 {
 	const bool setup_vmode = (flags & INIT_HAVE_VMODE) == 0;
 
-	debug_printf("INIT &argc=%p &argv=%p\n", &argc, &argv);
 	MICROLOG(L"Init");
 
 	debug_set_thread_name("main");
@@ -905,7 +904,7 @@ void Init(int argc, char* argv[], uint flags)
 	// (required for finding our output log files).
 	new CLogger;
 
-	// Call LoadLanguage(NULL) to initialise the I18n system, but
+	// Call LoadLanguage(NULL) to initialize the I18n system, but
 	// without loading an actual language file - translate() will
 	// just show the English key text, which is better than crashing
 	// from a null pointer when attempting to translate e.g. error messages.
@@ -1106,9 +1105,3 @@ void Init(int argc, char* argv[], uint flags)
 	}
 
 }
-
-#if MSC_VERSION
-# pragma optimize("", on)	// restore; see above.
-#endif
-
-

source/ps/Loader.cpp

@@ -233,12 +233,13 @@ LibError LDR_ProgressiveLoad(double time_budget, wchar_t* description,
 		// call this task's function and bill elapsed time.
 		const double t0 = get_time();
 		int status = lr.func(lr.param, time_left);
+		const bool timed_out = ldr_was_interrupted(status);
 		const double elapsed_time = get_time() - t0;
 		time_left -= elapsed_time;
 		task_elapsed_time += elapsed_time;
 
 		// either finished entirely, or failed => remove from queue.
-		if(status <= 0)
+		if(!timed_out)
 		{
 			debug_printf("LOADER| completed %ls in %g ms; estimate was %g ms\n", lr.description.c_str(), task_elapsed_time*1e3, estimated_duration*1e3);
 			task_elapsed_time = 0.0;
@@ -254,18 +255,15 @@ LibError LDR_ProgressiveLoad(double time_budget, wchar_t* description,
 			// function interrupted itself; add its estimated progress.
 			// note: monotonicity is guaranteed since we never add more than
 			//   its estimated_duration_ms.
-			if(status > 0)
-			{
-				status = MIN(status, 100);	// clamp in case estimate is too high
+			if(timed_out)
 				current_estimate += estimated_duration * status/100.0;
-			}
 
 			progress = current_estimate / total_estimated_duration;
 		}
 
 		// do we need to continue?
 		// .. function interrupted itself, i.e. timed out; abort.
-		if(status > 0)
+		if(timed_out)
 		{
 			ret = ERR_TIMED_OUT;
 			goto done;

source/ps/Loader.h

@@ -98,9 +98,10 @@ extern LibError LDR_BeginRegistering();
 //   it will then be de-queued.
 // - if the work can be split into smaller subtasks, process those until
 //   <time_left> is reached or exceeded and then return an estimate
-//   of progress in percent (!= 0, or it's treated as "finished").
-// - on failure, return a negative error code; LDR_ProgressiveLoad
-//   will abort immediately and return that.
+//   of progress in percent (<= 100, otherwise it's a warning;
+//   != 0, or it's treated as "finished")
+// - on failure, return a negative error code or 'warning' (see above);
+//   LDR_ProgressiveLoad will abort immediately and return that.
 typedef int (*LoadFunc)(void* param, double time_left);
 
 // register a task (later processed in FIFO order).

source/ps/LoaderThunks.h

@@ -17,6 +17,13 @@
 #pragma warning(disable: 4121)
 #endif
 
+// does this return code indicate the coroutine yielded and
+// wasn't yet finished?
+static bool ldr_was_interrupted(int ret)
+{
+	return (0 < ret && ret <= 100);
+}
+
 template<class T> struct MemFun_t
 {
 	T* const this_;
@@ -33,7 +40,8 @@ template<class T> static int MemFunThunk(void* param, double UNUSED(time_left))
 {
 	MemFun_t<T>* const mf = (MemFun_t<T>*)param;
 	int ret = (mf->this_->*mf->func)();
-	if(ret <= 0)	// did not time out
+
+	if(!ldr_was_interrupted(ret))
 		delete mf;
 	return ret;
 }
@@ -66,7 +74,7 @@ template<class T, class Arg> static int MemFun1Thunk(void* param, double UNUSED(
 {
 	MemFun1_t<T, Arg>* const mf = (MemFun1_t<T, Arg>*)param;
 	int ret = (mf->this_->*mf->func)(mf->arg);
-	if(ret <= 0)	// did not time out
+	if(!ldr_was_interrupted(ret))
 		delete mf;
 	return ret;
 }

source/simulation/EntityManager.cpp

@@ -221,6 +221,9 @@ void CEntityManager::dispatchAll( CMessage* msg )
 }
 */
 
+TIMER_ADD_CLIENT(tc_1);
+TIMER_ADD_CLIENT(tc_2);
+
 void CEntityManager::InitializeAll()
 {
 	CTerrain* terrain = g_Game->GetWorld()->GetTerrain();
@@ -233,10 +236,11 @@ void CEntityManager::InitializeAll()
 	{
 		if( m_entities[i].m_refcount && !m_entities[i].m_entity->m_destroyed )
 		{
+			// [2006-06-26 2780ms total]
 			CEntity* e = m_entities[i].m_entity;
 			e->Initialize();
 
-			// 2006-03-04 ~8ms total
+			// [2006-06-26 8ms total]
 			e->updateCollisionPatch();
 		}
 	}

source/simulation/Simulation.cpp

@@ -51,11 +51,13 @@ int CSimulation::Initialize(CGameAttributes* pAttribs)
 
 	// Call the game startup script 
 	// TODO: Maybe don't do this if we're in Atlas
+	// [2006-06-26 20ms]
 	g_ScriptingHost.RunScript( "scripts/game_startup.js" );
 
+	// [2006-06-26 3647ms]
 	g_EntityManager.InitializeAll();
 
-	// 2006-03-04: ~33ms
+	// [2006-06-26: 61ms]
 	m_pWorld->GetLOSManager()->Initialize(pAttribs->m_LOSSetting);
 
 	return 0;