NavMap::sync(): only acquire write lock when writes are needed

[cgodley]

Fixes #96820

• Recommend viewing the diff with whitespace ignored (git diff -b). A large portion of the diff is indentation that didn't seem easily avoidable
• Not 100% sure it's the best strategy or that I found 100% of the possible dirty flags. It's my first time contributing to Godot.
• Works great in my local testing
• Style: I'm not sure if is_dirty() is the best name. I used this name because check_dirty() elsewhere has side effects, but is_dirty() does not have any side effects. Feel free to change it.

[smix8]

Thanks for looking into improving this. I agree the lock should only be held when needed.

I am just not sure if the changes here are the right fix. It just pushes the performance problems to situations where the map changes which are even more a problem for the average project. Doing a double-loop over all objects to check for dirty affects all projects performance by a small margin while it only fixes a very niche and extreme performance issue here.

The issue is that on a map with 35.000+ polygons, which is way, way too much, every single pathfinding thread blocks the map for 0.03 or more with the read lock. That is enough to make the map stutter on every tiny change by blocking the main thread because it needs to wait forever until a running read thread has finished before it can aquire the write lock. On a reasonable sized map the lock is held for such a small time window that the lock problem is marginal.

The likely real solution would be to move the pathfinding map to a static iteration that threads can run on without affecting the new iteration building. That unfortunately requires far bigger changes as no navobject is currently prepared for async updates. It is a big pointer and polygon legacy spaghetti.

[cgodley]

Thank you for the fast reply. It makes sense we don't want to worsen anything related to map changes if that is an area people already have issues with.

Agree 35k polygons is high, my intention was to exaggerate the stutter in the MRP to make it easily visible during testing.

Closing the PR since I agree there are better ways to do this.