Skip to content

Add benchmarks for spawning and inserting bundles #19762

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

SkiFire13
Copy link
Contributor

Objective

@copygirl
Copy link
Contributor

What's the purpose of the W and B components? They appear unused, currently.

@SkiFire13
Copy link
Contributor Author

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?)

Copy link
Contributor

@ElliottjPierce ElliottjPierce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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((
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ElliottjPierce added A-ECS Entities, components, systems, and events C-Benchmarks Stress tests and benchmarks used to measure how fast things are S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jun 21, 2025
@SkiFire13
Copy link
Contributor Author

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.

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. Option bundles, Box<dyn Bundle> and enum bundles, which were missing from that PR) I would prefer not to do the split in each of those PRs.

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 Option benchmarks together in another file, etc etc).

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.

My worry was that reusing the same World would make each iteration more expensive than the previous one because it's inserting in a World with existing entities, so you end up benchmarking how costly are the reallocs of the entities Vecs. In fact I just tried to change it to reuse the same World and the benchmarks suddently became 4-5x slower. To try to fix this I added a .clear_entities() at the end of each .iter(), and that made them slightly faster than they currently are, so it should have reduced a bit of overhead.

@ElliottjPierce
Copy link
Contributor

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 Option benchmarks together in another file, etc etc).

I like this idea. But it's just personal preference. Up to you.

My worry was that reusing the same World would make each iteration more expensive than the previous one because it's inserting in a World with existing entities, so you end up benchmarking how costly are the reallocs of the entities Vecs. In fact I just tried to change it to reuse the same World and the benchmarks suddently became 4-5x slower. To try to fix this I added a .clear_entities() at the end of each .iter(), and that made them slightly faster than they currently are, so it should have reduced a bit of overhead.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Benchmarks Stress tests and benchmarks used to measure how fast things are S-Needs-Review Needs reviewer attention (from anyone!) to move forward
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants