Merge pull request #5934 from mwerle/refactor/savegame_stats_job

Improve performance when enumerating saves by running SaveGameStats in a job

data/libs/utils.lua

@@ -960,4 +960,20 @@ utils.getFromIntervals = function(array, value)
 	return array[utils.getIndexFromIntervals(array, value)][1]
 end
 
+--
+-- Function: utils.profile
+--
+-- Simple manual scoped profiler to print the execution time of some invocation.
+-- The returned function should be called to terminate the profile scope.
+--
+utils.profile = function(name)
+	local start = Engine.nowTime
+
+	return function()
+		local duration = Engine.nowTime - start
+
+		print(string.format("PROFILE | %s took %.4fms", name, duration * 1000.0))
+	end
+end
+
 return utils

data/pigui/modules/saveloadgame.lua

@@ -2,6 +2,7 @@
 -- Licensed under the terms of the GPL v3. See licenses/GPL-3.txt
 
 local Game       = require 'Game'
+local Event      = require 'Event'
 local FileSystem = require 'FileSystem'
 local Lang       = require 'Lang'
 local ShipDef    = require 'ShipDef'
@@ -197,42 +198,56 @@ end
 
 --=============================================================================
 
-local function makeEntryForSave(file)
-	local compatible, saveInfo = pcall(Game.SaveGameStats, file.name)
-	if not compatible then
-		saveInfo = {}
-	end
-
+-- Event callback once the savegame information has been loaded
+-- Updates the entryCache with the full details of the savegame.
+function SaveLoadWindow:onSaveGameStats(saveInfo)
+	-- local profileEndScope = utils.profile("SaveLoadWindow:onSaveGameStats()")
 	local location = saveInfo.system or lc.UNKNOWN
-
 	if saveInfo.docked_at then
 		location = location .. ", " .. saveInfo.docked_at
 	end
 
-	local saveEntry = SaveGameEntry:clone({
-		name = file.name,
-		compatible = compatible,
-		isAutosave = file.name:sub(1, 1) == "_",
-		character = saveInfo.character,
-		timestamp = file.mtime.timestamp,
-		gameTime = saveInfo.time,
-		duration = saveInfo.duration,
-		locationName = location,
-		credits = saveInfo.credits,
-		shipName = saveInfo.shipName,
-		shipHull = saveInfo.shipHull,
-	})
-
 	-- Old saves store only the name of the ship's *model* file for some dumb reason
 	-- Treat the model name as the ship id and otherwise ignore it if we have proper data
+	local shipHull
 	if not saveInfo.shipHull then
 		local shipDef = ShipDef[saveInfo.ship]
-
 		if shipDef then
-			saveEntry.shipHull = shipDef.name
+			shipHull = shipDef.name
 		end
+	else
+		shipHull = saveInfo.shipHull
 	end
 
+	local entry = self.entryCache[saveInfo.filename]
+	entry.character = saveInfo.character
+	entry.compatible = saveInfo.compatible
+	entry.credits = saveInfo.credits
+	entry.duration = saveInfo.duration
+	entry.gameTime = saveInfo.time
+	entry.locationName = location
+	entry.shipName = saveInfo.shipName
+	entry.shipHull = shipHull
+
+	-- profileEndScope()
+end
+
+local function onSaveGameStats(saveInfo)
+	ui.saveLoadWindow:onSaveGameStats(saveInfo)
+end
+
+-- Trigger load of savegame information and return bare-bones entry
+local function makeEntryForSave(file)
+	-- local profileEndScope = utils.profile("makeEntryForSave()")
+	Game.SaveGameStats(file.name)
+
+	local saveEntry = SaveGameEntry:clone({
+		name = file.name,
+		isAutosave = file.name:sub(1, 1) == "_",
+		timestamp = file.mtime.timestamp,
+	})
+
+	-- profileEndScope()
 	return saveEntry
 end
 
@@ -243,6 +258,7 @@ end
 --=============================================================================
 
 function SaveLoadWindow:makeFilteredList()
+	-- local profileEndScope = utils.profile("SaveLoadWindow::makeFilteredList()")
 	local shouldShow = function(f)
 		if not self.showAutosaves and f.name:sub(1, 1) == "_" then
 			return false
@@ -266,9 +282,11 @@ function SaveLoadWindow:makeFilteredList()
 	if not utils.contains_if(self.filteredFiles, isSelectedFile) then
 		self.selectedFile = nil
 	end
+	-- profileEndScope()
 end
 
 function SaveLoadWindow:makeFileList()
+	-- local profileEndScope = utils.profile("SaveLoadWindow::makeFileList()")
 	local ok, files = pcall(Game.ListSaves)
 
 	if not ok then
@@ -282,7 +300,15 @@ function SaveLoadWindow:makeFileList()
 		return a.mtime.timestamp > b.mtime.timestamp
 	end)
 
+	-- Cache details about each savefile
+	for _, file in ipairs(self.files) do
+		if not self.entryCache[file.name] or self.entryCache[file.name].timestamp ~= file.mtime.timestamp then
+			self.entryCache[file.name] = makeEntryForSave(file)
+		end
+	end
+
 	self:makeFilteredList()
+	-- profileEndScope()
 end
 
 function SaveLoadWindow:loadSelectedSave()
@@ -332,21 +358,6 @@ function SaveLoadWindow:deleteSelectedSave()
 	end)
 end
 
-function SaveLoadWindow:update()
-	ModalWindow.update(self)
-
-	-- Incrementally update cache until all files are up to date
-	-- We don't need to manually clear the cache, as changes to the list of
-	-- files will trigger the cache to be updated
-	local uncached = utils.find_if(self.files, function(_, file)
-		return not self.entryCache[file.name] or self.entryCache[file.name].timestamp ~= file.mtime.timestamp
-	end)
-
-	if uncached then
-		self.entryCache[uncached.name] = makeEntryForSave(uncached)
-	end
-end
-
 --=============================================================================
 
 function SaveLoadWindow:onOpen()
@@ -552,6 +563,8 @@ end
 
 --=============================================================================
 
+Event.Register("onSaveGameStats", onSaveGameStats)
+
 ui.saveLoadWindow = SaveLoadWindow
 
 return {}

src/Pi.cpp

@@ -508,7 +508,7 @@ void StartupScreen::Start()
 	// XXX UI requires Lua  but Pi::ui must exist before we start loading
 	// templates. so now we have crap everywhere :/
 	Output("Lua::Init()\n");
-	Lua::Init();
+	Lua::Init(Pi::GetAsyncJobQueue());
 
 	// TODO: Get the lua state responsible for drawing the init progress up as fast as possible
 	// Investigate using a pigui-only Lua state that we can initialize without depending on

src/SaveGameManager.cpp

@@ -17,6 +17,29 @@ static const char s_saveDirName[] = "savefiles";
 
 static const int s_saveVersion = 90;
 
