Skip to content
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

Benches for wave spawning #18057

Closed

Conversation

ElliottjPierce
Copy link
Contributor

@ElliottjPierce ElliottjPierce commented Feb 26, 2025

Objective

Establish benchmarks for waves of entity spawning as discussed in #18054. This does not include (for now) wave insertions, as mere insertions don't affect entity allocation.

Solution

Add benchmarks for permutations of entity count, wave count, component size, and batch spawning function.

The benchmarks do not show much difference in the component size variance, so I would be happy to remove that and decrease complexity if desired.

Showcase

Yikes! Here are the results on M2 MAX:

Screenshot 2025-02-26 at 4 11 41 PM
(Ignore the regressions and improvements. That's from testing the benches.)

Here, I only ran the EmptyComponent benches.

With only one wave, spawn_batch and insert_or_spawn_batch both run in roughly 3ms because there are no shenanigans in Entities regarding allocation.

So if one wave takes 3ms, 2 waves should take 6ms, right? Nope! spawn_batch on 2 waves takes 6ms, but insert_or_spawn_batch with the infamous alloc_at_without_replacement takes 760ms!!. That's a little more than 6.

Running 16 waves was taking too long, but I'll leave it running for a bit so we can get a number. I would estimate ~11 seconds (700ms per wave on average) for insert_or_spawn_batch. This implies that the relationship isn't quite linear since the above 2 waves was also 700ms. But this could be memory allocator shenanigans too.

EDIT: It did end up being ~11.5 seconds.

@alice-i-cecile alice-i-cecile added this to the 0.16 milestone Feb 26, 2025
@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Benchmarks Stress tests and benchmarks used to measure how fast things are X-Uncontroversial This work is generally agreed upon D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Feb 26, 2025
@alice-i-cecile
Copy link
Member

Nice stuff! Now that we've isolated the problem in question to an API that we want to cut anyways, I'm a bit iffy on adding these benchmarks. This specific problem feels very bizarrely idiosyncratic. While this PR was 100% worth the time to demonstrate the problem, I'm not sure that this set of benches is going to catch any new problems.

The benchmarks do not show much difference in the component size variance, so I would be happy to remove that and decrease complexity if desired.

If we decide to keep this let's cut that.

@alice-i-cecile alice-i-cecile added S-Needs-Design This issue requires design work to think about how it would best be accomplished X-Contentious There are nontrivial implications that should be thought through and removed X-Uncontroversial This work is generally agreed upon S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Feb 27, 2025
@ElliottjPierce
Copy link
Contributor Author

I'm a bit iffy on adding these benchmarks.

Totally agree. If the solution is to remove the think we're benchmarking, what's really the point? Still glad I got the numbers though.

If you do choose to keep this, just ping me, and I'll clean it up. Otherwise feel free to close the pr.

Also can't help but adding that the most recent label change makes I think 100% of my prs and issues contentious or controversial. Rightfully so, but it is funny.

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 D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Design This issue requires design work to think about how it would best be accomplished X-Contentious There are nontrivial implications that should be thought through
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants