Rationalize busy waits

[RandomShaper]

Using the yield primitive lets the OS know our intent better than a microwait. This may improve energy efficiency and/or performance.

---

2024-10-31:

@lyuma's comment about the specific cases made me realize we don't even need to discuss how to implement a yield mechanism, since there's no real need for that.

So, I've updated the PR addressing each case of delay_usec(1) in loops:

• EditorFileSystem: It was used to poll for the status of the batched import. I've overhauled it so a semaphore controls it instead.
• EditogLog: I've added a non-polling sync mechanism to RichTextLabel. (By the way, RichTextLabel threading warrants further threading improvements, but that's out of the scope of this PR.)
• EditorNode::immediate_confirmation_dialog(), EditorFileSystemImportFormatSupportQueryBlend: They are already using main loop iterations, so pacing is implicit.
• HTTPRequest: In threaded mode, it uses blocking sockets, so the loop isn't really going to run freely. Therefore, I've just removed the relevant call. (Is blocking not supported somewhere?)

[mrsaturnsan]

Yield has really poor performance on macOS. The function gets compiled down to sched_yield, which can yield the process to other processes running on the system. Lack of SIMT also means you can't yield work to other threads.

[RandomShaper]

I've added a special implementation for Apple platforms.

[mrsaturnsan]

Thanks for looking into making changes.

Unfortunately, pthread_yield_np also messed with thread priorities :(

There's not really a good option, but you're better off spinning and burning cycles rather than yielding. Ideally you could use os_unfair_lock to let the scheduler know what thread another thread is waiting on.

[lyuma]

EditorNode::immediate_confirmation_dialog
editor/editor_log.cpp
EditorFileSystemImportFormatSupportQueryBlend
HTTPRequest::_thread_func
static_test_daemon

I don't think busy waiting is needed or even appropriate in any of these cases (except maybe editor_log if it's invoked by multiple threads).

May I ask why this code isn't using a semaphore or condition variable or similar mechanism?

[RandomShaper]

@mrsaturnsan, good point. I was misinformed that pthread_yield_np() was better on MacOS.

@lyuma, you'be brought the greatest sanity here. Please check the updated description.

[clayjohn]

The code and the approach look good to me, but I am not an expert in this area so TIWAGOS

[Calinou]

cc @adamscott regarding HTTPRequest in threaded mode. Will this be an issue on the web platform?

[adamscott]

I got this error warning while running a threaded HTTPRequest on the Web platform.

WARNING: HTTPClientWeb polled multiple times in one frame, but request cannot progress more than once per frame on the Web platform.
overrideMethod @ hook.js:608
onPrintError @ godot.editor.js:15281
put_char @ godot.editor.js:1669
write @ godot.editor.js:1618
write @ godot.editor.js:3476
doWritev @ godot.editor.js:9118
_fd_write @ godot.editor.js:9136
__emscripten_receive_on_main_thread_js @ godot.editor.js:6214
$run_js_func @ godot.web.editor.dev.wasm32.wasm-9a60df2e:0x3748754
$call_then_finish_task @ godot.web.editor.dev.wasm32.wasm-9a60df2e:0x3748655
$call_with_ctx @ godot.web.editor.dev.wasm32.wasm-9a60df2e:0x3748588
$em_task_queue_execute @ godot.web.editor.dev.wasm32.wasm-9a60df2e:0x3747d5c
$receive_notification @ godot.web.editor.dev.wasm32.wasm-9a60df2e:0x374804c
$em_task_queue_execute @ godot.web.editor.dev.wasm32.wasm-9a60df2e:0x3747d5c
$_emscripten_check_mailbox @ godot.web.editor.dev.wasm32.wasm-9a60df2e:0x3750d03
callUserCallback @ godot.editor.js:5748
checkMailbox @ godot.editor.js:5780
Promise.then
__emscripten_thread_mailbox_await @ godot.editor.js:5762
checkMailbox @ godot.editor.js:5779
Promise.then
__emscripten_thread_mailbox_await @ godot.editor.js:5762
checkMailbox @ godot.editor.js:5779
Promise.then
__emscripten_thread_mailbox_await @ godot.editor.js:5762
checkMailbox @ godot.editor.js:5779
...
godot.editor.js:15281    at: poll (platform/web/http_client_web.cpp:240)

Either:

• we shush the warning on the Web (kinda bad)
• we check the capabilities of the platform to poll http requests before (relatively clean)
• we just add a #ifdef WEB_ENABLED macro to force waiting (ugly as heck)

cc. @Faless

[RandomShaper]

Wow, that check comes from seven years ago!: #16781.

Back them, HTTP requests on the web were backed by XMLHttpRequest instead of fetch(). I wonder if the limitation still holds. In any case, I wonder what's the harm of multiple pollings on the same frame. Was it to ensure HTTPRequest is implemented correctly?

[RandomShaper]

CC @Faless

[adamscott]

I'm gonna check this out.

[adamscott]

cc @adamscott regarding HTTPRequest in threaded mode. Will this be an issue on the web platform?

Just realized that HTTPRequest in threaded mode isn't compatible with the Web platform at all (we cannot set_blocking_mode(true), as threaded mode does). So this is a non-issue (?).

[RandomShaper]

Since we're too slow in coming up with a consensus on the HTTPRequest change, I'm leaving it untouched so this PR can be merged. HTTPRequest can be discussed and changed separately.

[akien-mga]

@lyuma Please take another look if you can, so we can get this merged.

[akien-mga]

Thanks!