+// A simple job to load a savegame into a Json object
+class LoadGameToJsonJob : public Job
+{
+public:
+	LoadGameToJsonJob(std::string_view filename, void(*callback)(std::string_view, const Json &)) :
+		m_filename(filename), m_callback(callback)
+	{
+	}
+
+	virtual void OnRun() {
+		m_rootNode = SaveGameManager::LoadGameToJson(m_filename);
+	};
+	virtual void OnFinish() {
+		m_callback(m_filename, m_rootNode);
+	};
+	virtual void OnCancel() {};
+private:
+	std::string m_filename;
+	Json m_rootNode;
+	void(*m_callback)(std::string_view, const Json &);
+};
+
+
 void SaveGameManager::Init()
 {
 	if (!FileSystem::userFiles.MakeDirectory(s_saveDirName)) {
@@ -59,6 +82,11 @@ Json SaveGameManager::LoadGameToJson(const std::string &filename)
 	return JsonUtils::LoadJsonSaveFile(FileSystem::JoinPathBelow(s_saveDirName, filename), FileSystem::userFiles);
 }
 
+Job *SaveGameManager::LoadGameToJsonAsync(std::string_view filename, void(*callback)(std::string_view, const Json &))
+{
+	return new LoadGameToJsonJob(filename, callback);
+}
+
 Game *SaveGameManager::LoadGame(const std::string &name)
 {
 	Output("SaveGameManager::LoadGame('%s')\n", name.c_str());

src/SaveGameManager.h

@@ -11,6 +11,7 @@
 #include <vector>
 
 class Game;
+class Job;
 
 class SaveGameManager {
 public:
@@ -48,6 +49,19 @@ class SaveGameManager {
 	 */
 	static Json LoadGameToJson(const std::string &name);
 
+	/** Create a job which can be scheduled on a job queue to load the game as
+	 * a Json object.
+	 * This is provided as LoadGameToJson can be expensive.
+	 *
+	 * The \p callback is called in the main thread with the Json data once the
+	 * job has completed.
+	 *
+	 * \param[in] name The name of the savegame to load.
+	 * \param[in] callback A callback to be called once the data has been loaded.
+	 * \return On success, a newly-created Job which can be passed to a job queue.
+	 */
+	static Job *LoadGameToJsonAsync(std::string_view name, void(*callback)(std::string_view, const Json &));
+
 	/** Save the game.
 	 * NOTE: This function will throw an exception if an error occurs while
 	 * saving the game.

src/editor/EditorApp.cpp

@@ -108,7 +108,7 @@ void EditorApp::OnStartup()
 	m_editorCfg->Save(); // write defaults if the file doesn't exist
 
 	EnumStrings::Init();
-	Lua::Init();
+	Lua::Init(GetAsyncJobQueue());
 	ModManager::Init();
 
 	ModManager::LoadMods(m_editorCfg.get());

src/lua/Lua.cpp

@@ -44,9 +44,9 @@ namespace Lua {
 
 	void InitMath();
 
-	void Init()
+	void Init(JobQueue *asyncJobQueue)
 	{
-		manager = new LuaManager();
+		manager = new LuaManager(asyncJobQueue);
 		InitMath();
 	}
 

src/lua/Lua.h

@@ -6,14 +6,17 @@
 
 #include "LuaManager.h"
 
+class JobQueue;
+
 // home for the global Lua context. here so its shareable between pioneer and
 // modelviewer. probably sucks in the long term
 namespace Lua {
 
 	extern LuaManager *manager;
 
 	// Initialize the lua instance
-	void Init();
+	void Init(JobQueue *asyncJobQueue);
+
 	// Uninitialize the lua instance
 	void Uninit();
 

src/lua/LuaEngine.cpp

@@ -103,6 +103,28 @@ static int l_engine_attr_time(lua_State *l)
 	return 1;
 }
 
+/*
+ * Attribute: nowTime
+ *
+ * Returns an arbitrary value in seconds relative to some epoch corresponding
+ * to the precise time this value is accessed. This should be used only for
+ * profiling and debugging purposes to calculate a duration in sub-millisecond
+ * units.
+ *
+ * Availability:
+ *
+ *   October 2024
+ *
+ * Status:
+ *
+ *   experimental
+ */
+static int l_engine_attr_now_time(lua_State *l)
+{
+	lua_pushnumber(l, Profiler::Clock::ms(Profiler::Clock::getticks()) / 1000.0);
+	return 1;
+}
+
 /*
  * Attribute: frameTime
  *
@@ -1171,6 +1193,7 @@ void LuaEngine::Register()
 		{ "rand", l_engine_attr_rand },
 		{ "ticks", l_engine_attr_ticks },
 		{ "time", l_engine_attr_time },
+		{ "nowTime", l_engine_attr_now_time },
 		{ "frameTime", l_engine_attr_frame_time },
 		{ "pigui", l_engine_attr_pigui },
 		{ "version", l_engine_attr_version },