From 8c8d6de3e7f93b291fb0bfd1a5c042dd404bda0b Mon Sep 17 00:00:00 2001 From: Stuart Carnie Date: Wed, 9 Apr 2025 07:23:58 +1000 Subject: [PATCH] Apple: Add pthread implementation of `Thread` class This allows Apple platforms to override the default stack size of a thread in the WorkerThreadPool, which is 512KiB by default. This must be increased, as SPIRV-Cross, used by the Metal driver, can use deeply nested stacks, as can debug builds. --- core/object/worker_thread_pool.cpp | 15 +++- core/os/thread.h | 2 + drivers/apple/SCsub | 1 + drivers/apple/thread_apple.cpp | 129 +++++++++++++++++++++++++++++ drivers/apple/thread_apple.h | 110 ++++++++++++++++++++++++ drivers/metal/metal_utils.h | 2 + drivers/unix/thread_posix.cpp | 7 ++ platform/ios/platform_config.h | 2 + platform/ios/platform_thread.h | 33 ++++++++ platform/macos/platform_config.h | 2 + platform/macos/platform_thread.h | 33 ++++++++ tests/core/templates/test_rid.h | 2 +- 12 files changed, 336 insertions(+), 2 deletions(-) create mode 100644 drivers/apple/thread_apple.cpp create mode 100644 drivers/apple/thread_apple.h create mode 100644 platform/ios/platform_thread.h create mode 100644 platform/macos/platform_thread.h diff --git a/core/object/worker_thread_pool.cpp b/core/object/worker_thread_pool.cpp index 84ef71c815..3be0ce4b5f 100644 --- a/core/object/worker_thread_pool.cpp +++ b/core/object/worker_thread_pool.cpp @@ -780,10 +780,23 @@ void WorkerThreadPool::init(int p_thread_count, float p_low_priority_task_ratio) threads.resize(p_thread_count); + Thread::Settings settings; +#ifdef __APPLE__ + // The default stack size for new threads on Apple platforms is 512KiB. + // This is insufficient when using a library like SPIRV-Cross, + // which can generate deep stacks and result in a stack overflow. +#ifdef DEV_ENABLED + // Debug builds need an even larger stack size. + settings.stack_size = 2 * 1024 * 1024; // 2 MiB +#else + settings.stack_size = 1 * 1024 * 1024; // 1 MiB +#endif +#endif + for (uint32_t i = 0; i < threads.size(); i++) { threads[i].index = i; threads[i].pool = this; - threads[i].thread.start(&WorkerThreadPool::_thread_function, &threads[i]); + threads[i].thread.start(&WorkerThreadPool::_thread_function, &threads[i], settings); thread_ids.insert(threads[i].thread.get_id(), i); } } diff --git a/core/os/thread.h b/core/os/thread.h index f5c9a20e59..bb6d42c2a2 100644 --- a/core/os/thread.h +++ b/core/os/thread.h @@ -119,6 +119,8 @@ private: public: static void _set_platform_functions(const PlatformFunctions &p_functions); + _FORCE_INLINE_ static void yield() { std::this_thread::yield(); } + _FORCE_INLINE_ ID get_id() const { return id; } // get the ID of the caller thread _FORCE_INLINE_ static ID get_caller_id() { diff --git a/drivers/apple/SCsub b/drivers/apple/SCsub index 83ac27f4b6..4dbb462c00 100644 --- a/drivers/apple/SCsub +++ b/drivers/apple/SCsub @@ -5,3 +5,4 @@ Import("env") # Driver source files env.add_source_files(env.drivers_sources, "*.mm") +env.add_source_files(env.drivers_sources, "*.cpp") diff --git a/drivers/apple/thread_apple.cpp b/drivers/apple/thread_apple.cpp new file mode 100644 index 0000000000..d07aa3ac88 --- /dev/null +++ b/drivers/apple/thread_apple.cpp @@ -0,0 +1,129 @@ +/**************************************************************************/ +/* thread_apple.cpp */ +/**************************************************************************/ +/* This file is part of: */ +/* GODOT ENGINE */ +/* https://godotengine.org */ +/**************************************************************************/ +/* Copyright (c) 2014-present Godot Engine contributors (see AUTHORS.md). */ +/* Copyright (c) 2007-2014 Juan Linietsky, Ariel Manzur. */ +/* */ +/* Permission is hereby granted, free of charge, to any person obtaining */ +/* a copy of this software and associated documentation files (the */ +/* "Software"), to deal in the Software without restriction, including */ +/* without limitation the rights to use, copy, modify, merge, publish, */ +/* distribute, sublicense, and/or sell copies of the Software, and to */ +/* permit persons to whom the Software is furnished to do so, subject to */ +/* the following conditions: */ +/* */ +/* The above copyright notice and this permission notice shall be */ +/* included in all copies or substantial portions of the Software. */ +/* */ +/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, */ +/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF */ +/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. */ +/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY */ +/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, */ +/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE */ +/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ +/**************************************************************************/ + +#include "thread_apple.h" + +#include "core/error/error_macros.h" +#include "core/object/script_language.h" +#include "core/string/ustring.h" + +SafeNumeric Thread::id_counter(1); // The first value after .increment() is 2, hence by default the main thread ID should be 1. +thread_local Thread::ID Thread::caller_id = Thread::id_counter.increment(); + +struct ThreadData { + Thread::Callback callback; + void *userdata; + Thread::ID caller_id; +}; + +void *Thread::thread_callback(void *p_data) { + ThreadData *thread_data = static_cast(p_data); + + // Set the caller ID for this thread + caller_id = thread_data->caller_id; + + ScriptServer::thread_enter(); // Scripts may need to attach a stack. + + // Call the actual callback + thread_data->callback(thread_data->userdata); + + ScriptServer::thread_exit(); + + // Clean up + memdelete(thread_data); + + return nullptr; +} + +Error Thread::set_name(const String &p_name) { + int err = pthread_setname_np(p_name.utf8().get_data()); + return err == 0 ? OK : ERR_INVALID_PARAMETER; +} + +Thread::ID Thread::start(Thread::Callback p_callback, void *p_user, const Settings &p_settings) { + ERR_FAIL_COND_V_MSG(id != UNASSIGNED_ID, UNASSIGNED_ID, "A Thread object has been re-started without wait_to_finish() having been called on it."); + id = id_counter.increment(); + + ThreadData *thread_data = memnew(ThreadData); + thread_data->callback = p_callback; + thread_data->userdata = p_user; + thread_data->caller_id = id; + + // Create the thread + pthread_attr_t attr; + pthread_attr_init(&attr); + + switch (p_settings.priority) { + case PRIORITY_LOW: + pthread_attr_set_qos_class_np(&attr, QOS_CLASS_UTILITY, 0); + break; + case PRIORITY_NORMAL: + pthread_attr_set_qos_class_np(&attr, QOS_CLASS_USER_INITIATED, 0); + break; + case PRIORITY_HIGH: + pthread_attr_set_qos_class_np(&attr, QOS_CLASS_USER_INTERACTIVE, 0); + break; + } + + if (p_settings.stack_size > 0) { + pthread_attr_setstacksize(&attr, p_settings.stack_size); + } + + // Create the thread + pthread_create(&pthread, &attr, thread_callback, thread_data); + + // Clean up attributes + pthread_attr_destroy(&attr); + + return id; +} + +void Thread::wait_to_finish() { + ERR_FAIL_COND_MSG(id == UNASSIGNED_ID, "Attempt of waiting to finish on a thread that was never started."); + ERR_FAIL_COND_MSG(id == get_caller_id(), "Threads can't wait to finish on themselves, another thread must wait."); + + int err = pthread_join(pthread, nullptr); + if (err != 0) { + ERR_FAIL_MSG("Thread::wait_to_finish() failed to join thread."); + } + pthread = pthread_t(); + id = UNASSIGNED_ID; +} + +Thread::~Thread() { + if (id != UNASSIGNED_ID) { +#ifdef DEBUG_ENABLED + WARN_PRINT( + "A Thread object is being destroyed without its completion having been realized.\n" + "Please call wait_to_finish() on it to ensure correct cleanup."); +#endif + pthread_detach(pthread); + } +} diff --git a/drivers/apple/thread_apple.h b/drivers/apple/thread_apple.h new file mode 100644 index 0000000000..b6880ca32e --- /dev/null +++ b/drivers/apple/thread_apple.h @@ -0,0 +1,110 @@ +/**************************************************************************/ +/* thread_apple.h */ +/**************************************************************************/ +/* This file is part of: */ +/* GODOT ENGINE */ +/* https://godotengine.org */ +/**************************************************************************/ +/* Copyright (c) 2014-present Godot Engine contributors (see AUTHORS.md). */ +/* Copyright (c) 2007-2014 Juan Linietsky, Ariel Manzur. */ +/* */ +/* Permission is hereby granted, free of charge, to any person obtaining */ +/* a copy of this software and associated documentation files (the */ +/* "Software"), to deal in the Software without restriction, including */ +/* without limitation the rights to use, copy, modify, merge, publish, */ +/* distribute, sublicense, and/or sell copies of the Software, and to */ +/* permit persons to whom the Software is furnished to do so, subject to */ +/* the following conditions: */ +/* */ +/* The above copyright notice and this permission notice shall be */ +/* included in all copies or substantial portions of the Software. */ +/* */ +/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, */ +/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF */ +/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. */ +/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY */ +/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, */ +/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE */ +/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ +/**************************************************************************/ + +#pragma once + +#include "core/templates/safe_refcount.h" +#include "core/typedefs.h" + +#include +#include // For hardware interference size + +class String; + +class Thread { +public: + typedef void (*Callback)(void *p_userdata); + + typedef uint64_t ID; + + enum : ID { + UNASSIGNED_ID = 0, + MAIN_ID = 1 + }; + + enum Priority { + PRIORITY_LOW, + PRIORITY_NORMAL, + PRIORITY_HIGH + }; + + struct Settings { + Priority priority; + /// Override the default stack size (0 means default) + uint64_t stack_size = 0; + Settings() { priority = PRIORITY_NORMAL; } + }; + +#if defined(__cpp_lib_hardware_interference_size) + GODOT_GCC_WARNING_PUSH_AND_IGNORE("-Winterference-size") + static constexpr size_t CACHE_LINE_BYTES = std::hardware_destructive_interference_size; + GODOT_GCC_WARNING_POP +#else + // At a negligible memory cost, we use a conservatively high value. + static constexpr size_t CACHE_LINE_BYTES = 128; +#endif + +private: + friend class Main; + + ID id = UNASSIGNED_ID; + pthread_t pthread; + + static SafeNumeric id_counter; + static thread_local ID caller_id; + + static void *thread_callback(void *p_data); + + static void make_main_thread() { caller_id = MAIN_ID; } + static void release_main_thread() { caller_id = id_counter.increment(); } + +public: + _FORCE_INLINE_ static void yield() { pthread_yield_np(); } + + _FORCE_INLINE_ ID get_id() const { return id; } + // get the ID of the caller thread + _FORCE_INLINE_ static ID get_caller_id() { + return caller_id; + } + // get the ID of the main thread + _FORCE_INLINE_ static ID get_main_id() { return MAIN_ID; } + + _FORCE_INLINE_ static bool is_main_thread() { return caller_id == MAIN_ID; } + + static Error set_name(const String &p_name); + + ID start(Thread::Callback p_callback, void *p_user, const Settings &p_settings = Settings()); + bool is_started() const { return id != UNASSIGNED_ID; } + /// Waits until thread is finished, and deallocates it. + void wait_to_finish(); + + Thread() = default; + ~Thread(); +}; diff --git a/drivers/metal/metal_utils.h b/drivers/metal/metal_utils.h index e74fa55234..8f2f311927 100644 --- a/drivers/metal/metal_utils.h +++ b/drivers/metal/metal_utils.h @@ -32,6 +32,8 @@ #import +#import + #pragma mark - Boolean flags namespace flags { diff --git a/drivers/unix/thread_posix.cpp b/drivers/unix/thread_posix.cpp index 7f3b9e9761..885f865bc6 100644 --- a/drivers/unix/thread_posix.cpp +++ b/drivers/unix/thread_posix.cpp @@ -35,6 +35,11 @@ #include "core/os/thread.h" #include "core/string/ustring.h" +#if defined(PLATFORM_THREAD_OVERRIDE) && defined(__APPLE__) +void init_thread_posix() { +} +#else + #ifdef PTHREAD_BSD_SET_NAME #include #endif @@ -73,4 +78,6 @@ void init_thread_posix() { Thread::_set_platform_functions({ .set_name = set_name }); } +#endif // PLATFORM_THREAD_OVERRIDE && __APPLE__ + #endif // UNIX_ENABLED diff --git a/platform/ios/platform_config.h b/platform/ios/platform_config.h index 2f6bbdce2f..bc8b45603a 100644 --- a/platform/ios/platform_config.h +++ b/platform/ios/platform_config.h @@ -32,6 +32,8 @@ #include +#define PLATFORM_THREAD_OVERRIDE + #define PTHREAD_RENAME_SELF #define _weakify(var) __weak typeof(var) GDWeak_##var = var; diff --git a/platform/ios/platform_thread.h b/platform/ios/platform_thread.h new file mode 100644 index 0000000000..42827e53fc --- /dev/null +++ b/platform/ios/platform_thread.h @@ -0,0 +1,33 @@ +/**************************************************************************/ +/* platform_thread.h */ +/**************************************************************************/ +/* This file is part of: */ +/* GODOT ENGINE */ +/* https://godotengine.org */ +/**************************************************************************/ +/* Copyright (c) 2014-present Godot Engine contributors (see AUTHORS.md). */ +/* Copyright (c) 2007-2014 Juan Linietsky, Ariel Manzur. */ +/* */ +/* Permission is hereby granted, free of charge, to any person obtaining */ +/* a copy of this software and associated documentation files (the */ +/* "Software"), to deal in the Software without restriction, including */ +/* without limitation the rights to use, copy, modify, merge, publish, */ +/* distribute, sublicense, and/or sell copies of the Software, and to */ +/* permit persons to whom the Software is furnished to do so, subject to */ +/* the following conditions: */ +/* */ +/* The above copyright notice and this permission notice shall be */ +/* included in all copies or substantial portions of the Software. */ +/* */ +/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, */ +/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF */ +/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. */ +/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY */ +/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, */ +/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE */ +/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ +/**************************************************************************/ + +#pragma once + +#include "drivers/apple/thread_apple.h" diff --git a/platform/macos/platform_config.h b/platform/macos/platform_config.h index 2f6bbdce2f..bc8b45603a 100644 --- a/platform/macos/platform_config.h +++ b/platform/macos/platform_config.h @@ -32,6 +32,8 @@ #include +#define PLATFORM_THREAD_OVERRIDE + #define PTHREAD_RENAME_SELF #define _weakify(var) __weak typeof(var) GDWeak_##var = var; diff --git a/platform/macos/platform_thread.h b/platform/macos/platform_thread.h new file mode 100644 index 0000000000..42827e53fc --- /dev/null +++ b/platform/macos/platform_thread.h @@ -0,0 +1,33 @@ +/**************************************************************************/ +/* platform_thread.h */ +/**************************************************************************/ +/* This file is part of: */ +/* GODOT ENGINE */ +/* https://godotengine.org */ +/**************************************************************************/ +/* Copyright (c) 2014-present Godot Engine contributors (see AUTHORS.md). */ +/* Copyright (c) 2007-2014 Juan Linietsky, Ariel Manzur. */ +/* */ +/* Permission is hereby granted, free of charge, to any person obtaining */ +/* a copy of this software and associated documentation files (the */ +/* "Software"), to deal in the Software without restriction, including */ +/* without limitation the rights to use, copy, modify, merge, publish, */ +/* distribute, sublicense, and/or sell copies of the Software, and to */ +/* permit persons to whom the Software is furnished to do so, subject to */ +/* the following conditions: */ +/* */ +/* The above copyright notice and this permission notice shall be */ +/* included in all copies or substantial portions of the Software. */ +/* */ +/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, */ +/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF */ +/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. */ +/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY */ +/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, */ +/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE */ +/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ +/**************************************************************************/ + +#pragma once + +#include "drivers/apple/thread_apple.h" diff --git a/tests/core/templates/test_rid.h b/tests/core/templates/test_rid.h index bcc267c50d..b63136d585 100644 --- a/tests/core/templates/test_rid.h +++ b/tests/core/templates/test_rid.h @@ -140,7 +140,7 @@ TEST_CASE("[RID_Owner] Thread safety") { uint32_t target = (p_step / 2 + 1) * threads.size(); sync[buf_idx].fetch_add(1, std::memory_order_relaxed); do { - std::this_thread::yield(); + Thread::yield(); } while (sync[buf_idx].load(std::memory_order_relaxed) != target); }