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

Don't panic in the ECS where we know a fetch is valid #12149

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

james7132
Copy link
Member

@james7132 james7132 commented Feb 27, 2024

Objective

Partly addresses #12107. We're using panicking Index/IndexMut fetches on Tables and Archetypes in potentially hot operations in the ECS. This creates a lot of extra codegen when we're we know the corresponding Table or Archetype exists.

Solution

  • Remove the Index and IndexMut implementations on Tables and Archeytpes, and replace them with get/get_mut where appropriate.
  • Create and use Tables::empty and Tables::empty_mut as the empty table is always guaranteed to exist.
  • Use DebugCheckedUnwrap in areas where we we know the table or archetype exist.
  • Create a UnsafeVecExtensions trait that provides a Vec::swap_remove_unchecked that is the same as swap_remove but without the safety check. Use it where we are using swap_remove like removing component ticks.
  • Do the same with a number of areas where we're currently using unwrap.

TODO: Clean up the invariants on unsafe functions and their callsites.

Performance

This PR primarily impacts Entity{Ref/Mut/WorldMut}, command application, and direct World interaction.

TODO: Benchmark.


Changelog

TODO

Migration Guide

TODO

@james7132 james7132 added A-ECS Entities, components, systems, and events C-Performance A change motivated by improving speed, memory usage or compile times labels Feb 27, 2024
@james7132 james7132 changed the title Dont panic Don't panic in the ECS where we know a fetch is valid Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events 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.

1 participant