From 2d1dd41ef5dcb51ddb607ba572e63b605b9191be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20J=2E=20Est=C3=A9banez?= Date: Tue, 10 Sep 2024 11:08:51 +0200 Subject: [PATCH 1/2] WorkerThreadPool: Enhance lifetime for more flexibility --- core/object/worker_thread_pool.cpp | 5 ++++- core/register_core_types.cpp | 8 +------- main/main.cpp | 18 ++++++++++++++++++ 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/core/object/worker_thread_pool.cpp b/core/object/worker_thread_pool.cpp index fe7bbd474c3..da503966b18 100644 --- a/core/object/worker_thread_pool.cpp +++ b/core/object/worker_thread_pool.cpp @@ -326,6 +326,8 @@ WorkerThreadPool::TaskID WorkerThreadPool::add_native_task(void (*p_func)(void * } WorkerThreadPool::TaskID WorkerThreadPool::_add_task(const Callable &p_callable, void (*p_func)(void *), void *p_userdata, BaseTemplateUserdata *p_template_userdata, bool p_high_priority, const String &p_description) { + ERR_FAIL_COND_V_MSG(threads.is_empty(), INVALID_TASK_ID, "Can't add a task because the WorkerThreadPool is either not initialized yet or already terminated."); + task_mutex.lock(); // Get a free task Task *task = task_allocator.alloc(); @@ -538,6 +540,7 @@ void WorkerThreadPool::notify_yield_over(TaskID p_task_id) { } WorkerThreadPool::GroupID WorkerThreadPool::_add_group_task(const Callable &p_callable, void (*p_func)(void *, uint32_t), void *p_userdata, BaseTemplateUserdata *p_template_userdata, int p_elements, int p_tasks, bool p_high_priority, const String &p_description) { + ERR_FAIL_COND_V_MSG(threads.is_empty(), INVALID_TASK_ID, "Can't add a group task because the WorkerThreadPool is either not initialized yet or already terminated."); ERR_FAIL_COND_V(p_elements < 0, INVALID_TASK_ID); if (p_tasks < 0) { p_tasks = MAX(1u, threads.size()); @@ -749,5 +752,5 @@ WorkerThreadPool::WorkerThreadPool() { } WorkerThreadPool::~WorkerThreadPool() { - finish(); + DEV_ASSERT(threads.size() == 0 && "finish() hasn't been called!"); } diff --git a/core/register_core_types.cpp b/core/register_core_types.cpp index c866ff04152..220ed9da313 100644 --- a/core/register_core_types.cpp +++ b/core/register_core_types.cpp @@ -107,8 +107,6 @@ static Time *_time = nullptr; static core_bind::Geometry2D *_geometry_2d = nullptr; static core_bind::Geometry3D *_geometry_3d = nullptr; -static WorkerThreadPool *worker_thread_pool = nullptr; - extern Mutex _global_mutex; static GDExtensionManager *gdextension_manager = nullptr; @@ -297,8 +295,6 @@ void register_core_types() { GDREGISTER_NATIVE_STRUCT(AudioFrame, "float left;float right"); GDREGISTER_NATIVE_STRUCT(ScriptLanguageExtensionProfilingInfo, "StringName signature;uint64_t call_count;uint64_t total_time;uint64_t self_time"); - worker_thread_pool = memnew(WorkerThreadPool); - OS::get_singleton()->benchmark_end_measure("Core", "Register Types"); } @@ -349,7 +345,7 @@ void register_core_singletons() { Engine::get_singleton()->add_singleton(Engine::Singleton("Time", Time::get_singleton())); Engine::get_singleton()->add_singleton(Engine::Singleton("GDExtensionManager", GDExtensionManager::get_singleton())); Engine::get_singleton()->add_singleton(Engine::Singleton("ResourceUID", ResourceUID::get_singleton())); - Engine::get_singleton()->add_singleton(Engine::Singleton("WorkerThreadPool", worker_thread_pool)); + Engine::get_singleton()->add_singleton(Engine::Singleton("WorkerThreadPool", WorkerThreadPool::get_singleton())); OS::get_singleton()->benchmark_end_measure("Core", "Register Singletons"); } @@ -382,8 +378,6 @@ void unregister_core_types() { // Destroy singletons in reverse order to ensure dependencies are not broken. - memdelete(worker_thread_pool); - memdelete(_engine_debugger); memdelete(_marshalls); memdelete(_classdb); diff --git a/main/main.cpp b/main/main.cpp index 18ffedef187..9c9542325ef 100644 --- a/main/main.cpp +++ b/main/main.cpp @@ -140,6 +140,7 @@ static Engine *engine = nullptr; static ProjectSettings *globals = nullptr; static Input *input = nullptr; static InputMap *input_map = nullptr; +static WorkerThreadPool *worker_thread_pool = nullptr; static TranslationServer *translation_server = nullptr; static Performance *performance = nullptr; static PackedData *packed_data = nullptr; @@ -690,6 +691,8 @@ Error Main::test_setup() { register_core_settings(); // Here globals are present. + worker_thread_pool = memnew(WorkerThreadPool); + translation_server = memnew(TranslationServer); tsman = memnew(TextServerManager); @@ -800,6 +803,8 @@ void Main::test_cleanup() { ResourceSaver::remove_custom_savers(); PropertyListHelper::clear_base_helpers(); + WorkerThreadPool::get_singleton()->finish(); + #ifdef TOOLS_ENABLED GDExtensionManager::get_singleton()->deinitialize_extensions(GDExtension::INITIALIZATION_LEVEL_EDITOR); uninitialize_modules(MODULE_INITIALIZATION_LEVEL_EDITOR); @@ -841,6 +846,9 @@ void Main::test_cleanup() { if (physics_server_2d_manager) { memdelete(physics_server_2d_manager); } + if (worker_thread_pool) { + memdelete(worker_thread_pool); + } if (globals) { memdelete(globals); } @@ -931,6 +939,7 @@ Error Main::setup(const char *execpath, int argc, char *argv[], bool p_second_ph register_core_settings(); //here globals are present + worker_thread_pool = memnew(WorkerThreadPool); translation_server = memnew(TranslationServer); performance = memnew(Performance); GDREGISTER_CLASS(Performance); @@ -2620,6 +2629,10 @@ error: if (translation_server) { memdelete(translation_server); } + if (worker_thread_pool) { + worker_thread_pool->finish(); + memdelete(worker_thread_pool); + } if (globals) { memdelete(globals); } @@ -4501,6 +4514,8 @@ void Main::cleanup(bool p_force) { ResourceLoader::clear_translation_remaps(); ResourceLoader::clear_path_remaps(); + WorkerThreadPool::get_singleton()->finish(); + ScriptServer::finish_languages(); // Sync pending commands that may have been queued from a different thread during ScriptServer finalization @@ -4591,6 +4606,9 @@ void Main::cleanup(bool p_force) { if (physics_server_2d_manager) { memdelete(physics_server_2d_manager); } + if (worker_thread_pool) { + memdelete(worker_thread_pool); + } if (globals) { memdelete(globals); } From c8acf561ef0c66c03d9e15e46e753c98ab485050 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20J=2E=20Est=C3=A9banez?= Date: Tue, 10 Sep 2024 11:29:54 +0200 Subject: [PATCH 2/2] Make languages' thread enter/exit more resilient --- core/object/script_language.cpp | 17 +++++++++++++++++ core/object/script_language.h | 2 ++ core/object/worker_thread_pool.cpp | 15 +++++++++------ core/object/worker_thread_pool.h | 2 -- 4 files changed, 28 insertions(+), 8 deletions(-) diff --git a/core/object/script_language.cpp b/core/object/script_language.cpp index 57e51951377..d5b7bc768da 100644 --- a/core/object/script_language.cpp +++ b/core/object/script_language.cpp @@ -41,6 +41,7 @@ ScriptLanguage *ScriptServer::_languages[MAX_LANGUAGES]; int ScriptServer::_language_count = 0; bool ScriptServer::languages_ready = false; Mutex ScriptServer::languages_mutex; +thread_local bool ScriptServer::thread_entered = false; bool ScriptServer::scripting_enabled = true; bool ScriptServer::reload_scripts_on_save = false; @@ -326,6 +327,10 @@ bool ScriptServer::are_languages_initialized() { return languages_ready; } +bool ScriptServer::thread_is_entered() { + return thread_entered; +} + void ScriptServer::set_reload_scripts_on_save(bool p_enable) { reload_scripts_on_save = p_enable; } @@ -335,6 +340,10 @@ bool ScriptServer::is_reload_scripts_on_save_enabled() { } void ScriptServer::thread_enter() { + if (thread_entered) { + return; + } + MutexLock lock(languages_mutex); if (!languages_ready) { return; @@ -342,9 +351,15 @@ void ScriptServer::thread_enter() { for (int i = 0; i < _language_count; i++) { _languages[i]->thread_enter(); } + + thread_entered = true; } void ScriptServer::thread_exit() { + if (!thread_entered) { + return; + } + MutexLock lock(languages_mutex); if (!languages_ready) { return; @@ -352,6 +367,8 @@ void ScriptServer::thread_exit() { for (int i = 0; i < _language_count; i++) { _languages[i]->thread_exit(); } + + thread_entered = false; } HashMap ScriptServer::global_classes; diff --git a/core/object/script_language.h b/core/object/script_language.h index e38c344ae57..d9e2ab1d3c7 100644 --- a/core/object/script_language.h +++ b/core/object/script_language.h @@ -54,6 +54,7 @@ class ScriptServer { static int _language_count; static bool languages_ready; static Mutex languages_mutex; + static thread_local bool thread_entered; static bool scripting_enabled; static bool reload_scripts_on_save; @@ -101,6 +102,7 @@ public: static void init_languages(); static void finish_languages(); static bool are_languages_initialized(); + static bool thread_is_entered(); }; class PlaceHolderScriptInstance; diff --git a/core/object/worker_thread_pool.cpp b/core/object/worker_thread_pool.cpp index da503966b18..cf396c26760 100644 --- a/core/object/worker_thread_pool.cpp +++ b/core/object/worker_thread_pool.cpp @@ -63,17 +63,14 @@ void WorkerThreadPool::_process_task(Task *p_task) { // Tasks must start with these at default values. They are free to set-and-forget otherwise. set_current_thread_safe_for_nodes(false); MessageQueue::set_thread_singleton_override(nullptr); + // Since the WorkerThreadPool is started before the script server, // its pre-created threads can't have ScriptServer::thread_enter() called on them early. // Therefore, we do it late at the first opportunity, so in case the task // about to be run uses scripting, guarantees are held. + ScriptServer::thread_enter(); + task_mutex.lock(); - if (!curr_thread.ready_for_scripting && ScriptServer::are_languages_initialized()) { - task_mutex.unlock(); - ScriptServer::thread_enter(); - task_mutex.lock(); - curr_thread.ready_for_scripting = true; - } p_task->pool_thread_index = pool_thread_index; prev_task = curr_thread.current_task; curr_thread.current_task = p_task; @@ -516,6 +513,12 @@ void WorkerThreadPool::yield() { int th_index = get_thread_index(); ERR_FAIL_COND_MSG(th_index == -1, "This function can only be called from a worker thread."); _wait_collaboratively(&threads[th_index], ThreadData::YIELDING); + + // If this long-lived task started before the scripting server was initialized, + // now is a good time to have scripting languages ready for the current thread. + // Otherwise, such a piece of setup won't happen unless another task has been + // run during the collaborative wait. + ScriptServer::thread_enter(); } void WorkerThreadPool::notify_yield_over(TaskID p_task_id) { diff --git a/core/object/worker_thread_pool.h b/core/object/worker_thread_pool.h index 5be4f209270..6374dbe8c7e 100644 --- a/core/object/worker_thread_pool.h +++ b/core/object/worker_thread_pool.h @@ -112,7 +112,6 @@ private: uint32_t index = 0; Thread thread; - bool ready_for_scripting : 1; bool signaled : 1; bool yield_is_over : 1; Task *current_task = nullptr; @@ -120,7 +119,6 @@ private: ConditionVariable cond_var; ThreadData() : - ready_for_scripting(false), signaled(false), yield_is_over(false) {} };