Use EntityIndexSet in Children

[SkiFire13]

Objective

• Takes over Use EntityIndexSet in Children #23296
• Fixes UB found in EntityWorldMut::replace_children in multi-threaded with pure safe code #25349

From the original PR:

Objective

• Fix Extremely slow entity despawning #23295

• Alternative to Optimize entity removal by scanning from the end #23297

Solution

• Use an index set for fast lookups, while maintaining insertion order and index lookups used by existing features.

[Victoronz]

Note that this will simply shift performance characteristics of the underlying type, not improve them across the board.
For our use cases the change in performance trade-offs might end up being mostly a plus, but without performance benchmarks it is somewhat of an unknown.

[github-actions]

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-25811

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

[alice-i-cecile]

@SkiFire13 @Victoronz can you resolve comment threads as you work through the issues? I want to review this but it's hard for me to follow right now :)

[SkiFire13]

@alice-i-cecile I resolved most of them, I only left one open waiting for the reply from @Victoronz

[alice-i-cecile]

I want to see benchmarks of this before it merges: this is a major change in hot loop, at least partially motivated by improving performance.

[SkiFire13]

@alice-i-cecile there should be a tentive benchmark in the original PR.

Unfortunately I will be on vacation without my pc for the next couple of days so I won't be able to make benchmarks for this. However consider that:

• this is generally doing more work (checking that entities aren't in Children when before we didn't), so it can be slower
• on the other hand when we do these checks it can be a little faster in cases where we have a lot of entities
• the alternative for soundness would be doing this kind of children deduplication in the transform propagation every single frame for every children that might have changed. I would assume this to also be slower than the current situation

[alice-i-cecile]

Yep, no worries. I'll run some benchmarks on Sunday myself, and see what we can do to improve things in general :) I agree that this is important work that shouldn't be dropped.

[SkiFire13]

Apparently this approach may be flawed due to indexmap::set::MutableValues being safe 🫠

Should we just write our own IndexSet<Entity>?