From 0c620929e0f2d15a2b81f703830ae897fbbc280d Mon Sep 17 00:00:00 2001 From: Webster Sheets Date: Thu, 10 Oct 2024 19:49:53 -0400 Subject: [PATCH 1/3] Add utils.profile - Simple manual profiler for checking the time cost of certain Lua methods - Add Engine.nowTime, which returns the exact time since an unspecified epoch at the moment it's evaluated --- data/libs/utils.lua | 16 ++++++++++++++++ src/lua/LuaEngine.cpp | 23 +++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/data/libs/utils.lua b/data/libs/utils.lua index 2d9ea9d341..a30fc5f44e 100644 --- a/data/libs/utils.lua +++ b/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 diff --git a/src/lua/LuaEngine.cpp b/src/lua/LuaEngine.cpp index 6a1fe6e9fe..454d389749 100644 --- a/src/lua/LuaEngine.cpp +++ b/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 }, From f31dbedd249e376244d180531a3d292eb8458f2c Mon Sep 17 00:00:00 2001 From: Michael Werle Date: Wed, 9 Oct 2024 17:50:26 +0900 Subject: [PATCH 2/3] refactor: run SaveGameStats in a job The idea is to run this expensive task in an asynchronous job and notify the Lua side once it is completed. This is to prevent the stutter from occurring every time the SaveGameStats API is called from Lua. This commit implements this idea by: * add a job manager to the LuaManager class which can run any job on the Lua job queue. * add a LoadGameToJsonJob implementation in SaveGameManager, and a new function, LoadGameToJsonAsync(), which schedules an instance of this job on the Lua job queue. * call this new function from Lua, as well as registering a callback for when the job completes, in saveloadgame.lua. Anecdotal testing in debug builds shows an impressive speed increase when loading the list of savegames. --- data/pigui/modules/saveloadgame.lua | 67 ++++++++++++------ src/Pi.cpp | 2 +- src/SaveGameManager.cpp | 28 ++++++++ src/SaveGameManager.h | 14 ++++ src/editor/EditorApp.cpp | 2 +- src/lua/Lua.cpp | 4 +- src/lua/Lua.h | 5 +- src/lua/LuaGame.cpp | 105 ++++++++++++++++------------ src/lua/LuaManager.cpp | 11 ++- src/lua/LuaManager.h | 11 ++- src/main.cpp | 2 +- 11 files changed, 175 insertions(+), 76 deletions(-) diff --git a/data/pigui/modules/saveloadgame.lua b/data/pigui/modules/saveloadgame.lua index b4a5644016..23cf0655b0 100644 --- a/data/pigui/modules/saveloadgame.lua +++ b/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 @@ -283,6 +301,7 @@ function SaveLoadWindow:makeFileList() end) self:makeFilteredList() + -- profileEndScope() end function SaveLoadWindow:loadSelectedSave() @@ -552,6 +571,8 @@ end --============================================================================= +Event.Register("onSaveGameStats", onSaveGameStats) + ui.saveLoadWindow = SaveLoadWindow return {} diff --git a/src/Pi.cpp b/src/Pi.cpp index 21ed4b6a2d..a44c0d37e0 100644 --- a/src/Pi.cpp +++ b/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 diff --git a/src/SaveGameManager.cpp b/src/SaveGameManager.cpp index 3963dc7c40..18ecd4253c 100644 --- a/src/SaveGameManager.cpp +++ b/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()); diff --git a/src/SaveGameManager.h b/src/SaveGameManager.h index bea9ac691b..134d70f716 100644 --- a/src/SaveGameManager.h +++ b/src/SaveGameManager.h @@ -11,6 +11,7 @@ #include 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. diff --git a/src/editor/EditorApp.cpp b/src/editor/EditorApp.cpp index 8db4bbe64f..3c5c9d7508 100644 --- a/src/editor/EditorApp.cpp +++ b/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()); diff --git a/src/lua/Lua.cpp b/src/lua/Lua.cpp index c25eda6fad..0059b7838d 100644 --- a/src/lua/Lua.cpp +++ b/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(); } diff --git a/src/lua/Lua.h b/src/lua/Lua.h index 0637845fba..9b6d29f731 100644 --- a/src/lua/Lua.h +++ b/src/lua/Lua.h @@ -6,6 +6,8 @@ #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 { @@ -13,7 +15,8 @@ namespace Lua { extern LuaManager *manager; // Initialize the lua instance - void Init(); + void Init(JobQueue *asyncJobQueue); + // Uninitialize the lua instance void Uninit(); diff --git a/src/lua/LuaGame.cpp b/src/lua/LuaGame.cpp index 9542d2b56d..be4a6d88f4 100644 --- a/src/lua/LuaGame.cpp +++ b/src/lua/LuaGame.cpp @@ -9,6 +9,7 @@ #include "GameSaveError.h" #include "Json.h" #include "Lang.h" +#include "LuaEvent.h" #include "LuaObject.h" #include "LuaTable.h" #include "LuaUtils.h" @@ -73,74 +74,90 @@ static int l_game_start_game(lua_State *l) return 0; } -/* - * Function: SaveGameStats - * - * Return stats about a game. - * - * > Game.SaveGameStats(filename) - * - * Parameters: - * - * filename - The filename of the save game to retrieve stats for. - * Stats will be loaded from the 'savefiles' directory in the user's game directory. - * - * Availability: - * - * 2018-02-10 - * - * Status: - * - * experimental - */ -static int l_game_savegame_stats(lua_State *l) +/* Marshall the game info into a LuaTable and send it as an event to Lua. + * Callback from the savegame_stats job when the job is finished. This function + * will run in the main thread. */ +static void onSaveGameStatsJobFinished(std::string_view filename, const Json &rootNode) { - const std::string filename = LuaPull(l, 1); + PROFILE_SCOPED() + auto ls = Lua::manager->GetLuaState(); + ScopedTable t(ls); + t.Set("filename", filename); try { - Json rootNode = SaveGameManager::LoadGameToJson(filename); - - LuaTable t(l, 0, 3); - t.Set("time", rootNode["time"].get()); // if this is a newer saved game, show the embedded info if (rootNode["game_info"].is_object()) { - Json gameInfo = rootNode["game_info"]; + const Json &gameInfo = rootNode["game_info"]; + t.Set("compatible", true); t.Set("system", gameInfo["system"].get()); t.Set("ship", gameInfo["ship"].get()); t.Set("credits", gameInfo["credits"].get()); t.Set("flight_state", gameInfo["flight_state"].get()); - if (gameInfo["docked_at"].is_string()) + if (gameInfo.count("docked_at")) { t.Set("docked_at", gameInfo["docked_at"].get()); - - if (gameInfo.count("shipHull")) + } + if (gameInfo.count("shipHull")) { t.Set("shipHull", gameInfo["shipHull"].get()); - if (gameInfo.count("shipName")) + } + if (gameInfo.count("shipName")) { t.Set("shipName", gameInfo["shipName"].get()); - if (gameInfo.count("duration")) + } + if (gameInfo.count("duration")) { t.Set("duration", gameInfo["duration"].get()); - if (gameInfo.count("character")) + } + if (gameInfo.count("character")) { t.Set("character", gameInfo["character"].get()); + } } else { // this is an older saved game...try to show something useful - Json shipNode = rootNode["space"]["bodies"][rootNode["player"].get() - 1]; + const Json &shipNode = rootNode["space"]["bodies"][rootNode["player"].get() - 1]; t.Set("frame", rootNode["space"]["bodies"][shipNode["body"]["index_for_frame"].get() - 1]["body"]["label"].get()); t.Set("ship", shipNode["model_body"]["model_name"].get()); + t.Set("compatible", false); } - - return 1; - } catch (const CouldNotOpenFileException &e) { - const std::string message = stringf(Lang::COULD_NOT_OPEN_FILENAME, formatarg("path", filename)); - lua_pushlstring(l, message.c_str(), message.size()); - return lua_error(l); } catch (const Json::type_error &) { - return luaL_error(l, Lang::GAME_LOAD_CORRUPT); + t.Set("compatible", false); } catch (const Json::out_of_range &) { - return luaL_error(l, Lang::GAME_LOAD_CORRUPT); - } catch (const SavedGameCorruptException &) { - return luaL_error(l, Lang::GAME_LOAD_CORRUPT); + t.Set("compatible", false); } + + LuaEvent::Queue("onSaveGameStats", LuaTable(t)); + LuaEvent::Emit(); +} + +/* + * Function: SaveGameStats + * + * Start a Job to read the SaveGameStats for a particular save game + * + * > Game.SaveGameStats(filename) + * + * Parameters: + * + * filename - The filename of the save game to retrieve stats for. + * Stats will be loaded from the 'savefiles' directory in the user's game directory. + * + * Availability: + * + * 2018-02-10 + * + * Modified: + * + * 2024-10-11 - the function no longer returns the Stats directly, but starts + * a Job. The Lua caller is responsible for registering an + * "onSaveGameStats" event handler to process the data. + * + * Status: + * + * experimental + */ +static int l_game_savegame_stats(lua_State *l) +{ + const std::string filename = LuaPull(l, 1); + Lua::manager->ScheduleJob(SaveGameManager::LoadGameToJsonAsync(filename, onSaveGameStatsJobFinished)); + return 0; } diff --git a/src/lua/LuaManager.cpp b/src/lua/LuaManager.cpp index f27eac0f1a..e8a0becd54 100644 --- a/src/lua/LuaManager.cpp +++ b/src/lua/LuaManager.cpp @@ -9,8 +9,8 @@ bool instantiated = false; -LuaManager::LuaManager() : - m_lua(0) +LuaManager::LuaManager(JobQueue *asyncJobQueue) : + m_lua(0), m_jobs(asyncJobQueue) { if (instantiated) { Output("Can't instantiate more than one LuaManager"); @@ -51,3 +51,10 @@ void LuaManager::CollectGarbage() { lua_gc(m_lua, LUA_GCCOLLECT, 0); } + +void LuaManager::ScheduleJob(Job *job) +{ + if (job) { + m_jobs.Order(job); + } +} diff --git a/src/lua/LuaManager.h b/src/lua/LuaManager.h index 7be3b4ef7a..bd3e69c4cb 100644 --- a/src/lua/LuaManager.h +++ b/src/lua/LuaManager.h @@ -4,22 +4,31 @@ #ifndef _LUAMANAGER_H #define _LUAMANAGER_H +#include "JobQueue.h" #include "LuaUtils.h" class LuaManager { public: - LuaManager(); + // Create a new LuaManager + // @param[in] asyncJobQueue A job queue to run Lua jobs on. This should be + // an asynchronous job queue to allow jobs to run in the + // background via the \p ScheduleJob method. + LuaManager(JobQueue *asyncJobQueue); ~LuaManager(); lua_State *GetLuaState() { return m_lua; } size_t GetMemoryUsage() const; void CollectGarbage(); + // Schedule a job to be run on the LuaManager job queue + void ScheduleJob(Job *job); + private: LuaManager(const LuaManager &); LuaManager &operator=(const LuaManager &) = delete; lua_State *m_lua; + JobSet m_jobs; }; #endif diff --git a/src/main.cpp b/src/main.cpp index 747221ff46..325c8a06af 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -186,7 +186,7 @@ extern "C" int main(int argc, char **argv) // Galaxy generation is (mostly) self-contained, no need to e.g. // turn on the renderer or load UI for this. - Lua::Init(); + Lua::Init(Pi::GetAsyncJobQueue()); Pi::luaNameGen = new LuaNameGen(Lua::manager); LuaObject::RegisterClass(); FILE *file = filename == "-" ? stdout : fopen(filename.c_str(), "w"); From f20eb57830730a951e2bf3df0cec6a0749fc4d10 Mon Sep 17 00:00:00 2001 From: Webster Sheets Date: Wed, 16 Oct 2024 16:44:15 -0400 Subject: [PATCH 3/3] refactor: remove manual rate-limiting in saveloadgame.lua - Prior code limited synchronous Game.SaveGameStats calls to once-per-frame - Since the function is now async, we can dispatch as many jobs as we want and their results will be processed on completion --- data/pigui/modules/saveloadgame.lua | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/data/pigui/modules/saveloadgame.lua b/data/pigui/modules/saveloadgame.lua index 23cf0655b0..5cb3317b03 100644 --- a/data/pigui/modules/saveloadgame.lua +++ b/data/pigui/modules/saveloadgame.lua @@ -300,6 +300,13 @@ 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 @@ -351,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()