Add benchmarks for spawning and inserting bundles#19762
Add benchmarks for spawning and inserting bundles#19762alice-i-cecile merged 4 commits intobevyengine:mainfrom
Conversation
|
What's the purpose of the |
|
Good catch! They were used by some other benchmarks in #19491 but I removed those, so the components became unused (and for some reason the compiler didn't complain?) |
ElliottjPierce
left a comment
ElliottjPierce
left a comment
There was a problem hiding this comment.
Overall, this looks good to me.
Two quick notes (which are probably fine as is but are worth pointing out):
First, this is a lot of very small files. That's not a problem, and I don't know what your plans are for more benches, but at least this pr may be easier in one file. IDK.
Second, this benches the flat "spawn these bundles on a fresh world" situation, which probably isn't what we want to bench for static bundles. The iter, what's being benched, includes benching creating the world, creating the bundle info, etc. If that's what you're trying to bench, that's fine, but most of the time in practice, when a user inserts a bundle, the bundle info, component registration, and archetype will typically already exist. It might be worth doing one iteration of the loop before the iter just to prime the world, but again, it depends on what you're trying to bench.
| bencher.iter(|| { | ||
| let mut world = World::new(); | ||
| for _ in 0..ENTITY_COUNT { | ||
| world.spawn(black_box(( |
There was a problem hiding this comment.
I see the black_box being used here on the bundle before spawning it. You'll definitely need the blackbox somewhere, but I'm not sure here is where it should be...
I'm not sure if it would work exactly, or what precisely you're trying to bench, but putting the black box over the bundle makes rustc unsure where the bundle comes from. That makes sense for dynamic bundles, but for static bundles, I don't mind letting the compiler see that information since that's going to be more realistic. Have you tried passing a &mut World and Entity to the blackbox instead? That might be more realistic for static bundles. That said, I'm not expert, and it really does depend on what you're testing. As it is, this is kinda more benching spawning an fn() -> impl StaticBundle. So up to you.
There was a problem hiding this comment.
Initially I put them for consistency with the dynamic ones, because for them I would expect the compiler to not know whether there is a Some or a None. In this case it doesn't really matter since all the components are ZST, but in any case I removed it.
Have you tried passing a &mut World and Entity to the blackbox instead? That might be more realistic for static bundles.
Since I moved the World outside the iter I believe this is not automatically done by criterion when calling the inner closure (since it black_boxes it, which has the effect of black_boxing all the captured variables including the World).
ElliottjPierce
added
A-ECS
You can see #19491 for an overview of the benchmarks I had in mind, which I now plan to introduce when I'm implementing the relevant features. Since some of those features can be implemented in parallel (e.g. Another option however would be to split the benchmarks by the feature they are testing rather than the situation (so e.g. all the static bundle benchmarks from this PR together, then all the
My worry was that reusing the same |
I like this idea. But it's just personal preference. Up to you.
Yeah, no easy answer here, and it depends on what you're goal is. I wish criterion had a way to bench only part of the iter method, but I don't think that's possible. I think what you have now is a good balance. At the end of the day, you need to either pay for the create world, register, and spawn or the spawn, drop, and clear. Either way the benches are not just benching bundle action speed unfortunately. |
alice-i-cecile
added
S-Ready-For-Final-Review
4 participants
Objective
Optionbundles andBox<dyn Bundle>#19491