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

Speed up ColliderTransform propagation and extract collider hierarchy logic into ColliderHierarchyPlugin #377

Merged
merged 3 commits into from
Jun 21, 2024

Conversation

Jondolf
Copy link
Owner

@Jondolf Jondolf commented Jun 19, 2024

Objective

Note: This is for the bevy-0.14 branch, because it uses new observer functionality (although it's not strictly necessary). It will be merged to main for the upcoming release.

Fixes #356.

To handle transforms in collider hierarchies properly, the engine must currently propagate a ColliderTransform component down the hierarchy several times per frame. This is made even worse by the fact that this propagation is done at every substep, not just once or twice per frame.

This wouldn't be too bad if it only affected physics entities, but the current system traverses through all entities that have a Transform. As reported in #356, this leads to absolutely awful performance for scenes with a large number of non-physics entities. This means that the engine scales very poorly with entity count, which is unacceptable.

Solution

Speed up propagation with ColliderAncestor

When colliders are added as children, automatically mark all ancestors with the ColliderAncestor component. Similarly, clean up the markers when a collider is removed.

ColliderTransform is only propagated down trees with ColliderAncestor entities. This way, we can essentially skip all unnecessary propagation.

In a test scene with one root entity, 100,000 child entities, and 12 substeps, I was previously getting ~22 fps even though the entities only had a SpatialBundle and no physics components. With the changes in this PR, I now get ~200 fps. Adding colliders to individual entities doesn't impact performance in any meaningful way, as the propagation is only done for those entities.

Note that this propagation will be even less of an issue in the upcoming Avian release, because there it doesn't even need to be run in the substepping loop.

The ColliderAncestor logic has a pretty comprehensive unit test for adding and removing colliders in a hierarchy, but I haven't verified yet if simply changing the Parent without moving the Collider works as expected. Adding more tests and potentially fixing these more niche cases can be done in a follow-up.

Add ColliderHierarchyPlugin

The ColliderBackendPlugin was getting big, so I decided to extract the logic related to hierarchies and ColliderTransform into a new ColliderHierarchyPlugin.

Ideally, you could use the plugins fully independently and for example completely abandon hierarchies if you wanted to, but currently, they are still lightly coupled. Eliminating this coupling is a task for a follow-up.


Migration Guide

Hierarchy and transform logic for colliders has been extracted from the ColliderBackendPlugin into a new ColliderHierarchyPlugin, which by default is included in the PhysicsPlugins plugin group.

If you are adding plugins manually, make sure you have both if you want that functionality.

@Jondolf Jondolf added C-Performance Improvements or questions related to performance C-Breaking-Change This change removes or changes behavior or APIs, requiring users to adapt labels Jun 19, 2024
@Jondolf
Copy link
Owner Author

Jondolf commented Jun 19, 2024

Hmm, I might be able to use a similar approach for the "normal" Bevy transform propagation that we run in like four other places... I'll test this in a follow-up

@Jondolf
Copy link
Owner Author

Jondolf commented Jun 21, 2024

I have the follow-up implemented, and it reworks this further. I'll merge this first though, and open the follow-up right after

@Jondolf Jondolf merged commit 1570143 into bevy-0.14 Jun 21, 2024
4 checks passed
@Jondolf Jondolf deleted the reduce-unnecessary-propagation branch June 21, 2024 19:09
Jondolf added a commit that referenced this pull request Jun 21, 2024
…380)

# Objective

#377 refactored collider hierarchy logic and extracted it into a `ColliderHierarchyPlugin<C: ScalableCollider>`, along with significantly speeding up `ColliderTransform` propagation using a `ColliderAncestor` marker component to ignore trees that don't have colliders.

However, a similar marker component approach can *also* be used to speed up the many copies of transform propagation that the engine has. By limiting the propagation runs to physics entities, we could drastically reduce the overhead for scenes with lots of non-physics entities, further addressing #356.

## Solution

- Extract the ancestor marker logic from `ColliderHierarchyPlugin` into an `AncestorMarkerPlugin<C: Component>` that adds an `AncestorMarker<C>` component for all ancestors of entities with the given component `C`. The logic was also refactored to be more robust and fix some edge cases and fully support hierarchy changes, along with adding more tests.
- Use this plugin in `SyncPlugin` to mark rigid body ancestors.
- Define custom transform propagation systems that *only* traverse trees that have physics entities (rigid bodies or colliders). See the comments in `sync.rs` above the propagation systems to see how this works.
- To support custom collision backends without having to add generics to all the propagation systems, and to avoid needing to unnecessarily duplicate systems, the `ColliderBackendPlugin` now adds a general, automatically managed `ColliderMarker` component for all colliders, which allows filtering collider entities without knowing the collider type. This lets us get rid of the generics on `ColliderHierarchyPlugin`!
- I also changed some scheduling and system sets in the `PrepareSet` to account for some changes and to be more logical.

## Results

Test scene: 12 substeps, 1 root entity, 100,000 child entities. All entities have just a `SpatialBundle`, and one child entity also has a `Collider`.

- Before [#377](#377): ~22 FPS
- After [#377](#377): ~200 FPS
- After this PR: ~490 FPS

Of course, this scene is not very representative of an actual game scene, but it does show just how much of an impact the transform propagation was having. A *lot* of games (probably most of them) do have many more non-physics entities than physics entities, and the overhead added by the marker components and new checks should be very minimal in comparison.

---

## Migration Guide

Some `PrepareSet` system sets have changed order.

Before:

1. `PreInit`
2. `PropagateTransforms`
3. `InitRigidBodies`
4. `InitMassProperties`
5. `InitColliders`
6. `InitTransforms`
7. `Finalize`

After:

1. `PreInit`
2. `InitRigidBodies`
3. `InitColliders`
4. `PropagateTransforms`
5. `InitMassProperties`
6. `InitTransforms`
7. `Finalize`

Additionally, the `ColliderHierarchyPlugin` added in #377 no longer needs generics. The plugin is new anyway however, so this isn't really a breaking change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Breaking-Change This change removes or changes behavior or APIs, requiring users to adapt C-Performance Improvements or questions related to performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant