optimize batch_and_prepare_render_phase

[re0312]

Objective

• since Automatic batching/instancing of draw commands #9685 ,bevy introduce automatic batching of draw commands,

• batch_and_prepare_render_phase take the responsibility for batching phaseItem,

• GetBatchData trait is used for indentify each phaseitem how to batch. it defines a associated type Data used for Query to fetch data from world.

• however,the impl of GetBatchData in bevy always set type Data=Entity then we acually get following code
  let entity:Entity =query.get(item.entity()) that cause unnecessary overhead .

Solution

• remove associated type Data and Filter from GetBatchData ,
• change the type of the query_item parameter in get_batch_data from Self::Data to Entity.
• batch_and_prepare_render_phase no longer takes a query using F::Data, F::Filter
• get_batch_data now returns Option<(Self::BufferData, Option<Self::CompareData>)>

---

Performance

based in main merged with #11290
Window 11 ,Intel 13400kf, NV 4070Ti

frame time from 3.34ms to 3 ms, ~ 10%

batch_and_prepare_render_phase from 800us ~ 400 us

Migration Guide

trait GetBatchData no longer hold associated type Data and Filter
get_batch_data query_item type from Self::Data to Entity and return Option<(Self::BufferData, Option<Self::CompareData>)>
batch_and_prepare_render_phase should not have a query

[re0312]

@matiqo15 Perhaps the label 'C-Performance' would be more suitable for this topic.

[james7132]

If it's not being used, it's best to remove it entirely, as well as the associated type. Unless @superdump or @robtfm have any other objections to dropping this functionality from the trait.

[re0312]

yeah, I also intend to remove it , IMO,we should not do any related-query during batch&prepare phase,not only does this incur additional overhead, but it also hinders the potential for parallelization in the future. once Bevy has a more powerful parallel infrastructure, we can easily parallelize this phase.

[robtfm]

ok so currently F::Filter gets checked by query.get() and is effectively checked again by mesh_instances.get(), so the cost of the system reducing by 50% when all items match makes sense (and there should still be a smaller benefit if not all phase items match the query, tending to equal as the number of phase item types tends to infinity).

the query was because originally we stored data on the entity instead of in the non-ecs-based RenderMeshInstances. i'm not sure we are fully committed to that approach, but the change wouldn't be hard to revert if we need to and the impact is worthwhile. any third party users can follow the same pattern as well. let's nuke the with_query variant.

[re0312]

IIRC,F::Filter or mesh_instances.get() overhead is relatively small , the bottleneck lies in performing query.get(entity) for each entity,it needs to introduce extra init_fetch and set_archetype/table per entity, therefore, when none of the phase items match, the performance gain is still quite significant.

[mockersf]

Could you update the "solution" part of the description with your latest changes?

[re0312]

Could you update the "solution" part of the description with your latest changes?

done!