Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
cgodley opened this issue Sep 10, 2024 · 2 comments · Fixed by #100497

Comments

@cgodley
Copy link

cgodley commented Sep 10, 2024

Tested versions

  • Reproducible in v4.3.stable
  • write_lock was first introduced in commit 4cc8748
  • Issue is resolved if I compile engine with write_lock removed

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
Copy link
Author

cgodley commented Sep 11, 2024

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

@firescaleVR
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants