Skip to content

Conversation

james7132
Copy link
Member

@james7132 james7132 commented Aug 12, 2025

Objective

Use less memory overhead when storing resources and sparse set components, maintain less code. Fixes #14824. Supersedes #18245.

Solution

Use BlobArray and ThinColumn to replace BlobVec and Column in ResourceData and ComponentSparseSet. This both uses less memory overhead for each resource and sparse set component we use, and also allows us to avoid keeping two nearly identical implementations of BlobVec/BlobArray and ThinColumn/Column around.

Testing

Tested this against existing unit tests and miri.

Co-Authored By: [email protected]

@james7132 james7132 added A-ECS Entities, components, systems, and events C-Performance A change motivated by improving speed, memory usage or compile times C-Code-Quality A section of code that is hard to understand or change S-Needs-Benchmarking This set of changes needs performance benchmarking to double-check that they help D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes D-Unsafe Touches with unsafe code in some way labels Aug 12, 2025
@james7132 james7132 added this to the 0.18 milestone Aug 12, 2025
@james7132 james7132 changed the title Use BlobArray and ThinColumn in ResoruceData and ComponentSparseSet Use BlobArray and ThinColumn in ResourceData and ComponentSparseSet Aug 13, 2025
@james7132 james7132 force-pushed the thin-resources-and-sparse-sets branch from f533905 to 560ca3f Compare August 13, 2025 06:46
@james7132
Copy link
Member Author

I ran a set of benchmarks and it doesn't seem to have a tangible improvement to entity insertion or deletion, and we don't have any benchmarks for resource insertion, retrieval, or removal.

The compiler likely was optimizing away the extra branch and panic in the swap_remove. The assembly output seems to have minimally changed.

This otherwise still is a code quality improvement and reduces the overall amount of unsafe code we're maintaining.

@james7132 james7132 removed the C-Performance A change motivated by improving speed, memory usage or compile times label Aug 13, 2025
@james7132 james7132 marked this pull request as ready for review August 13, 2025 19:04
@james7132 james7132 added S-Needs-Review Needs reviewer attention (from anyone!) to move forward and removed S-Needs-Benchmarking This set of changes needs performance benchmarking to double-check that they help labels Aug 13, 2025
@james7132 james7132 requested review from chescock and ecoskey August 18, 2025 23:38
Copy link
Contributor

@chescock chescock left a comment

Choose a reason for hiding this comment

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

Yay, lots of unsafe code removed! The change itself looks good, but I left some nitpicking about comments on the parts that took me a while to understand.

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-Code-Quality A section of code that is hard to understand or change D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes D-Unsafe Touches with unsafe code in some way S-Needs-Review Needs reviewer attention (from anyone!) to move forward
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use ThinColumn for both resources and sparse sets
2 participants