Optimize Query::get_component #2965
Conversation
alice-i-cecile
added
A-ECS
|
Do we have any existing benchmarks that would be appropriate for this? |
Co-authored-by: Boxy <supbscripter@gmail.com>
crates/bevy_ecs/src/query/filter.rs
Outdated
| @@ -503,6 +518,10 @@ macro_rules! impl_tick_filter { | |||
| fn matches_table(&self, table: &Table) -> bool { | |||
| table.has_column(self.component_id) | |||
| } | |||
|
|
|||
| fn get_id<Comp: 'static>(&self) -> Option<(ComponentId, RWAccess)> { | |||
There was a problem hiding this comment.
Can we make these functions const?
I'm counting this as an approval. |
alice-i-cecile
added
the
S-Ready-For-Final-Review
|
I'd like to see some benchmark results before merging this, given that performance is the motivator behind the change, this increases code complexity, and bevy_ecs perf is notoriously fickle. |
|
@TheRawMeatball, when you get a chance can you rebase this? We have ECS benchmarks now; I can give running the comparison a shot. |
|
I'll rebase this and run benches later today, though I don't think we have any benchmarks for this function. |
|
Fair enough. I'll see about adding some in a separate PR. |
# Objective Add a benchmark to measure the performance of get_component, particularly for cases involving random access. Enables #2965
# Objective Add a benchmark to measure the performance of get_component, particularly for cases involving random access. Enables #2965
|
@TheRawMeatball merge conflicts and failing CI for you <3 We should also double-check those benchmarks, if only for the release notes hype. |
alice-i-cecile
added
S-Needs-Benchmarking
# Objective Add a benchmark to measure the performance of get_component, particularly for cases involving random access. Enables bevyengine#2965
alice-i-cecile
added
the
S-Adopt-Me
# Objective Add a benchmark to measure the performance of get_component, particularly for cases involving random access. Enables bevyengine#2965
|
Since get_component has changed a lot since this PR was opened, is there a reason to keep it open? |
Right now Query::get_component creates a new EntityRef per get call, and then uses get_unchecked methods to get the data, causing more lookups and branches than necessary. This PR optimizes get_component by taking advantage of data already cached on Query s and taking advantage of derive component to const-ify certain branches.