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

Static and cached marker components #11864

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

NthTensor
Copy link
Contributor

@NthTensor NthTensor commented Feb 14, 2024

Objective

Bevy assumes all entities are entirely dynamic. This is a good default, because it encourages us to keep bevy scalable and efficient. Unfortunately this means we often waste some of our limited compute each frame re-exporting and re-processing data that hasn't actually changed.

There should be a way to mark entities that only need to be processed once without needing to rely on change detection (which is not efficient when the number of changes is small compared to the number of queried items).

Solution

Add two new table-backed marker components: Static and Cached.

Bevy assumes that while an entity is marked as Static, it's properties will not change in any way visible to the user. You can basically think of Static entities as being part of "the map" or "the game world". Bevy doesn't set Static on any entities by default; that is left to the end user. Static objects can be easily excluded from systems like propagate_transforms with Without<Static>.

After the first frame in which a Static entity is view-visible, it is marked as Cached. Render extraction systems can simply remove a entity_map.clear() and add a Without<Cached> query bound to retain extracted Static information. Cached is removed automatically from entities that are not Static, and can also be manually unset in the main world to cause a cache flush.

Static and Cached are tabled-backed. This means there is a reasonably large fixed cost to adding or removing these components. But this also means that entities with these components get kicked off into their own archetypes, and so have no query-iteration overhead (unlike Changed<T>). This can lead to large performance gains in several schedules (but particularly in render extraction) when the vast majority of rendered objects are Static, and Static is only rarely added or removed.


The current implementation is a prototype. I am still trying to identify other hot queries that could benefit from Without<Cached>.

I have profiled this against many_cubes and many_lights. Both show a significant potential for improvement over main, but I'd like to hold off on posting any of my results until this is more complete.

Here are some potential issues:

  • Cache invalidation has no GC and relies on visibility. In order to make query extraction efficient, entries in render_mesh_instances are never cleared. Cached controls when they are overwritten, and only visible entities are accessed. The memory cost is minimal compared to the time cost of clearing out-of-view cached items.
  • Currently there is no way to disable or enable Static and Cached stuff. I plan to add this but it may require tweaking propagate_transforms.
  • propagate_transforms needs to visit every static node with children. Combining this approach with Parallelized transform propagation #11760 will significantly improve this issue.

@ItsDoot ItsDoot added A-Rendering Drawing game state to the screen C-Performance A change motivated by improving speed, memory usage or compile times labels Feb 14, 2024
Copy link
Member

@BD103 BD103 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks really cool!

I am concerned about users overusing Static, though. Are diagnostics (logging) for mutating a Static entity viable?

Comment on lines +172 to +175
/// Cached is a special marker component closely related to [`Static`].
///
/// Systems in the [`PostUpdate`] schedule that query for cached entities should run after [`refresh_cached`]
/// to avoid a possible one frame lag when [`Static`] is removed.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should there be a warning against manually inserting Cached into an entity? From your description, it seems that only render extraction should be adding it.

@james7132 james7132 self-requested a review February 19, 2024 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Performance A change motivated by improving speed, memory usage or compile times
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants