ecs
ecs copied to clipboard
Value type components
Description
This PR aims to address the current performance and thread-safety limitations of the ECS by switching to using value type components. This allows better memory usage patterns, better performance, easier thread safety, and opens up many possible alleys for improvement (such as creating a system scheduler that analyses read and write dependencies between systems to automatically run systems in parallel where possible).
This will also address issue #46.
Detailed Design
These changes will overhaul massive portions of the API and will likely warrant a bump of the major version.
Here are the main tasks to be completed:
- [x] Remove
AnyObject
conformance fromComponent
protocol - [ ] Update Nexus API to add methods for mutating components
- [ ] Simplify the nexus API to enhance maintainability (there are lots of unnecessary methods that could probably be removed, what's your opinion on this @ctreffs? my main rationale behind simplifying the API is that it seems pretty difficult to maintain when there are so many methods to update)
- [ ] Implement read-only and read-write families
- [ ] Get the
FSM
API working again
Possible future optimisations
After implementing the basis of value type components there are many optimisations that can be implemented.
- [ ] Allow families to have a mixture of read-only and read-write components
- [ ] Create a
System
API to formalise how systems should be declared in a type-safe and thread-safe manner - [ ] Create a
TickScheduler
API which can run systems in a specified order and maybe even run systems in parallel when they are identified to have no common dependencies
Swift limitations
The main limitation that I have identified so far is that there isn't a way (that I know of) to restrict a protocol to only value types (unlike restricting to reference types which is possible with AnyObject
. The only workaround I can see for this is putting advice all throughout the documentation saying to use value types for components. It technically shouldn't break the ECS if a component is a reference type, however it breaks a lot of thread-safety guarantees and would probably make for pretty confusing code.
Testing
Once I am finished, I will make sure that all the tests pass and, most importantly, are still valid with the new component semantics. Currently they all pass but they most definitely shouldn't.
Source Impact
This will break every single package which depends on the ECS.
Checklist
- [x] I've read the Contribution Guidelines
- [x] I've followed the coding style of the rest of the project.
- [ ] I've added tests covering all new code paths my change adds to the project (to the extent possible).
- [ ] I've added benchmarks covering new functionality (if appropriate).
- [ ] I've verified that my change does not break any existing tests or introduce unexpected benchmark regressions.
- [ ] I've updated the documentation (if appropriate).
It seems like I accidentally included my commit to change the family size limit from 8 to 16 components, is that ok?
It seems like I accidentally included my commit to change the family size limit from 8 to 16 components, is that ok?
To be honest - since this will be a huge undertaking anyways I'd like to keep this out of the PR for now - maybe just open another PR that we can merge before we continue with this and have it already in main.
Simplify the nexus API to enhance maintainability (there are lots of unnecessary methods that could probably be removed, what's your opinion on this @ctreffs? my main rationale behind simplifying the API is that it seems pretty difficult to maintain when there are so many methods to update)
The outward facing API is not very clean right now I agree. However we have to be mindful about which APIs will be deprecated. Let's do a 2 step with the following steps:
- Deprecate API that seems not necessary in a separate PR
- Re-merge that PR into the current one
This will allow reasoning about API separate from the structure based refactor.
Codecov Report
Merging #47 (ad7349a) into master (6f1ddd2) will decrease coverage by
36.44%
. The diff coverage is100.00%
.
@@ Coverage Diff @@
## master #47 +/- ##
===========================================
- Coverage 98.09% 61.65% -36.45%
===========================================
Files 30 23 -7
Lines 1368 1523 +155
===========================================
- Hits 1342 939 -403
- Misses 26 584 +558
Implement read-only and read-write families
Should be an add-on PR following after this one.
The main limitation that I have identified so far is that there isn't a way (that I know of) to restrict a protocol to only value types (unlike restricting to reference types which is possible with AnyObject. The only workaround I can see for this is putting advice all throughout the documentation saying to use value types for components. It technically shouldn't break the ECS if a component is a reference type, however it breaks a lot of thread-safety guarantees and would probably make for pretty confusing code.
There are multiple aspects to this:
a) not using structs will break memory-locality/cache-friendlyness so using classes for components is never recommended
b) we do not use a registration based approach (so the Nexus does not know about all available component types) - this is by design and we don't want to change that - so we do not have the luxury to check on registerComponent
as other ECS do
c) there is indeed no way of enforcing only structs being extended with the Component
protocol
However I propose a mixed approach. We do not want to hamper performance by always checking if a component is a struct, but we can do a little bit better as not checking at all.
We can use a Mirror on the component types when a family is defined.
Mirror provides the ability to check for DisplayStyle where the given type can be checked for being a struct
.
I would only check this only on debug builds so this will only be affecting performance for non-release builds and help the programmer make educated choices.
Another way would be to analyse what Apple is doing with the ECS (using structs) in RealityKit https://developer.apple.com/documentation/realitykit/component
I would only check this only on debug builds so this will only be affecting performance for non-release builds and help the programmer make educated choices.
I think adding debug checks is probably the best solution to that issue right now
Implement read-only and read-write families Should be an add-on PR following after this one.
By this I'm assuming you mean only implement read-write families in this pr? because otherwise the ecs would not be properly functioning and the pr would break families
This will allow reasoning about API separate from the structure based refactor.
Yep, that sounds sensible. I'll work on that PR first then.
To be honest - since this will be a huge undertaking anyways I'd like to keep this out of the PR for now - maybe just open another PR that we can merge before we continue with this and have it already in main.
Ok, I guess I'll have to once again google how to revert a commit from the git history lol. And then I'll create a PR for that change.
By this I'm assuming you mean only implement read-write families in this pr? because otherwise the ecs would not be properly functioning and the pr would break families
exactly - we have to keep the scope manageable and not get ahead of ourselves. All these ideas are great and should be available ultimately. Just one step at a time :-)