Speed up NavigationServer3D synchronizations #112908
Open
+381
−366
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Speeds up
NavigationServer3Dsynchronizations by removing unnecessary physics step and command queue delays for navmesh related updates.resolves #112652
What changed?
process()as the physics step adds an extra long delay to updates and is only required for avoidance stepping.xyz_force_update()functions for regions, links and maps to the server api (in case of maps revives the old depr). These functions cause an immediate main thread update and synchronization.Most syncs now happen on the process() callback.
Since the NavigationServer process runs just BEFORE the RenderingServer sync but AFTER all the script related inputs it is now possible for users to set things in script and have it sync and update in the same frame later. E.g. use the callback of the navmesh baking or server map changed signal to finish a query within the same main loop.
This will obviously only work while everything is on the main thread. With threads and async iterations there will still be a delay due to thread task start and join so expect at least another frame depending on how heavy the task is.
The new forced update functions allow to override this temporarily and force an immediate main thread sync.
Forced Updates
By using these new forced update functions it is possible to update region navmesh and do an immediate path query in the same frame.
These functions come with a heavy performance cost. Only use when you know what you are doing.
Regarding the forced updates, regions and links always need to be synced first before the map is synced. Since links require the polygons from region to connect the regions should update first.
The maps needs updated region and link data for a full navmesh and pathfinding update. E.g. just forcing an update on the map will do nothing if the region update for the navmesh is still pending.
So recommended order for a full main thread update would be:
Why changed?
Users were unhappy with the unpredictable NavigationServer updates. Especially when they needed updated navmesh data to do queries in setup functions where waiting for x-amount of frames was suboptimal.
This delay was primarily caused by the physics process being run unreliable based on time passed so when users would change something they had to wait for the next physics process before the server did any sync at all.
With the newer thread async iterations, due to the thread start and join breaking out of the main loop, at least one more sync point was easily missed, so users had to wait at least double. With a very slow physics step tick rate this would mean that users often had to wait x-amount of frames.
On top, since regions and maps do updates in parallel on different threads it is also not predictable when each thread finishes a task. So sometimes a map would update and finish first, emit its changed signal, but not have include the expected region update because the region task was heavy and still running.
The Godot NavigationServer, since its origin, had a command queue for basically all setters and a sync point with the physics step.
This was because avoidance is a stepped process in sync with physics, so it needed to flush and run its updates with every physics step. Because this command queue was used for everything it frequently caused update commands for e.g. navmesh changes, to be stuck for a very long time in the queue until the next physics process step.
The command queue is no longer really required. It never really mattered for the navmesh related synchronization of regions, links and maps. The main stability problems back were all the thread unsafe resources, not the server setters. All the important data by now is stored on the map iteration or region iteration and the resources are made thread-safe. The actual map or region are only read very briefly on sync and under mutex/rwlock so it does not really matter if users try to change their properties between.
For the avoidance ever since there is a separate rvo simulation with their own agent and obstacles it also does not matter that much anymore. On each step the agent gets updated properties before the rvo simulation runs its thread job. If users change properties before and after that it does not affect the simulation.