Fix script profiling

It was broken on multiple places (some also apply to the SVN version).

source/ps/GameSetup/GameSetup.cpp

@@ -886,6 +886,20 @@ bool Init(const CmdLineArgs& args, int flags)
 	// This must come after VFS init, which sets the current directory
 	// (required for finding our output log files).
 	g_Logger = new CLogger;
+
+	new CProfileViewer;
+	new CProfileManager;	// before any script code
+
+	g_ScriptStatsTable = new CScriptStatsTable;
+	g_ProfileViewer.AddRootTable(g_ScriptStatsTable);
+
+	// Set up the console early, so that debugging
+	// messages can be logged to it. (The console's size
+	// and fonts are set later in InitPs())
+	g_Console = new CConsole();
+
+	// g_ConfigDB, command line args, globals
+	CONFIG_Init(args);
 
 	// Using a global object for the runtime is a workaround until Simulation and AI use 
 	// their own threads and also their own runtimes.
@@ -912,26 +926,12 @@ bool Init(const CmdLineArgs& args, int flags)
 	hooks.translate_free = psTranslateFree;
 	app_hooks_update(&hooks);
 
-	// Set up the console early, so that debugging
-	// messages can be logged to it. (The console's size
-	// and fonts are set later in InitPs())
-	g_Console = new CConsole();
-
 	CNetHost::Initialize();
 
-	new CProfileViewer;
-	new CProfileManager;	// before any script code
-
-	g_ScriptStatsTable = new CScriptStatsTable;
-	g_ProfileViewer.AddRootTable(g_ScriptStatsTable);
-
 #if CONFIG2_AUDIO
 	ISoundManager::CreateSoundManager();
 #endif
 
-	// g_ConfigDB, command line args, globals
-	CONFIG_Init(args);
-
 	// Check if there are mods specified on the command line,
 	// or if we already set the mods (~INIT_MODS),
 	// else check if there are mods that should be loaded specified

source/ps/Profile.h

@@ -29,6 +29,11 @@
 #include "ps/Singleton.h"
 #include "ps/ThreadUtil.h"
 
+#include <boost/flyweight.hpp>
+#include <boost/flyweight/key_value.hpp>
+#include <boost/flyweight/no_locking.hpp>
+#include <boost/flyweight/no_tracking.hpp>
+
 #define PROFILE_AMORTIZE_FRAMES 30
 #define PROFILE_AMORTIZE_TURNS 1
 
@@ -38,6 +43,25 @@ class CProfileNodeTable;
 class CStr8;
 class CStrW;
 
+// To profile scripts usefully, we use a call hook that's called on every enter/exit,
+// and need to find the function name. But most functions are anonymous so we make do
+// with filename plus line number instead.
+// Computing the names is fairly expensive, and we need to return an interned char*
+// for the profiler to hold a copy of, so we use boost::flyweight to construct interned
+// strings per call location.
+//
+// TODO: Check again how much the overhead for getting filename and line really is and if
+// it has increased with the new approach after the SpiderMonkey 31 upgrade.
+//
+// Flyweight types (with no_locking because the call hooks are only used in the
+// main thread, and no_tracking because we mustn't delete values the profiler is
+// using and it's not going to waste much memory)
+typedef boost::flyweight<
+	std::string,
+	boost::flyweights::no_tracking,
+	boost::flyweights::no_locking
+> StringFlyweight;
+
 class CProfileNode
 {
 	NONCOPYABLE(CProfileNode);

source/scriptinterface/NativeWrapperDefns.h

@@ -84,16 +84,17 @@ struct ScriptInterface_NativeMethodWrapper<void, TC> {
 // Fast natives don't trigger the hook we use for profiling, so explicitly
 // notify the profiler when these functions are being called.
 // ScriptInterface_impl::Register stores the name in a reserved slot.
-// (TODO: this doesn't work for functions registered via InterfaceScripted.h.
-// Maybe we should do some interned JS_GetFunctionId thing.)
 #define SCRIPT_PROFILE \
 	if (g_ScriptProfilingEnabled) \
 	{ \
-		const char* name = "(unknown)"; \
-		JS::RootedValue nameval(cx, JS_GetReservedSlot( &args.callee(), 0)); \
-		if (!nameval.isUndefined()) \
-			name = static_cast<const char*>(JSVAL_TO_PRIVATE(nameval)); \
-		CProfileSampleScript profile(name); \
+		std::string name = "(unknown)"; \
+		JS::RootedString str(cx, JS_GetFunctionId(JS_ValueToFunction(cx, args.calleev()))); \
+		if (str) \
+		{ \
+			JS::RootedValue strVal(cx, JS::StringValue(str)); \
+			ScriptInterface::FromJSVal(cx, strVal, name); \
+		} \
+		CProfileSampleScript profile(StringFlyweight(std::string(name)).get().c_str()); \
 	}
 
 // JSFastNative-compatible function that wraps the function identified in the template argument list

source/scriptinterface/ScriptInterface.cpp

@@ -390,27 +390,6 @@ void ScriptInterface_impl::Register(const char* name, JSNative fptr, uint nargs)
 	JSAutoRequest rq(m_cx);
 	JS::RootedObject nativeScope(m_cx, m_nativeScope);
 	JS::RootedFunction func(m_cx, JS_DefineFunction(m_cx, nativeScope, name, fptr, nargs, JSPROP_ENUMERATE | JSPROP_READONLY | JSPROP_PERMANENT));
-
-	if (!func)
-		return;
-
-	if (g_ScriptProfilingEnabled)
-	{
-		// Store the function name in a slot, so we can pass it to the profiler.
-
-		// Use a flyweight std::string because we can't assume the caller has
-		// a correctly-aligned string and that its lifetime is long enough
-		typedef boost::flyweight<
-			std::string,
-			boost::flyweights::no_tracking
-			// can't use no_locking; Register might be called in threads
-		> LockedStringFlyweight;
-
-		LockedStringFlyweight fw(name);
-		JS::RootedObject funcObj(m_cx, JS_GetFunctionObject(func));
-		JS::RootedValue privateVal(m_cx, JS::PrivateValue((void*)fw.get().c_str()));
-		JS_SetReservedSlot(funcObj, 0, privateVal);
-	}
 }
 
 ScriptInterface::ScriptInterface(const char* nativeScopeName, const char* debugName, const shared_ptr<ScriptRuntime>& runtime) :

source/scriptinterface/ScriptRuntime.cpp

@@ -124,6 +124,9 @@ ScriptRuntime::ScriptRuntime(shared_ptr<ScriptRuntime> parentRuntime, int runtim
 
 	if (g_ScriptProfilingEnabled)
 	{
+		// Execute and call hooks are disabled if the runtime debug mode is disabled
+		JS_SetRuntimeDebugMode(m_rt, true);
+
 		// Profiler isn't thread-safe, so only enable this on the main thread
 		if (ThreadUtil::IsMainThread())
 		{
@@ -316,14 +319,14 @@ void* ScriptRuntime::jshook_function(JSContext* cx, JSAbstractFramePtr fp, bool
 		}
 	}
 
-	// No name - compute from the location instead
+	// No name - use fileName and line instead
 	JS::AutoFilename fileName;
 	unsigned lineno;
 	JS::DescribeScriptedCaller(cx, &fileName, &lineno);
 
 	std::stringstream ss;
 	ss << "(" << fileName.get() << ":" << lineno << ")";
-	g_Profiler.StartScript(ss.str().c_str());
+	g_Profiler.StartScript(StringFlyweight(ss.str()).get().c_str());
 
 	return closure;
 }

source/scriptinterface/ScriptRuntime.h

@@ -19,10 +19,6 @@
 #define INCLUDED_SCRIPTRUNTIME
 
 #include <sstream>
-#include <boost/flyweight.hpp>
-#include <boost/flyweight/key_value.hpp>
-#include <boost/flyweight/no_locking.hpp>
-#include <boost/flyweight/no_tracking.hpp>
 
 #include "ScriptTypes.h"
 #include "ScriptExtraHeaders.h"
@@ -100,73 +96,6 @@ class ScriptRuntime
 		bool UNUSED(isConstructing), bool before,
 		bool* UNUSED(ok), void* closure);
 
-	// To profile scripts usefully, we use a call hook that's called on every enter/exit,
-	// and need to find the function name. But most functions are anonymous so we make do
-	// with filename plus line number instead.
-	// Computing the names is fairly expensive, and we need to return an interned char*
-	// for the profiler to hold a copy of, so we use boost::flyweight to construct interned
-	// strings per call location.
-
-	// Identifies a location in a script
-	struct ScriptLocation
-	{
-		JSContext* cx;
-		JSScript* script;
-		jsbytecode* pc;
-
-		bool operator==(const ScriptLocation& b) const
-		{
-			return cx == b.cx && script == b.script && pc == b.pc;
-		}
-
-		friend std::size_t hash_value(const ScriptLocation& loc)
-		{
-			std::size_t seed = 0;
-			boost::hash_combine(seed, loc.cx);
-			boost::hash_combine(seed, loc.script);
-			boost::hash_combine(seed, loc.pc);
-			return seed;
-		}
-	};
-
-	// Computes and stores the name of a location in a script
-	struct ScriptLocationName
-	{
-		ScriptLocationName(const ScriptLocation& loc)
-		{
-			JSContext* cx = loc.cx;
-			JSScript* script = loc.script;
-			jsbytecode* pc = loc.pc;
-
-			std::string filename = JS_GetScriptFilename(script);
-			size_t slash = filename.rfind('/');
-			if (slash != filename.npos)
-				filename = filename.substr(slash+1);
-
-			uint line = JS_PCToLineNumber(cx, script, pc);
-
-			std::stringstream ss;
-			ss << "(" << filename << ":" << line << ")";
-			name = ss.str();
-		}
-
-		std::string name;
-	};
-
-	// Flyweight types (with no_locking because the call hooks are only used in the
-	// main thread, and no_tracking because we mustn't delete values the profiler is
-	// using and it's not going to waste much memory)
-	typedef boost::flyweight<
-		std::string,
-		boost::flyweights::no_tracking,
-		boost::flyweights::no_locking
-	> StringFlyweight;
-	typedef boost::flyweight<
-		boost::flyweights::key_value<ScriptLocation, ScriptLocationName>,
-		boost::flyweights::no_tracking,
-		boost::flyweights::no_locking
-	> LocFlyweight;
-
 };
 
 #endif // INCLUDED_SCRIPTRUNTIME