Godot Physics solver optimization #47846

pouleyKetchoupp · 2021-04-13T01:53:31Z

Several optimizations in the way solver islands are processed in both 2D and 3D physics:

Use LocalVector instead of linked list to avoid cache misses (with persistent storage based on worst case scenario)
Remove pairs when setup fails (no valid contact) to avoid unnecessary solving of non-colliding rigid bodies just to return immediately (3d only, that was already done in 2d)

Tested in:
https://github.com/godotengine/godot-demo-projects/blob/master/3d/physics_tests/tests/performance/test_perf_contacts.tscn

Performance improvement: 5-10% of the physics step time with 500 bodies.

Several optimizations in the way solver islands are processed in both 2D and 3D physics: - Use LocalVector instead of linked list to avoid cache misses (with persistent storage based on worst case scenario) - Remove pairs when setup fails (no valid contact) to avoid unnecessary solving of non-colliding rigid bodies just to return immediately

AndreaCatania

Code seems good, just few comments.

AndreaCatania · 2021-04-13T15:22:31Z

servers/physics_2d/step_2d_sw.cpp


-	for (const List<Pair<Constraint2DSW *, int>>::Element *E = p_body->get_constraint_list().front(); E; E = E->next()) {
+	// Faster with reversed iterations.


Interesting, if you can add why it's faster, it would be useful.

The truth is I'm not sure exactly why, but I could see a clear difference in all the tests I've run :D
I'll check a bit more if I can figure it out, otherwise I'll leave a note with a TODO to investigate that more later.

hmm that's even more interesting then. Most likely this improvement is just related to this specific project, so if you change project setup the forward may be faster. I think that this highlight a possible optimization: If you can understand what's the determining factor, you could consider to add a sorting mechanism (somehow) so you can apply this optimization to any project.

Yeah that makes sense. I agree it would be interesting to investigate that specifically with different use cases. For now I'm keeping a reversed order, because it's closer to what happened before this PR, due to how islands where fed in a reversed order (each new pair was set as the new root, and the previous root was set as the next pair).

AndreaCatania · 2021-04-13T15:31:19Z

servers/physics_2d/step_2d_sw.cpp

+			}
+			LocalVector<Constraint2DSW *> &constraint_island = constraint_islands[island_count - 1];
+			constraint_island.clear();
+			constraint_island.reserve(ISLAND_SIZE_RESERVE);


Clear doesn't de-allocate the memory, like reset does, probably you can avoid call reserve here.

I'm still using reserve here because of the case below where an island is created for just one area and this island would then be re-used here, to make sure it allocates for enough constraints.

Ok, make sense, another thing you could consider is pre-initialize those vectors. I'm saying this because the first time I saw such call there, it felt strange. However, it's not really important.

AndreaCatania · 2021-04-13T15:44:44Z

servers/physics_3d/step_3d_sw.h

-	void _solve_island(Constraint3DSW *p_island, int p_iterations, real_t p_delta);
-	void _check_suspend(Body3DSW *p_island, real_t p_delta);
+	LocalVector<LocalVector<Body3DSW *>> body_islands;
+	LocalVector<LocalVector<Constraint3DSW *>> constraint_islands;


The local vector doesn't deallocate the memory when you remove an object, so it works really well when you use it a lot.

When you copy a vector, you lost any pre-allocated memory: https://github.com/godotengine/godot/blob/master/core/templates/local_vector.h#L238-L251
By calling constraint_islands.resize(island_count);, you may lose the pre-allocated memory for the sub vectors. Each time you add a new object to the isle, since the vector is ""new"", it may need to re-allocate the memory. If you can avoid that, it would be better.

Have you considered to create a LocalVector pool, where you reuse the vectors instead? A naive solution may be use new:

- LocalVector<LocalVector<>> + LocalVector<LocalVector<> *>

Using a LocalVector pool is a great idea, I'll see if I can update to add that.

After testing this again, it doesn't look like any copy happens in the situation where constraint_islands.resize(island_count); has to re-allocate. It just does a memrealloc and the sub vectors are still keeping their capacity intact:

godot/core/templates/local_vector.h

Lines 141 to 150 in b4b7c97

