Add EntityRef/EntityMut based options to ReflectComponent

[james7132]

Objective

ReflectComponent::get and friends internally use World::get and similar APIs which spend time looking up the EntityLocation on each call. This can add significant overhead when fetching multiple components from the same entity, such as when animating multiple components via reflection.

Solution

Extend the functions of ReflectComponent to fetch references to components using EntityRef and EntityMut instead of a World/Entity. This allows users to fetch a single EnttiyRef/EntityMut and query for any or all of it's components without refetching the entity's location for each one.

Ideally, a World-specific version of ReflectComponent would cache the ComponentId so we could cut out the TypeId -> ComponentId lookup as well. However this requires a &mut World on ReflectComponent initialization and would only be valid for that specific World.

---

Changelog

Added: ReflectComponent::reflect_ref
Added: ReflectComponent::reflect_mut_ref
Added: ReflectComponent::reflect_unchecked_mut_ref

[jakobhellermann]

Have you profiled the alternative of using

let reflect_component = type_register.data<ReflectComponent>();
reflect_component.get(entity)

vs

let reflect_from_ptr = type_registry.data::<ReflectFromPtr>();

let ptr = entity_ref.get_by_id(component_id);
reflect_from_ptr.as_reflect(ptr)

?

I'd like to get rid of ReflectComponent/ReflectResource in favor of this.

[james7132]

I have not, though my gut feeling like it's going to be slower due to not having any of the compile time optimizations we've been adding to the generic path. The use of *_by_id functions also puts unsafe in the userspace code more readily, so I'm hesitant to suggest it's usage.

[jakobhellermann]

The use of *_by_id functions also puts unsafe in the userspace code more readily, so I'm hesitant to suggest it's usage.

This could be fixed by having wrappers at a safe boundary, like (TypeId, &TypeRegistry) -> &dyn Reflect

[james7132]

This could be fixed by having wrappers at a safe boundary, like (TypeId, &TypeRegistry) -> &dyn Reflect

This would still lose the EntityRef::get<T> vs EntityRef::get_by_id difference where it uses the compile time constant to optimize out a branch. This would have a negative impact on "animate anything" which will be highly reliant on this API to be performant.

[jakobhellermann]

Why did you keep the reflect_mut function pointer instead of implementing it in terms of the underlying EntityMut one?

[james7132]

Why did you keep the reflect_mut function pointer instead of implementing it in terms of the underlying EntityMut one?

The primary one is that the lifetime of a EntityMut and it's fetched borrows is held in terms of where the it's created from. Creating an EntityMut inside that function leaks the lifetime to the outside, so it doesn't compile.

[james7132]

Closing this as it's already been implemented by #7206.