# code cleanup/fixes

improvements from work:
- no longer export classes; instead inline functions where it makes
sense (e.g. the scope timers) or export the member functions directly
- fix icc11 warnings
- add some comments

This was SVN commit r6531.

source/lib/allocators/bucket.h

@@ -27,7 +27,7 @@
  *
  * opaque! do not read/write any fields!
  **/
-struct LIB_API Bucket
+struct Bucket
 {
 	/**
 	 * currently open bucket.

source/lib/allocators/shared_ptr.h

@@ -1,6 +1,8 @@
 #ifndef INCLUDED_SHARED_PTR
 #define INCLUDED_SHARED_PTR
 
+#include "lib/sysdep/arch/x86_x64/x86_x64.h"
+
 struct DummyDeleter
 {
 	template<class T>
@@ -27,4 +29,19 @@ struct ArrayDeleter
 // (note: uses CheckedArrayDeleter)
 LIB_API shared_ptr<u8> Allocate(size_t size);
 
+struct AlignedDeleter
+{
+	template<class T>
+	void operator()(T* t)
+	{
+		_mm_free(t);
+	}
+};
+
+template<class T>
+shared_ptr<T> AllocateAligned(size_t size)
+{
+	return shared_ptr<T>((T*)_mm_malloc(size, x86_x64_L1CacheLineSize()), AlignedDeleter());
+}
+
 #endif	// #ifndef INCLUDED_SHARED_PTR

source/lib/code_annotation.h

@@ -157,7 +157,7 @@ namespace detail
 
 namespace noncopyable_  // protection from unintended ADL
 {
-	class LIB_API noncopyable
+	class noncopyable
 	{
 	protected:
 		noncopyable() {}

source/lib/debug.h

@@ -450,6 +450,9 @@ LIB_API void debug_puts(const char* text);
 /**
  * return the caller of a certain function on the call stack.
  *
+ * this function is useful for recording (partial) stack traces for
+ * memory allocation tracking, etc.
+ *
  * @param context, lastFuncToSkip - see debug_DumpStack
  * @return address of the caller
  **/

source/lib/file/path.h

@@ -23,13 +23,13 @@
 struct PathTraits;
 typedef fs::basic_path<std::string, PathTraits> Path;
 
-struct LIB_API PathTraits
+struct PathTraits
 {
 	typedef std::string internal_string_type;
 	typedef std::string external_string_type;
 
-	static external_string_type to_external(const Path&, const internal_string_type& src);
+	static LIB_API external_string_type to_external(const Path&, const internal_string_type& src);
-	static internal_string_type to_internal(const external_string_type& src);
+	static LIB_API internal_string_type to_internal(const external_string_type& src);
 };
 
 namespace boost

source/lib/file/vfs/vfs_lookup.cpp

@@ -17,13 +17,10 @@
 #include "vfs_populate.h"
 
 #include "lib/timer.h"
-TIMER_ADD_CLIENT(tc_lookup);
 
 
 LibError vfs_Lookup(const VfsPath& pathname, VfsDirectory* startDirectory, VfsDirectory*& directory, VfsFile** pfile, size_t flags)
 {
-TIMER_ACCRUE(tc_lookup);
-
 	// extract and validate flags (ensure no unknown bits are set)
 	const bool addMissingDirectories    = (flags & VFS_LOOKUP_ADD) != 0;
 	const bool createMissingDirectories = (flags & VFS_LOOKUP_CREATE) != 0;

source/lib/res/handle.h

@@ -13,11 +13,17 @@
 
 #include "lib/file/vfs/vfs_path.h"
 
-// 0 = invalid handle value; < 0 is an error code.
+/**
-// 64 bits, because we want tags to remain unique: tag overflow may
+ * `handle' representing a reference to a resource (sound, texture, etc.)
-// let handle use errors slip through, or worse, cause spurious errors.
+ *
-// with 32 bits, we'd need >= 12 for the index, leaving < 512K tags -
+ * 0 is the (silently ignored) invalid handle value; < 0 is an error code.
-// not a lot.
+ *
+ * this is 64 bits because we want tags to remain unique. (tags are a
+ * counter that disambiguate several subsequent uses of the same
+ * resource array slot). 32-bit handles aren't enough because the index
+ * field requires at least 12 bits, thus leaving only about 512K possible
+ * tag values.
+ **/
 typedef i64 Handle;
 
 #endif	// #ifndef INCLUDED_HANDLE

source/lib/sysdep/numa.h

@@ -98,9 +98,9 @@ template<typename T>
 class numa_Allocator
 {
 public:
-	shared_ptr<T> operator()(size_t size) const
+	shared_ptr<T> operator()(size_t size, LargePageDisposition largePageDisposition = LPD_DEFAULT, size_t* ppageSize = 0) const
 	{
-		return shared_ptr<T>((T*)numa_Allocate(size), numa_Deleter<T>());
+		return shared_ptr<T>((T*)numa_Allocate(size, largePageDisposition, ppageSize), numa_Deleter<T>());
 	}
 };
 

source/lib/sysdep/os/win/wdbg.cpp

@@ -24,10 +24,10 @@ int debug_IsPointerBogus(const void* p)
 	if(p < (void*)0x10000)
 		return true;
 #if ARCH_AMD64
-	if(p == (const void*)0xCCCCCCCCCCCCCCCCull)
+	if(p == (const void*)(uintptr_t)0xCCCCCCCCCCCCCCCCull)
 		return true;
 #elif ARCH_IA32
-	if(p == (const void*)0xCCCCCCCCul)
+	if(p == (const void*)(uintptr_t)0xCCCCCCCCul)
 		return true;
 #endif
 

source/lib/sysdep/os/win/wdbg_sym.cpp

@@ -120,7 +120,7 @@ struct TI_FINDCHILDREN_PARAMS2
 
 // actual implementation; made available so that functions already under
 // the lock don't have to unlock (slow) to avoid recursive locking.
-static LibError debug_resolve_symbol_lk(void* ptr_of_interest, char* sym_name, char* file, int* line)
+static LibError ResolveSymbol_lk(void* ptr_of_interest, char* sym_name, char* file, int* line)
 {
 	sym_init();
 
@@ -183,7 +183,7 @@ static LibError debug_resolve_symbol_lk(void* ptr_of_interest, char* sym_name, c
 LibError debug_ResolveSymbol(void* ptr_of_interest, char* sym_name, char* file, int* line)
 {
 	WinScopedLock lock(WDBG_SYM_CS);
-	return debug_resolve_symbol_lk(ptr_of_interest, sym_name, file, line);
+	return ResolveSymbol_lk(ptr_of_interest, sym_name, file, line);
 }
 
 
@@ -281,10 +281,15 @@ static LibError ia32_walk_stack(_tagSTACKFRAME64* sf)
 
 #endif
 
+// note: RtlCaptureStackBackTrace (http://msinilo.pl/blog/?p=40)
+// is likely to be much faster than StackWalk64 (especially relevant
+// for debug_GetCaller), but wasn't known during development and
+// remains undocumented.
 
 typedef VOID (WINAPI *PRtlCaptureContext)(PCONTEXT);
 static PRtlCaptureContext s_RtlCaptureContext;
 
+
 LibError wdbg_sym_WalkStack(StackFrameCallback cb, uintptr_t cbData, const CONTEXT* pcontext, const char* lastFuncToSkip)
 {
 	// to function properly, StackWalk64 requires a CONTEXT on
@@ -1207,7 +1212,7 @@ static LibError dump_sym_function_type(DWORD UNUSED(type_id), const u8* p, DumpS
 	// isn't exposed via TI_GET_SYMNAME, so we resolve it ourselves.
 
 	char name[DBG_SYMBOL_LEN];
-	LibError err = debug_resolve_symbol_lk((void*)p, name, 0, 0);
+	LibError err = ResolveSymbol_lk((void*)p, name, 0, 0);
 
 	if(state.indirection == 0)
 		out(L"0x%p ", p);
@@ -1767,7 +1772,7 @@ static LibError dump_frame_cb(const _tagSTACKFRAME64* sf, uintptr_t UNUSED(cbDat
 	void* func = (void*)(uintptr_t)sf->AddrPC.Offset;
 
 	char func_name[DBG_SYMBOL_LEN]; char file[DBG_FILE_LEN]; int line;
-	LibError ret = debug_resolve_symbol_lk(func, func_name, file, &line);
+	LibError ret = ResolveSymbol_lk(func, func_name, file, &line);
 	if(ret == INFO::OK)
 	{
 		// don't trace back further than the app's entry point

source/lib/sysdep/os/win/wposix/wtime.cpp

@@ -263,7 +263,7 @@ match_string (const char **buf, const char **strs)
 	int i = 0;
 
 	for (i = 0; strs[i] != NULL; ++i) {
-		int len = strlen (strs[i]);
+		size_t len = strlen (strs[i]);
 
 		if (strncasecmp (*buf, strs[i], len) == 0) {
 			*buf += len;

source/lib/timer.cpp

@@ -17,17 +17,12 @@
 #include <stdarg.h>
 
 #include "lib/posix/posix_time.h"
-#include "lib/sysdep/os_cpu.h"
 #if OS_WIN
 #include "lib/sysdep/os/win/whrt/whrt.h"
 #endif
 #if OS_UNIX
 # include <unistd.h>
 #endif
-#include "lib/config2.h"	// CONFIG2_TIMER_ALLOW_RDTSC
-#if ARCH_X86_X64 && CONFIG2_TIMER_ALLOW_RDTSC
-# include "lib/sysdep/arch/x86_x64/x86_x64.h"	// x86_x64_rdtsc
-#endif
 
 #if OS_UNIX || OS_WIN
 # define HAVE_GETTIMEOFDAY 1
@@ -124,147 +119,6 @@ double timer_Resolution()
 }
 
 
-//-----------------------------------------------------------------------------
-
-ScopeTimer::ScopeTimer(const char* description)
-	: m_t0(timer_Time()), m_description(description)
-{
-}
-
-
-ScopeTimer::~ScopeTimer()
-{
-	double t1 = timer_Time();
-	double dt = t1-m_t0;
-
-	// determine scale factor for pretty display
-	double scale = 1e6;
-	const char* unit = "us";
-	if(dt > 1.0)
-		scale = 1, unit = "s";
-	else if(dt > 1e-3)
-		scale = 1e3, unit = "ms";
-
-	debug_printf("TIMER| %s: %g %s\n", m_description, dt*scale, unit);
-}
-
-
-//-----------------------------------------------------------------------------
-// TimerUnit
-
-// since TIMER_ACCRUE et al. are called so often, we try to keep
-// overhead to an absolute minimum. storing raw tick counts (e.g. CPU cycles
-// returned by ia32_rdtsc) instead of absolute time has two benefits:
-// - no need to convert from raw->time on every call
-//   (instead, it's only done once when displaying the totals)
-// - possibly less overhead to querying the time itself
-//   (timer_Time may be using slower time sources with ~3us overhead)
-//
-// however, the cycle count is not necessarily a measure of wall-clock time
-// (see http://www.gamedev.net/reference/programming/features/timing).
-// therefore, on systems with SpeedStep active, measurements of I/O or other
-// non-CPU bound activity may be skewed. this is ok because the timer is
-// only used for profiling; just be aware of the issue.
-// if this is a problem, disable CONFIG2_TIMER_ALLOW_RDTSC.
-// 
-// note that overflow isn't an issue either way (63 bit cycle counts
-// at 10 GHz cover intervals of 29 years).
-
-#if ARCH_X86_X64 && CONFIG2_TIMER_ALLOW_RDTSC
-
-void TimerUnit::SetToZero()
-{
-	m_ticks = 0;
-}
-
-void TimerUnit::SetFromTimer()
-{
-	m_ticks = x86_x64_rdtsc();
-}
-
-void TimerUnit::AddDifference(TimerUnit t0, TimerUnit t1)
-{
-	m_ticks += t1.m_ticks - t0.m_ticks;
-}
-
-void TimerUnit::Subtract(TimerUnit t)
-{
-	m_ticks -= t.m_ticks;
-}
-
-std::string TimerUnit::ToString() const
-{
-	debug_assert(m_ticks >= 0.0);
-
-	// determine scale factor for pretty display
-	double scale = 1.0;
-	const char* unit = " c";
-	if(m_ticks > 10000000000LL)	// 10 Gc
-		scale = 1e-9, unit = " Gc";
-	else if(m_ticks > 10000000)	// 10 Mc
-		scale = 1e-6, unit = " Mc";
-	else if(m_ticks > 10000)	// 10 kc
-		scale = 1e-3, unit = " kc";
-
-	std::stringstream ss;
-	ss << m_ticks*scale;
-	ss << unit;
-	return ss.str();
-}
-
-double TimerUnit::ToSeconds() const
-{
-	return m_ticks / os_cpu_ClockFrequency();
-}
-
-#else
-
-void TimerUnit::SetToZero()
-{
-	m_seconds = 0.0;
-}
-
-void TimerUnit::SetFromTimer()
-{
-	m_seconds = timer_Time();
-}
-
-void TimerUnit::AddDifference(TimerUnit t0, TimerUnit t1)
-{
-	m_seconds += t1.m_seconds - t0.m_seconds;
-}
-
-void TimerUnit::Subtract(TimerUnit t)
-{
-	m_seconds -= t.m_seconds;
-}
-
-std::string TimerUnit::ToString() const
-{
-	debug_assert(m_seconds >= 0.0);
-
-	// determine scale factor for pretty display
-	double scale = 1e6;
-	const char* unit = " us";
-	if(m_seconds > 1.0)
-		scale = 1, unit = " s";
-	else if(m_seconds > 1e-3)
-		scale = 1e3, unit = " ms";
-
-	std::stringstream ss;
-	ss << m_seconds*scale;
-	ss << unit;
-	return ss.str();
-}
-
-double TimerUnit::ToSeconds() const
-{
-	return m_seconds;
-}
-
-#endif
-
-
 //-----------------------------------------------------------------------------
 // client API
 
@@ -319,18 +173,3 @@ void timer_DisplayClientTotals()
 
 	debug_printf("-----------------------------------------------------\n");
 }
-
-
-ScopeTimerAccrue::ScopeTimerAccrue(TimerClient* tc)
-	: m_tc(tc)
-{
-	m_t0.SetFromTimer();
-}
-
-
-ScopeTimerAccrue::~ScopeTimerAccrue()
-{
-	TimerUnit t1;
-	t1.SetFromTimer();
-	timer_BillClient(m_tc, m_t0, t1);
-}

source/lib/timer.h

@@ -11,6 +11,12 @@
 #ifndef INCLUDED_TIMER
 #define INCLUDED_TIMER
 
+#include "lib/config2.h"	// CONFIG2_TIMER_ALLOW_RDTSC
+#if ARCH_X86_X64 && CONFIG2_TIMER_ALLOW_RDTSC
+# include "lib/sysdep/arch/x86_x64/x86_x64.h"	// x86_x64_rdtsc
+# include "lib/sysdep/os_cpu.h"	// os_cpu_ClockFrequency
+#endif
+
 /**
  * timer_Time will subsequently return values relative to the current time.
  **/
@@ -31,11 +37,29 @@ LIB_API double timer_Resolution(void);
 // scope timing
 
 /// used by TIMER
-class LIB_API ScopeTimer : noncopyable
+class ScopeTimer : noncopyable
 {
 public:
-	ScopeTimer(const char* description);
+	ScopeTimer(const char* description)
-	~ScopeTimer();
+		: m_t0(timer_Time()), m_description(description)
+	{
+	}
+
+	~ScopeTimer()
+	{
+		double t1 = timer_Time();
+		double dt = t1-m_t0;
+
+		// determine scale factor for pretty display
+		double scale = 1e6;
+		const char* unit = "us";
+		if(dt > 1.0)
+			scale = 1, unit = "s";
+		else if(dt > 1e-3)
+			scale = 1e3, unit = "ms";
+
+		debug_printf("TIMER| %s: %g %s\n", m_description, dt*scale, unit);
+	}
 
 private:
 	double m_t0;
@@ -91,21 +115,132 @@ private:
 // this supplements in-game profiling by providing low-overhead,
 // high resolution time accounting of specific areas.
 
-union LIB_API TimerUnit
+// since TIMER_ACCRUE et al. are called so often, we try to keep
+// overhead to an absolute minimum. storing raw tick counts (e.g. CPU cycles
+// returned by ia32_rdtsc) instead of absolute time has two benefits:
+// - no need to convert from raw->time on every call
+//   (instead, it's only done once when displaying the totals)
+// - possibly less overhead to querying the time itself
+//   (timer_Time may be using slower time sources with ~3us overhead)
+//
+// however, the cycle count is not necessarily a measure of wall-clock time
+// (see http://www.gamedev.net/reference/programming/features/timing).
+// therefore, on systems with SpeedStep active, measurements of I/O or other
+// non-CPU bound activity may be skewed. this is ok because the timer is
+// only used for profiling; just be aware of the issue.
+// if this is a problem, disable CONFIG2_TIMER_ALLOW_RDTSC.
+// 
+// note that overflow isn't an issue either way (63 bit cycle counts
+// at 10 GHz cover intervals of 29 years).
+
+#if ARCH_X86_X64 && CONFIG2_TIMER_ALLOW_RDTSC
+
+class TimerUnit
 {
 public:
-	void SetToZero();
+	void SetToZero()
-	void SetFromTimer();
+	{
-	void AddDifference(TimerUnit t0, TimerUnit t1);
+		m_ticks = 0;
-	void Subtract(TimerUnit t);
+	}
-	std::string ToString() const;
+
-	double ToSeconds() const;
+	void SetFromTimer()
+	{
+		m_ticks = x86_x64_rdtsc();
+	}
+
+	void AddDifference(TimerUnit t0, TimerUnit t1)
+	{
+		m_ticks += t1.m_ticks - t0.m_ticks;
+	}
+
+	void Subtract(TimerUnit t)
+	{
+		m_ticks -= t.m_ticks;
+	}
+
+	std::string ToString() const
+	{
+		debug_assert(m_ticks >= 0.0);
+
+		// determine scale factor for pretty display
+		double scale = 1.0;
+		const char* unit = " c";
+		if(m_ticks > 10000000000LL)	// 10 Gc
+			scale = 1e-9, unit = " Gc";
+		else if(m_ticks > 10000000)	// 10 Mc
+			scale = 1e-6, unit = " Mc";
+		else if(m_ticks > 10000)	// 10 kc
+			scale = 1e-3, unit = " kc";
+
+		std::stringstream ss;
+		ss << m_ticks*scale;
+		ss << unit;
+		return ss.str();
+
+	}
+
+	double ToSeconds() const
+	{
+		return m_ticks / os_cpu_ClockFrequency();
+	}
 
 private:
 	u64 m_ticks;
+};
+
+#else
+
+class TimerUnit
+{
+public:
+	void SetToZero()
+	{
+		m_seconds = 0.0;
+	}
+
+	void SetFromTimer()
+	{
+		m_seconds = timer_Time();
+	}
+
+	void AddDifference(TimerUnit t0, TimerUnit t1)
+	{
+		m_seconds += t1.m_seconds - t0.m_seconds;
+	}
+
+	void Subtract(TimerUnit t)
+	{
+		m_seconds -= t.m_seconds;
+	}
+
+	std::string ToString() const
+	{
+		debug_assert(m_seconds >= 0.0);
+
+		// determine scale factor for pretty display
+		double scale = 1e6;
+		const char* unit = " us";
+		if(m_seconds > 1.0)
+			scale = 1, unit = " s";
+		else if(m_seconds > 1e-3)
+			scale = 1e3, unit = " ms";
+
+		std::stringstream ss;
+		ss << m_seconds*scale;
+		ss << unit;
+		return ss.str();	}
+
+	double ToSeconds() const
+	{
+		return m_seconds;
+	}
+
+private:
 	double m_seconds;
 };
 
+#endif
+
 // opaque - do not access its fields!
 // note: must be defined here because clients instantiate them;
 // fields cannot be made private due to POD requirement.
@@ -159,11 +294,21 @@ LIB_API void timer_BillClient(TimerClient* tc, TimerUnit t0, TimerUnit t1);
 LIB_API void timer_DisplayClientTotals();
 
 /// used by TIMER_ACCRUE
-class LIB_API ScopeTimerAccrue
+class ScopeTimerAccrue
 {
 public:
-	ScopeTimerAccrue(TimerClient* tc);
+	ScopeTimerAccrue(TimerClient* tc)
-	~ScopeTimerAccrue();
+		: m_tc(tc)
+	{
+		m_t0.SetFromTimer();
+	}
+
+	~ScopeTimerAccrue()
+	{
+		TimerUnit t1;
+		t1.SetFromTimer();
+		timer_BillClient(m_tc, m_t0, t1);
+	}
 
 private:
 	TimerUnit m_t0;