if (unlikely(p_size > capacity)) {

if (capacity == 0) {

capacity = 1;

}

while (capacity < p_size) {

capacity <<= 1;

}

data = (T *)memrealloc(data, capacity * sizeof(T));

CRASH_COND_MSG(!data, "Out of memory");

}

That's correct for the vectors that are not removed, but in that function we have:

https://github.com/godotengine/godot/blob/master/core/templates/local_vector.h#L136

for (U i = p_size; i < count; i++) { data[i].~T(); }

and
https://github.com/godotengine/godot/blob/master/core/templates/local_vector.h#L153

for (U i = count; i < p_size; i++) { memnew_placement(&data[i], T); }

That take care to remove and allocate the new sub vectors, in this case.

This mean that if in frame 1 you have 10 islands, the following frame it drops to 5, then it reach 10 again basically you have to reallocate 5 vectors again. Which is a waste.
If, it doesn't take too much time, you could research a way to avoid that: maybe by never resizing the main vector, but using an external variable to track the "active" isles.

Ah yeah, it shouldn't be too hard to only grow the vector's size and keep track of the active islands as you suggested. I'll test in a scenario with many islands to see if it helps when I have a chance.

@AndreaCatania Just as a follow-up, I've checked this scenario, and I didn't remember but the logic is already that it only grows. Basically the main vector can only resize following this logic:

if (constraint_islands.size() < island_count) { constraint_islands.resize(island_count); }

Which will prevent any previous sub-vectors from being deallocated.

AndreaCatania

I think the code looks good, and my feedbacks are not blocking since could even be implemented in separate PRs or not at all.

Good job!

akien-mga · 2021-04-14T16:04:33Z

Thanks!

pouleyKetchoupp added enhancement topic:physics labels Apr 13, 2021

pouleyKetchoupp added this to the 4.0 milestone Apr 13, 2021

pouleyKetchoupp requested a review from a team April 13, 2021 01:53

pouleyKetchoupp force-pushed the solver-optimization branch from d16d353 to b65d6b5 Compare April 13, 2021 02:00

AndreaCatania reviewed Apr 13, 2021

View reviewed changes

AndreaCatania approved these changes Apr 14, 2021

View reviewed changes

akien-mga requested a review from reduz April 14, 2021 07:44

akien-mga merged commit 9e0f873 into godotengine:master Apr 14, 2021

pouleyKetchoupp deleted the solver-optimization branch April 14, 2021 17:27

madmiraal mentioned this pull request Apr 17, 2021

Change physics disabled booleans to enabled #44141

Closed

pouleyKetchoupp mentioned this pull request Apr 27, 2021

Godot Physics collisions and solver processed on threads #48221

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Godot Physics solver optimization #47846

Godot Physics solver optimization #47846

pouleyKetchoupp commented Apr 13, 2021

AndreaCatania left a comment

AndreaCatania Apr 13, 2021

pouleyKetchoupp Apr 13, 2021

AndreaCatania Apr 14, 2021

pouleyKetchoupp Apr 14, 2021

AndreaCatania Apr 13, 2021

pouleyKetchoupp Apr 13, 2021

AndreaCatania Apr 14, 2021

AndreaCatania Apr 13, 2021

pouleyKetchoupp Apr 13, 2021

pouleyKetchoupp Apr 13, 2021

AndreaCatania Apr 14, 2021

pouleyKetchoupp Apr 14, 2021

pouleyKetchoupp Apr 16, 2021

AndreaCatania left a comment

akien-mga commented Apr 14, 2021


		for (const List<Pair<Constraint2DSW , int>>::Element E = p_body->get_constraint_list().front(); E; E = E->next()) {
		// Faster with reversed iterations.

	if (unlikely(p_size > capacity)) {
	if (capacity == 0) {
	capacity = 1;
	}
	while (capacity < p_size) {
	capacity <<= 1;
	}
	data = (T )memrealloc(data, capacity sizeof(T));
	CRASH_COND_MSG(!data, "Out of memory");
	}

Godot Physics solver optimization #47846

Godot Physics solver optimization #47846

Conversation

pouleyKetchoupp commented Apr 13, 2021

AndreaCatania left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AndreaCatania left a comment

Choose a reason for hiding this comment

akien-mga commented Apr 14, 2021