godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
90.83k stars 21.14k forks source link

NavMap::sync `write_lock` blocks main loop when no write is necessary, causes unnecessary stuttering #96820

Open cgodley opened 1 month ago

cgodley commented 1 month ago

Tested versions

System information

Godot v4.3.stable - macOS 14.6.1 - Vulkan (Forward+) - integrated Apple M1 Pro - Apple M1 Pro (8 Threads)

Issue description

My game frequently calls NavigationServer3D.map_get_path(). I use separate threads (WorkerThreadPool.add_task()) to avoid stuttering caused by blocking the main loop.

Beginning with git hash 4cc8748, my game frequently has significant stutters. This is because NavMap::sync() (in the main loop) acquires write_lock every single iteration, and so the main loop cannot proceed until all threads are done with read_lock. This seems unnecessarily slow. Instead, perhaps we could detect when a write will be required, and only then acquire the write_lock. Or perhaps someone has a better suggestion.

I've tried using simpler collision shapes, tagging specific areas as "requiring navigation" vs not requiring it, performing less frequent navigation, etc. but it has not been sufficient to fully resolve the stuttering. I suspect this issue has at least some effect on the frame time of all games using threaded navigation.

I think I have confirmed the cause by testing the MRP (attached) before/after recompiling Godot with write_lock removed. Before the change there is obvious stuttering. After removing write_lock, everything runs smoothly.

Steps to reproduce

Run the MRP, you will see a smoothly animating sphere for a few seconds. Then it will schedule some expensive navigation tasks in a low priority WorkerThreadPool. On affected versions of Godot, you will observe the framerate drop significantly while navigation is running despite the use of WorkerThreadPool.

Minimal reproduction project (MRP)

navmap_stutter.zip

cgodley commented 1 month ago

Will open a pull request soon to demonstrate one way to fix it

firescaleVR commented 1 month ago

This is terrible. Polling any unreachable place in the navmesh causes it to stall for 300-500milliseconds, and now thanks to 4cc8748 the main loop freezes making any future versions of godot unuseable for me.