[Web] Use actual PThread pool size for get_default_thread_pool_size()

[marcosc90]

fixes #104428

This resolves a freeze in WebAssembly builds when physics/3d/run_on_separate_thread=true and threading is enabled.

Previously, get_default_thread_pool_size() in OS_Web was hardcoded to return 1, which caused physics step group tasks to stall. The only available worker was occupied by this long-lived task:

godot/servers/physics_server_3d_wrap_mt.cpp

Lines 42 to 47 in 2303ce8

	void PhysicsServer3DWrapMT::_thread_loop() {
	while (!exit) {
	WorkerThreadPool::get_singleton()->yield();
	command_queue.flush_all();
	}
	}

As a result, the following task

godot/modules/godot_physics_3d/godot_step_3d.cpp

Line 347 in 2303ce8

WorkerThreadPool::GroupID group_task = WorkerThreadPool::get_singleton()->add_template_group_task(this, &GodotStep3D::_setup_constraint, nullptr, total_constraint_count, -1, true, SNAME("Physics3DConstraintSetup"));

would never execute, leading to a deadlock. This patch replaces the hardcoded value with a dynamic call to a JS hook that reflects the available pthread pool size at runtime.

bugsquad edit: fixes #104004

[adamscott]

For reference, here's the PR #54499 that introduced the static 1 value.

[adamscott]

I don't have an answer directly for https://github.com/godotengine/godot/pull/104458/files#r2008222268 though.

[marcosc90]

Pushed again to fix lint issues from suggested changes in the review.

[Faless]

Hi @marcosc90 , thank you for looking into this issue.

I will start with some context, but first, I didn't realize you could pass an expression as PTHREAD_POOL_SIZE, that is indeed much better than an hardcoded value. This also means we could expose it to the JS support code and make it configurable on export which is great 🎆 .

Now for some context...

---

Most browsers have a hard cap on threads, historically, that cap has been quite low.
When hardware concurrency is capped the value of navigator.hardwareConcurrency may be the cap (may, i.e. hardwareConcurrency may in fact be more than the cap).
This means we can't trust too much the value of navigator.hardwareConcurrency.

On the web, threads are spawned asynchronously, so code like:

thread.start(something)
// STUFF appends without returning the main loop
thread.join()

Will produce a deadlock.

This is why emscripten introduced the PTHREAD_POOL_SIZE option.

But this means that any thread spawned in addition to the PTHREAD_POOL_SIZE, will still cause deadlocks when occurring in the above scenario.

The issue we historically had with the godot thread pool was it filling the emscripten thread pool size, causing deadlocks in other parts of the engine (where the aformentioned start/join was used, sometimes by third party libraries).
This was made worse by the fact that we were using multiple WorkerThreadPools in the past (I don't know if that's the case anymore).

So, in general, we need to find a good trade-off, and that's not easy to find.

---

With this in mind, I suggest we may just want to make both values customizable via JS via platform/web/js/engine/config.js.

We can set those value to a reasonable defeault (8 for the emscripten pool, 4 for the godot pool).

We can make the value customizable when exporting via the editor by adding the values to the config in EditorExportPlatformWeb

We can also optionally add more logic (in a follow up PR) to EditorExportPlatformWeb to throw an error/warning if values are set too small for the current project settings.

We could also explore building with PTHREAD_POOL_SIZE_STRICT but that probably deserves its own discussion.

Let me know what you think.

[marcosc90]

Hi @Faless, that sounds good to me since currently on my custom build I'm using some custom values, so making the customizable would be great.

I'm not yet familiar with how all the settings work in godot, to confirm, do you mean adding those configs to the export template settings?

Just below Thread Support, adding something like:

• Emscripten Pool Size: 8
• Godot Pool Size: 4

[Faless]

I'm not yet familiar with how all the settings work in godot, to confirm, do you mean adding those configs to the export template settings?

Just below Thread Support, adding something like:

* Emscripten Pool Size: 8

* Godot Pool Size: 4

Yes, that's indeed what I've been thinking

[marcosc90]

I've been quite busy, I'll try to get this done during the weekend.

[marcosc90]

Just pushed some changes. I'm not really sure if this is exactly how configs should be passed, it is working correctly though.

I'm not good with public docs, so please feel free to add/suggest the description for both settings, I enabled "Allow edits"

[Faless]

Please apply the following patch so we don't pollute window.

patch.diff.txt

[marcosc90]

Much better, thanks @Faless . I was looking for a better way, but was still figuring it out how all the code glues together.

[clayjohn]

For the docs, you will need to run Godot from the engine root with the --doctool argument. It will automatically generate the correct docs bindings in the correct place so you don't trigger the CI error. See: https://docs.godotengine.org/en/latest/contributing/documentation/updating_the_class_reference.html#updating-class-reference-when-working-on-the-engine

I have made some rough suggestions, but this is outside my area of expertise, so the content may not be exactly accurate

[marcosc90]

Thanks @clayjohn

[clayjohn]

Looks good to me now. CC @Faless For final confirmation