-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Change datastructure in Components #19043
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
base: main
Are you sure you want to change the base?
Conversation
how often is a new item on |
@hukasu Only when a component is added. I don't quite know the stats for an average bevy game, but with a component count of 500 (which I think is on the high end), 500 times. |
i'm running locally tests to see the times with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implementation looks good to me! The performance is really hard to justify at the moment IMO. I do want this, and I do think this is a good implementation, but I think we should work on caching component id where possible before merging this.
We should also look into other options. Since the component id is still dense for now, can we try a specialized hash function? We have one for Entity
in the long run. The hashmap is a good move long term. We just need caching and a better hash function.
BTW, I'm cooking up better caching for queries ATM. Should be able to skip all component maps for queries in my upcoming PR.
IMO, divide and conquer between benching different map types and finding creative ways to cache and skip maps altogether. Then merge this (because it really does look good.)
Is flecs' approach of having the equivalent of a |
Yes, but IMO, that's an optimization to look at after components as entities. We need "components can be totally sparse" (which that wouldn't help with), -> Component ids are entities -> Optimize entity hashmap. (IMO) |
i've ran locally the benches 5 times now, and i have not see regressions as the ones shown here, i think the highest regression using i also saved the criterion reports but each zip is 33mb |
Just putting it out there that with our currently dense ids and few components in benchmarks, the hybrid storage will be effectively just what we have today (from benchmark perspective). @hukasu Could you run |
Objective
We've been looking at moving to sparse ComponentIds for a while now, partly in preparation for components as entities.
Solution
As such, I'm changing the
Vec<Option<ComponentInfo>>
into a new HybridMap, which combines the Vec storage with a HashMap for component indices below a certain threshold.Performance
Running the ECS performance tests, I got the following result. These are sorted from best to worst, with a negative percentage meaning a reduction in time. Of all the tests, 255 improved, and 139 regressed, although most stayed roughly equal.
I've tested with two thresholds: 128 and 256, and 128 was better. Although I will admit that my laptop is trying its heart out and I'm somewhat doubtful it can produce reliable results. The 128 to 256 jump shouldn't have made the difference it did.
Benchmark results