[Merged by Bors] - Change Detection Benchmarks#4972
[Merged by Bors] - Change Detection Benchmarks#4972ThierryBerger wants to merge 8 commits intobevyengine:mainfrom
Conversation
ThierryBerger
added
A-ECS
ThierryBerger
force-pushed
the
4883-benchmark-change
branch
from
3cf2d37 to
a4c6a83
Compare
DJMcNab
left a comment
DJMcNab
left a comment
There was a problem hiding this comment.
This is a good start, thanks.
No significant complaints from me, but a few things which need addressing
| #[component(storage = "SparseSet")] | ||
| struct Sparse(f32); | ||
|
|
||
| const RANGE: std::ops::Range<u32> = 5..7; |
There was a problem hiding this comment.
This is not a good constant name. However, I see that it's consistent with world_get, so it's probably fine?
There was a problem hiding this comment.
Yeah I'm coming from there, I agree on the criticism though.
ThierryBerger
force-pushed
the
4883-benchmark-change
branch
from
1a4f878 to
d0d09d9
Compare
ThierryBerger
added
the
S-Needs-Benchmarking
ThierryBerger
removed
the
S-Needs-Benchmarking
alice-i-cecile
left a comment
alice-i-cecile
left a comment
There was a problem hiding this comment.
I'm happy with this; it's not perfect, but it's consistent with its siblings.
|
|
||
| impl BenchModify for Table { | ||
| fn bench_modify(&mut self) { | ||
| self.0 += 1f32; |
There was a problem hiding this comment.
This doesn't change the situation since this function returns (), also a ZST that is being black_box'ed. Might be better to just directly use this in the benchmarks:
self.0 += 1f32;
black_box(self.0)There was a problem hiding this comment.
How would that work with the generic trait over Table and Sparse ? I could deref..? Or should we just keep the redundancy for now and address generics on a follow up PR ?
There was a problem hiding this comment.
Oh you probably mean that I change the implementation of each bench_modify ; I can do that tomorrow 👍
| black_box(&mut *table); | ||
| let mut query = world.query::<&mut T>(); | ||
| for mut component in query.iter_mut(&mut world) { | ||
| black_box(component.bench_modify()); |
There was a problem hiding this comment.
not sure about that black_box, probably doesn't hurt but as it is inside bench_modify now it's probably not useful here ?
- made macro to generate storage benches rather than duplicate - new test for detecting only a few changes
…ts ; fixed generic component parameter not used
ThierryBerger
force-pushed
the
4883-benchmark-change
branch
from
7466d6c to
cd09d29
Compare
|
bors r+ |
# Objective Fixes #4883. ## Solution - Add benchmarks for Changed and Added Disclaimer: I'm not that much familiar with benchmarking.
bors
bot
changed the title
# Objective Fixes bevyengine#4883. ## Solution - Add benchmarks for Changed and Added Disclaimer: I'm not that much familiar with benchmarking.
# Objective - #4972 introduce a benchmark to measure chang detection performance - However,it uses `iter_batch ` cause a lot of overhead in clone data to each routine closure(it feels like a bug in`iter_batch `) and constructs new query in every iter.This overhead masks the real change detection throughput we want to measure. Instead of evaluating raw change detection, the benchmark ends up dominated by data cloning and allocation costs. ## Solution - Use iter_batch_ref to reduce the benchmark overload - Use cached query to better reflect real-world usage scenarios. - Add more benmark --- ## Changelog
5 participants
Objective
Fixes #4883.
Solution
Disclaimer: I'm not that much familiar with benchmarking.