Sara/behaviour-tree-test
1
0
Fork 0
Code Issues Pull requests Projects Releases 2 Packages Wiki Activity Actions

Merge pull request #116064 from Jules5/fix-multithreaded-loading-progress

Browse source 
Fix multithreaded load status progress
This commit is contained in:
Thaddeus Crews 2026-02-24 09:29:05 -06:00
commit 69c274d3bc
No known key found for this signature in database
GPG key ID: 8C6E5FEB5FC03CCC
2 changed files with 22 additions and 47 deletions
Download patch file Download diff file Expand all files Collapse all files

67
core/io/resource_loader.cpp
View file

@ -219,26 +219,6 @@ void ResourceFormatLoader::_bind_methods() {
GDVIRTUAL_BIND(_load, "path", "original_path", "use_sub_threads", "cache_mode");
}
///////////////////////////////////
// These are used before and after a wait for a WorkerThreadPool task
// because that can lead to another load started in the same thread,
// something we must treat as a different stack for the purposes
// of tracking nesting.
#define PREPARE_FOR_WTP_WAIT \
int load_nesting_backup = ResourceLoader::load_nesting; \
Vector<String> load_paths_stack_backup = ResourceLoader::load_paths_stack; \
ResourceLoader::load_nesting = 0; \
ResourceLoader::load_paths_stack.clear();
#define RESTORE_AFTER_WTP_WAIT \
DEV_ASSERT(ResourceLoader::load_nesting == 0); \
DEV_ASSERT(ResourceLoader::load_paths_stack.is_empty()); \
ResourceLoader::load_nesting = load_nesting_backup; \
ResourceLoader::load_paths_stack = load_paths_stack_backup; \
load_paths_stack_backup.clear();
// This should be robust enough to be called redundantly without issues.
void ResourceLoader::LoadToken::clear() {
WorkerThreadPool::TaskID task_to_await = 0;
@ -277,9 +257,11 @@ void ResourceLoader::LoadToken::clear() {
// If task is unused, await it here, locally, now the token data is consistent.
if (task_to_await) {
PREPARE_FOR_WTP_WAIT
int load_nesting_backup = load_nesting;
load_nesting = 0;
WorkerThreadPool::get_singleton()->wait_for_task_completion(task_to_await);
RESTORE_AFTER_WTP_WAIT
DEV_ASSERT(load_nesting == 0);
load_nesting = load_nesting_backup;
}
}
@ -290,17 +272,6 @@ ResourceLoader::LoadToken::~LoadToken() {
Ref<Resource> ResourceLoader::_load(const String &p_path, const String &p_original_path, const String &p_type_hint, ResourceFormatLoader::CacheMode p_cache_mode, Error *r_error, bool p_use_sub_threads, float *r_progress) {
const String &original_path = p_original_path.is_empty() ? p_path : p_original_path;
load_nesting++;
if (load_paths_stack.size()) {
MutexLock thread_load_lock(thread_load_mutex);
const String &parent_task_path = load_paths_stack.get(load_paths_stack.size() - 1);
HashMap<String, ThreadLoadTask>::Iterator E = thread_load_tasks.find(parent_task_path);
// Avoid double-tracking, for progress reporting, resources that boil down to a remapped path containing the real payload (e.g., imported resources).
bool is_remapped_load = original_path == parent_task_path;
if (E && !is_remapped_load) {
E->value.sub_tasks.insert(original_path);
}
}
load_paths_stack.push_back(original_path);
print_verbose(vformat("Loading resource: %s", p_path));
@ -318,7 +289,6 @@ Ref<Resource> ResourceLoader::_load(const String &p_path, const String &p_origin
}
}
load_paths_stack.resize(load_paths_stack.size() - 1);
res_ref_overrides.erase(load_nesting);
load_nesting--;
@ -378,7 +348,6 @@ void ResourceLoader::_run_load_task(void *p_userdata) {
// Thread-safe either if it's the current thread or a brand new one.
CallQueue *own_mq_override = nullptr;
if (load_nesting == 0) {
DEV_ASSERT(load_paths_stack.is_empty());
if (!Thread::is_main_thread()) {
// Let the caller thread use its own, for added flexibility. Provide one otherwise.
if (MessageQueue::get_singleton() == MessageQueue::get_main_singleton()) {
@ -497,7 +466,6 @@ void ResourceLoader::_run_load_task(void *p_userdata) {
MessageQueue::set_thread_singleton_override(nullptr);
memdelete(own_mq_override);
}
DEV_ASSERT(load_paths_stack.is_empty());
}
curr_load_task = curr_load_task_backup;
@ -627,6 +595,12 @@ Ref<ResourceLoader::LoadToken> ResourceLoader::_load_start(const String &p_path,
}
}
// Task hierarchy
if (curr_load_task) {
load_task.parent_task = curr_load_task;
curr_load_task->sub_tasks.insert(load_task.local_path);
}
// If we want to ignore cache, but there's another task loading it, we can't add this one to the map.
must_not_register = ignoring_cache && thread_load_tasks.has(local_path);
if (must_not_register) {
@ -868,9 +842,11 @@ Ref<Resource> ResourceLoader::_load_complete_inner(LoadToken &p_load_token, Erro
// Loading thread is in the worker pool.
p_thread_load_lock.temp_unlock();
PREPARE_FOR_WTP_WAIT
int load_nesting_backup = load_nesting;
load_nesting = 0;
Error wait_err = WorkerThreadPool::get_singleton()->wait_for_task_completion(load_task.task_id);
RESTORE_AFTER_WTP_WAIT
DEV_ASSERT(load_nesting == 0);
load_nesting = load_nesting_backup;
DEV_ASSERT(!wait_err || wait_err == ERR_BUSY);
if (wait_err == ERR_BUSY) {
@ -921,21 +897,21 @@ Ref<Resource> ResourceLoader::_load_complete_inner(LoadToken &p_load_token, Erro
load_task_ptr = &load_task;
}
p_thread_load_lock.temp_unlock();
Ref<Resource> resource = load_task_ptr->resource;
if (r_error) {
*r_error = load_task_ptr->error;
}
if (resource.is_valid()) {
if (curr_load_task) {
if (load_task_ptr->parent_task) {
// A task awaiting another => Let the awaiter accumulate the resource changed connections.
DEV_ASSERT(curr_load_task != load_task_ptr);
DEV_ASSERT(load_task_ptr->parent_task != load_task_ptr);
for (const ThreadLoadTask::ResourceChangedConnection &rcc : load_task_ptr->resource_changed_connections) {
curr_load_task->resource_changed_connections.push_back(rcc);
load_task_ptr->parent_task->resource_changed_connections.push_back(rcc);
}
} else {
p_thread_load_lock.temp_unlock();
// A leaf task being awaited => Propagate the resource changed connections.
if (Thread::is_main_thread()) {
// On the main thread it's safe to migrate the connections to the standard signal mechanism.
@ -959,11 +935,11 @@ Ref<Resource> ResourceLoader::_load_complete_inner(LoadToken &p_load_token, Erro
}
}
}
p_thread_load_lock.temp_relock();
}
}
p_thread_load_lock.temp_relock();
return resource;
}
@ -1585,7 +1561,6 @@ bool ResourceLoader::timestamp_on_load = false;
thread_local bool ResourceLoader::import_thread = false;
thread_local int ResourceLoader::load_nesting = 0;
thread_local Vector<String> ResourceLoader::load_paths_stack;
thread_local HashMap<int, HashMap<String, Ref<Resource>>> ResourceLoader::res_ref_overrides;
thread_local ResourceLoader::ThreadLoadTask *ResourceLoader::curr_load_task = nullptr;

2
core/io/resource_loader.h
View file

@ -188,6 +188,7 @@ private:
ResourceFormatLoader::CacheMode cache_mode = ResourceFormatLoader::CACHE_MODE_REUSE;
Error error = OK;
Ref<Resource> resource;
ThreadLoadTask *parent_task = nullptr;
HashSet<String> sub_tasks;
bool awaited : 1; // If it's in the pool, this helps not awaiting from more than one dependent thread.
@ -213,7 +214,6 @@ private:
static thread_local bool import_thread;
static thread_local int load_nesting;
static thread_local HashMap<int, HashMap<String, Ref<Resource>>> res_ref_overrides; // Outermost key is nesting level.
static thread_local Vector<String> load_paths_stack;
static thread_local ThreadLoadTask *curr_load_task;
static SafeBinaryMutex<BINARY_MUTEX_TAG> thread_load_mutex;