-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Endianness based Entity representation #3788
Conversation
Here's a updated set of benchmarks from my machine (running off of a AMD 5950x). 34 benchmarks saw a tangible improvement, 24 saw a tangible regression in performance. |
Most pf the regressions don’t look too bad and the overall improvements are substantial especial on 2 or 3 of the benchmarks. I also think that those looking for determinism will be okay having to specify that themselves with regard to endianness of entities as this will not be the only concern they need to make sure they get right when seeking such determinism. I am in favour of this change. LGTM! |
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.
I like the look of the benchmarks, and the code changes are very well documented. Presumably this will play fine with niching?
In particular, the improvements to the |
What is niching @alice-i-cecile ? |
The ability to store an |
Ah yes, that is indeed quite awesome! As far as I can tell this PR should play well with niching. |
Niching only really has an effect on the assembly generated for constructing an Entity. |
913acba
to
c5708d9
Compare
c5708d9
to
bd81fc8
Compare
Reran benchmarks now that we have
|
I'll leave the interpretation to you but here are the results from an M1 Max:
|
Closing in favor of #10558. |
Objective
Fixes #2346. Optimize various functions on
Entity
. This impacts the performance of all corners of ECS and the engine as a whole.Solution
Use
#[repr(C, align(8))]
to makeEntity
a pusedo-u64. This is reliant on the endianness of the target platform, so two different definitions of Entity are made, one for little endian and one for big endian. This significantly improves the assembly generated for many operations without adding to the complexity of the type's implementation (see https://godbolt.org/z/jcxY5G9r4). The one major cost is that accessinggeneration
by itself requires one more instruction.Since the order of the fields is no longer consistent from platform to platform, a explicit implementation of
PartialOrd
is required to enforce correctness. Asto_bits
becomes a no-op when inlined, this comparison, while looking complex, boils down to a single u64 comparison.This PR aims to get the benefits of #2372 without introducing significant code complexity.