From 3b49576fa69e71de9003ee03a88ba445bcaa3574 Mon Sep 17 00:00:00 2001 From: Yves Date: Mon, 22 Sep 2014 20:13:04 +0000 Subject: [PATCH] Tunes GC scheduling a bit to reduce memory usage. The main problem was that GC was only called from the simulation before this patch. This means when you were waiting in the multiplayer lobby or just had the GUI open, it only called GC when getting close to the JS runtime size limit (I assume). Another problem was the Net Server runtime which didn't GC either. Here the runtime size limit is 16 MB though, so it's not too terrible. These issues have both been addressed and GC has been given a bit more time per incremental slice to make sure it gets done in time. It's still far from perfect, but there are too many changes in SpiderMonkey related to GC, so I don't want to spend too much time on this yet. Refs #2808 This was SVN commit r15787. --- source/gui/GUIManager.cpp | 4 ++ source/network/NetServer.cpp | 2 + source/ps/GameSetup/GameSetup.cpp | 7 ++- source/ps/Replay.cpp | 5 +- source/scriptinterface/ScriptInterface.cpp | 53 +++++++++++++++------- source/scriptinterface/ScriptInterface.h | 12 +++-- source/simulation2/Simulation2.cpp | 5 +- 7 files changed, 63 insertions(+), 25 deletions(-) diff --git a/source/gui/GUIManager.cpp b/source/gui/GUIManager.cpp index c7fa5fdbc8..426a6eb573 100644 --- a/source/gui/GUIManager.cpp +++ b/source/gui/GUIManager.cpp @@ -377,6 +377,10 @@ void CGUIManager::TickObjects() { PROFILE3("gui tick"); + // We share the script runtime with everything else that runs in the same thread. + // This call makes sure we trigger GC regularly even if the simulation is not running. + m_ScriptInterface->MaybeIncrementalRuntimeGC(1.0f); + // Save an immutable copy so iterators aren't invalidated by tick handlers PageStackType pageStack = m_PageStack; diff --git a/source/network/NetServer.cpp b/source/network/NetServer.cpp index 86575dec21..467f2a6d9f 100644 --- a/source/network/NetServer.cpp +++ b/source/network/NetServer.cpp @@ -391,6 +391,8 @@ bool CNetServerWorker::RunStep() // (Do as little work as possible while the mutex is held open, // to avoid performance problems and deadlocks.) + m_ScriptInterface->MaybeIncrementalRuntimeGC(0.5f); + JSContext* cx = m_ScriptInterface->GetContext(); JSAutoRequest rq(cx); diff --git a/source/ps/GameSetup/GameSetup.cpp b/source/ps/GameSetup/GameSetup.cpp index 1ff7ab9abf..a4ee7d398b 100644 --- a/source/ps/GameSetup/GameSetup.cpp +++ b/source/ps/GameSetup/GameSetup.cpp @@ -886,8 +886,11 @@ bool Init(const CmdLineArgs& args, int flags) // (required for finding our output log files). g_Logger = new CLogger; - // Workaround until Simulation and AI use their own threads and also their own runtimes - g_ScriptRuntime = ScriptInterface::CreateRuntime(384 * 1024 * 1024); + // Using a global object for the runtime is a workaround until Simulation and AI use + // their own threads and also their own runtimes. + const int runtimeSize = 384 * 1024 * 1024; + const int heapGrowthBytesGCTrigger = 20 * 1024 * 1024; + g_ScriptRuntime = ScriptInterface::CreateRuntime(runtimeSize, heapGrowthBytesGCTrigger); // Special command-line mode to dump the entity schemas instead of running the game. // (This must be done after loading VFS etc, but should be done before wasting time diff --git a/source/ps/Replay.cpp b/source/ps/Replay.cpp index 48f5499232..098aaeee74 100644 --- a/source/ps/Replay.cpp +++ b/source/ps/Replay.cpp @@ -132,7 +132,10 @@ void CReplayPlayer::Replay(bool serializationtest) new CProfileManager; g_ScriptStatsTable = new CScriptStatsTable; g_ProfileViewer.AddRootTable(g_ScriptStatsTable); - g_ScriptRuntime = ScriptInterface::CreateRuntime(384 * 1024 * 1024); + + const int runtimeSize = 384 * 1024 * 1024; + const int heapGrowthBytesGCTrigger = 20 * 1024 * 1024; + g_ScriptRuntime = ScriptInterface::CreateRuntime(runtimeSize, heapGrowthBytesGCTrigger); CGame game(true); g_Game = &game; diff --git a/source/scriptinterface/ScriptInterface.cpp b/source/scriptinterface/ScriptInterface.cpp index 0f6297ffd9..a87ee1be47 100644 --- a/source/scriptinterface/ScriptInterface.cpp +++ b/source/scriptinterface/ScriptInterface.cpp @@ -135,9 +135,11 @@ void GCSliceCallbackHook(JSRuntime* UNUSED(rt), JS::GCProgress progress, const J class ScriptRuntime { public: - ScriptRuntime(int runtimeSize): + ScriptRuntime(int runtimeSize, int heapGrowthBytesGCTrigger): m_rooter(NULL), - m_LastGCBytes(0) + m_LastGCBytes(0), + m_LastGCCheck(0.0f), + m_HeapGrowthBytesGCTrigger(heapGrowthBytesGCTrigger) { m_rt = JS_NewRuntime(runtimeSize, JS_USE_HELPER_THREADS); @@ -175,9 +177,10 @@ public: ENSURE(m_dummyContext); } - void MaybeIncrementalGC() + void MaybeIncrementalGC(double delay) { - PROFILE3("MaybeIncrementalGC"); + PROFILE2("MaybeIncrementalGC"); + if (JS::IsIncrementalGCEnabled(m_rt)) { // The idea is to get the heap size after a completed GC and trigger the next GC when the heap size has @@ -185,8 +188,20 @@ public: // In practice it doesn't quite work like that. When the incremental marking is completed, the sweeping kicks in. // The sweeping actually frees memory and it does this in a background thread (if JS_USE_HELPER_THREADS is set). // While the sweeping is happening we already run scripts again and produce new garbage. + + const int GCSliceTimeBudget = 30; // Milliseconds an incremental slice is allowed to run + + // Have a minimum time in seconds to wait between GC slices and before starting a new GC to distribute the GC + // load and to hopefully make it unnoticeable for the player. This value should be high enough to distribute + // the load well enough and low enough to make sure we don't run out of memory before we can start with the + // sweeping. + if (timer_Time() - m_LastGCCheck < delay) + return; + + m_LastGCCheck = timer_Time(); + int gcBytes = JS_GetGCParameter(m_rt, JSGC_BYTES); - //printf("gcBytes: %d \n", gcBytes); + //std::cout << "gcBytes: " << std::endl; // debugging if (m_LastGCBytes > gcBytes || m_LastGCBytes == 0) { //printf("Setting m_LastGCBytes: %d \n", gcBytes); // debugging @@ -196,16 +211,20 @@ public: // Run an additional incremental GC slice if the currently running incremental GC isn't over yet // ... or // start a new incremental GC if the JS heap size has grown enough for a GC to make sense - if (JS::IsIncrementalGCInProgress(m_rt) || (gcBytes - m_LastGCBytes > 20 * 1024 * 1024)) + if (JS::IsIncrementalGCInProgress(m_rt) || (gcBytes - m_LastGCBytes > m_HeapGrowthBytesGCTrigger)) { - /* Use for debugging - if (JS::IsIncrementalGCInProgress(m_rt)) - printf("Running incremental garbage collection because an incremental cycle is in progress. \n"); + // Use for debugging + /*if (JS::IsIncrementalGCInProgress(m_rt)) + printf("Running incremental GC because an incremental cycle is in progress. \n"); else - printf("Running incremental garbage collection because JSGC_BYTES - m_LastGCBytes > X ---- JSGC_BYTES: %d m_LastGCBytes: %d", gcBytes, m_LastGCBytes); - */ + printf("Running incremental GC because JSGC_BYTES - m_LastGCBytes > m_HeapGrowthBytesGCTrigger " + " ---- JSGC_BYTES: %d m_LastGCBytes: %d m_HeapGrowthBytesGCTrigger: %d ", + gcBytes, + m_LastGCBytes, + m_HeapGrowthBytesGCTrigger); + }*/ PrepareContextsForIncrementalGC(); - JS::IncrementalGC(m_rt, JS::gcreason::REFRESH_FRAME, 10); + JS::IncrementalGC(m_rt, JS::gcreason::REFRESH_FRAME, GCSliceTimeBudget); m_LastGCBytes = gcBytes; } } @@ -236,7 +255,9 @@ private: // Workaround for: https://bugzilla.mozilla.org/show_bug.cgi?id=890243 JSContext* m_dummyContext; + int m_HeapGrowthBytesGCTrigger; int m_LastGCBytes; + double m_LastGCCheck; void PrepareContextsForIncrementalGC() { @@ -374,9 +395,9 @@ private: } }; -shared_ptr ScriptInterface::CreateRuntime(int runtimeSize) +shared_ptr ScriptInterface::CreateRuntime(int runtimeSize, int heapGrowthBytesGCTrigger) { - return shared_ptr(new ScriptRuntime(runtimeSize)); + return shared_ptr(new ScriptRuntime(runtimeSize, heapGrowthBytesGCTrigger)); } //////////////////////////////////////////////////////////////// @@ -1398,9 +1419,9 @@ void ScriptInterface::DumpHeap() fprintf(stderr, "# Bytes allocated after GC: %u\n", JS_GetGCParameter(GetJSRuntime(), JSGC_BYTES)); } -void ScriptInterface::MaybeIncrementalRuntimeGC() +void ScriptInterface::MaybeIncrementalRuntimeGC(double delay) { - m->m_runtime->MaybeIncrementalGC(); + m->m_runtime->MaybeIncrementalGC(delay); } void ScriptInterface::MaybeGC() diff --git a/source/scriptinterface/ScriptInterface.h b/source/scriptinterface/ScriptInterface.h index ccca75bf77..dc24a04dba 100644 --- a/source/scriptinterface/ScriptInterface.h +++ b/source/scriptinterface/ScriptInterface.h @@ -59,6 +59,7 @@ class AutoGCRooter; // TODO: what's a good default? #define DEFAULT_RUNTIME_SIZE 16 * 1024 * 1024 +#define DEFAULT_HEAP_GROWTH_BYTES_GCTRIGGER 2 * 1024 *1024 struct ScriptInterface_impl; @@ -89,7 +90,8 @@ public: * Each runtime should only ever be used on a single thread. * @param runtimeSize Maximum size in bytes of the new runtime */ - static shared_ptr CreateRuntime(int runtimeSize = DEFAULT_RUNTIME_SIZE); + static shared_ptr CreateRuntime(int runtimeSize = DEFAULT_RUNTIME_SIZE, + int heapGrowthBytesGCTrigger = DEFAULT_HEAP_GROWTH_BYTES_GCTRIGGER); /** @@ -354,10 +356,12 @@ public: * be worth the amount of time it would take. It does this with our own logic and NOT some predefined JSAPI logic because * such functionality currently isn't available out of the box. * It does incremental GC which means it will collect one slice each time it's called until the garbage collection is done. - * This can and should be called quite regularly. It shouldn't cost much performance because it tries to run a GC only if - * necessary. + * This can and should be called quite regularly. The delay parameter allows you to specify a minimum time since the last GC + * in seconds (the delay should be a fraction of a second in most cases though). + * It will only start a new incremental GC or another GC slice if this time is exceeded. The user of this function is + * responsible for ensuring that GC can run with a small enough delay to get done with the work. */ - void MaybeIncrementalRuntimeGC(); + void MaybeIncrementalRuntimeGC(double delay); /** * Triggers a full non-incremental garbage collection immediately. That should only be required in special cases and normally diff --git a/source/simulation2/Simulation2.cpp b/source/simulation2/Simulation2.cpp index 2e934f1c10..dd64e538d4 100644 --- a/source/simulation2/Simulation2.cpp +++ b/source/simulation2/Simulation2.cpp @@ -449,10 +449,11 @@ void CSimulation2Impl::Update(int turnLength, const std::vector