-
-
Notifications
You must be signed in to change notification settings - Fork 892
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
Component polymorphism support feature & prototype #859
Comments
Hi, I see that your branch has 10 changed files with 2,170 additions and 112 deletions. That being said, let's start with scratching the surface though. A bunch of questions/doubts after a quick glance:
I won't go into the details of As I was reading your proposal, I thought of a much less impactful implementation to support polymorphic types. It would also fit with an upcoming feature, literally entering the same design. |
Thanks for your reply! I will write in details about ideas and implementation and put it in the heading comment for easier access as soon as possible. And also here is a brief response for your first questions:
|
I have added a lot of details into the heading comment, and will add more with progression of this discussion. Those should help to dive into what I am trying to achieve, and how this whole thing even works (or, at least, how its supposed to work). If some parts are incorrect or unclear, please, notify me and I will rewrite them. |
That's great. Thank you very much. I'll try to go through it as soon as possible. A quick question (and sorry if it sounds stupid but I still have to dig into your explanation). |
I'm not sure how physics libraries work, but if they have some sort of shape hierarchy, then it might be an example of non-extensible third-party class as well. From what I understand, having physics-related components is somewhat common. |
It is a good question, right now there are no way to declare hierarchy for third-party components. However it seems possible to add alternative way of declaring hierarchy separately from the classes. |
I'm trying to wrap my head around the implementation details. A bit at a time. Let's suppose there are 3 pools for types template<typename Component>
struct polymorphic_component_container; That is, if it's the base type or the final type. This also makes it difficult to understand how this type is used. |
I suppose, you have hierarchy like this:
When you add component
So When you add, for example
Returning to your question about When attached to an entity, component (lets assume it is Hope this will help a bit. |
Yeah, definitely. Thank you a lot. |
Yes, it limits multithreading for polymorphic components, which share a parent, because accesses to these components will also affect shared parent pool. Only pools, related to separate hierarchies can be accessed in parallel. However I think it is expected, that components, that are somehow related to each other, are not safe to access from different threads. |
This is a contrived point imho. |
Yes, I see, from this perspective this issue seems much scarier. From the first glance it seems impossible to make viable polymorphic component implementation with safe multithreaded access for intersecting hierarchy, while keeping whole idea of The one idea is accessing all child pools while trying to get component by its base type, instead of adding references to parent pools. This way it will be safe to modify different pools, because this modifications will not touch their shared parent pool. But in this way get and iterate operations will be extremely slow and will require a lot of random memory access. So this is not an option. I think the possible solution from design perspective could be adding some sort of compile time guards, when you can maybe declare what components you will access from which thread (I am not sure, how exactly this will look, it is just my first idea). This will require additional declaration, but it will not compile, in case of conflicts. This may also help with non-polymorphic components, to guard same types from being accessed from different threads. |
In the particular case of Qt, all It has little to nothing to do with the proposal, just something to keep an eye on: sometimes you shouldn't store things by value by design. |
I think such problem will be solved along with adding the possibility to declare third party hierarchies, I assume that you will be able to say something like " |
What about making this whole thing external with respect to the pools? Something like |
The idea here is same to this one:
And also this will require update all of such calls each time new derived type is added. And there can be dozens of child types, all of which will be required to check for each iteration, which is a major drawback, even if you fix the first one by separately declaring derived list once.
In the heading comment there is explanation about how this is solved right now - search for |
Could be improved (kinda): template< class T >
struct derived_types
{
using value = entt::type_list<T>;
};
template<>
struct derived_types< Base >
{
using value = entt::type_list< Base, Derived1, Derived2 >;
};
template< class... T >
entt::registry::poly_for_each (entt::type_list< T... >)
{
entt::registry::each< T > ()...;
}
template< class T >
entt::registry::poly_for_each (...)
{
entt::registry::poly_for_each (typename derived_types< T >::value{});
} |
Yep, as I've said. However there are still performance issues.
|
What if one implemented this with a storage mixin? |
This will remove the need for separate declaration, but the main, in my opinion, problem still remains. Say, I have 10 component types, that derive from one base (common case, if the base is some kind of an interface). With the original approach iterating all the components by this base will be nearly as fast, as doing same thing with non-polymorphic component - just iterate through one storage, everything is already there. However addition and deletion of derived components is slower, because we need to emplace/remove references for one parent. And it will not be thread safe between this 10 components. On the other hand, @Innokentiy-Alaytsev's approach will be completely thread safe and will not slow down addition and deletion. But get and iterate operations will suffer a lot. It is not the same case, as iterating through entitites with all of given components, it is now any of given components. And to do this, you are required to iterate through all entities, no smallest pool optimization is available + for each of all entitites you will need to check all 10 pools. But, maybe there is a better way to do this, which I cant see right away, so please, correct me, if I am wrong. |
Mmm no, wait. What I'm saying is that all pools created with types that derive from B could register themselves on creation in a list (somehow, ignore the technical details for a moment). So, when you want to iterate all and only the entities that have a B, you've a bunch of pools that you visit linearly. Anyway, just thinking aloud at the moment to see if we can have the cake and eat it. 🙂 |
Oh, I see now, dont know how I've missed this. In this way it will be a good solution, and the only cost will be just remembering already visited entities. I can try implementing something like this in the upcoming days, what do you think? |
Uhm, I mean, as you prefer. I won't stop you for sure. 🙂 |
Just to be clear, what I mean is something along this line. |
Yes, thank you a lot, I understand what you mean. I just was focused on the one solution I came up with, because I had such idea in mind before and wanted to implement it, it was obviously a mistake, I should have spend more time comparing different approaches. Although it may provide some advantages like iterating several polymorphic components at once, those just seem to be not so useful compared to the main disadvantages - how complex the implementation is and the multithreading one. And they can be implemented with your idea, maybe even with insufficient additional performance cost. As for your solution, I have one doubt right now: because registering pools happens at runtime, we don't know the exact type inside them, so converting pointer between them might be a problem with multiple inheritance. Suppose you have this types:
Now, when converting to from
If we cant solve the above problem, then we will need to declare all children for each type, and I really want to avoid such requirement. If you will come up with a better solution, please, let me know. And also I might just again be missing some obvious solution 😅 |
Or don't allow multiple inheritance. |
Yes, but I think it can be pretty useful, so it is not the best option, imho |
Stepping back to earlier discussions, here I came up with some templates to allow hierarchy declaration in several different ways, it is now not dependent on |
It's really a shame that we can't extract the base classes from a type. At least the most direct ones. It's not that the compiler doesn't know about them, so why not? It would make a lot of things a lot easier.
It was not a mistake actually. It has its pros and cons while the road from point A to point B is never straight. 🤷♂️
Yeah, for sure. When the
Far better imho. Lot of times we devs use third party libraries and it's important to desing things in such a way that they can also work with these tools. 👍 |
Thank you! I think now I will go and make an implementation, that could possibly already go into the library, so it would be easier to discuss it in details. I will post it here, when it will be ready. |
Hello, again! I've at last got my hands on implementing the new approach. Sorry, if it took a while, unfortunately, last days were busy for me. So here it is: https://github.com/zheka2304/entt/tree/polymorphic. It is smaller and much cleaner, than the previous one, and its based on ideas, that were proposed here, so it will be easier to describe. It lacks some functionality and is not completely done yet, but it has the core features. Lets first start with the updated polymorphic component declaration. Now, polymorphic components can be declared in different ways:
struct A : public entt::inherit<> {}; // base type
struct B : public entt::inherit<A> {}; // derived type
struct A {}; // base type
struct B : A {}; // derived type
// hierarchy declaration
template<>
struct entt::poly_direct_parent_types<A> {
using parent_types = type_list<>;
};
template<>
struct entt::poly_direct_parent_types<B> {
using parent_types = type_list<A>;
};
struct A {
using direct_parent_types = entt::type_list<>;
};
struct B : A {
using direct_parent_types = entt::type_list<A>;
}; There are also some utils to determine, if a type is polymorphic, and for getting all parent types of a given type: entt::is_poly_type_v<T>
entt::poly_parent_types_t<T> Also, now polymorphic component hierarchy can be declared for pointers, instead of the components themselves: template<>
struct entt::poly_direct_parent_types<A*> {
using parent_types = type_list<>;
};
template<>
struct entt::poly_direct_parent_types<B*> {
using parent_types = type_list<A*>;
}; In the above example The registry now has three new methods:
Now, lets move on to the core of the implementation. The example, given by @skypjack used
Sorry, if I've forgotten something, I still don't have much free time, so this text was written in a hurry. Please, write to me, if something is not clear or there are some mistakes. Thank you! |
During the past few days I've worked on some improvements in the same branch, so here they are:
I think right now it has most of the required functionality and the implementation is clean enough, so maybe I should move to polishing it and writing tests. Please, write your thoughts on it, I will appreciate any feedback. |
@zheka2304 have you pushed the last changes? The most recent commit is from 11 days ago from what I can see. Am I missing something? |
It is located in a separate branch - https://github.com/zheka2304/entt/tree/polymorphic |
I see, thanks. The first comments that come to my mind after a first glance:
Thanks and good job!! 🙂 |
Thank you for your feedback!
|
I don't know honestly. I can figure out an use case here. Can you help me with that?
I plan to add an |
Hi, and sorry for my late reply.
I've recently read about observers and it seems to me that they might benefit from this idea. We can, for example, observe updates of the base polymorphic component and then iterate over all entities, where any derived component has been updated, and do something with it. I am still exploring the conception and applications of observers in However I think, that even if we will decide to keep this feature, it should be disabled by default and enabled for certain component types manually, as it has a performance impact.
Understood and working on it and on the other things mentioned in the comments above. Only one question here: should I make algorithms independent from the |
Really, no need to apologize. 🙂
Mmm, yeah, in theory it might be an option but I still can't figure out a proper use case in practice. Not sure it's worth it but let's keep it in a corner for later. We'll have the time to think about it a little more.
Definitely. Pay for what you use. I don't want to affect the normal uses with a corner case feature.
I think so 🤔 not sure if it's possible but it would be great if it only worked with storage classes and the like. |
Ok, I will move to separating signal handling and polymorphic parts of the mixin. There is not much code, and it should be rewritten anyway, so it is safe to clean it up for now.
Here is my idea, how it should look. I've made a type Then, I've created With it, implementing polymorphic algorithms, in a way, they don't rely on registry type becomes easy. What do you think about it? |
You probably have the whole picture in mind and I'm glad. Although, honestly, I can't easily understand how it turns out. 😅 |
I've cleaned everything up, removed polymorphic header include in registry, so it can be included separately, only when it is required. I've also added some tests for polymorphic type traits and for algorithms & functionality.
Does this mean, that I should open a pull request with what I have right now? And if it means yes, into which branch should I make it? |
Well, opening a PR is probably the best way to make the code easy to review actually. |
Thank you for making it clear, I've created a pull request #871. Looking forward to continue our discussion there :) |
Sounds good. I'm closing the issue then. Let's continue the discussione directly on the PR. 👍 |
Hello,
I'd like to introduce my work on prototype feature, that adds component polymorphism support. Such thing might seem a little controversial, so I will immediately mention two things:
I already have working prototype in the experimental branch of my fork. Of course it requires some review and improvements, and some parts may not be made in a nicest way possible right now, but it is fully functional.
Here is a basic example:
You can try it yourself, it is already in the fork, along with some tests. I will appreciate any feedback about how to improve both the concept and the solution, and hope it will make it way into
EnTT
in the future.Now lets dive into some details:
Problems, I try to address
Maybe the main problem, this feature solves, is accessing components without knowing their exact type. It is sometimes can be very useful to access component as some interface/base type without knowing what exactly it is (so basically this is what polymorphism is about in general, and this feature is applying this concept to ECS).
In the context of ECS it results in 2 main ideas: we should be able to access component by any of its base types and we also should be able to access all such components, attached to an entity, because there of course can be more than one. Example of this idea is already shown above, we create two component types, that implement
Ticking
interface and add both of them to an entity, and then we can just iterate all ticking components and do something with them (probably just calltick
in this case).Additionally, to follow "pay only for what you use" rule, such polymorphic components must be treated separately and must not affect performance of other components.
Some specs
The basic idea is simple: component can be accessed not only by its own type, but by any of its parent types. From the example above we can see, that both
Physics
andAI
components inherit from, and so can be accessed as reference toTicking
. Here I tried to come up with a bit more formal set of rules:entt::polymorphic
orentt::inherit<Parent1, Parent2, ...>
. Polymorphic component can inherit only from other polymorphic components to keep them separate from non-polymorphic. With this requirement we can completely separate all logic at the compile time and preserve performance for non-polymorphic components.3.1. Still get one of this components, if we just need one. This operation ideally must be as fast, as getting one non-polymorphic component. Which component will be returned cannot be specified in this case, so we just return any of them.
3.2. Iterate over all components of this type, attached to an entity. Same as with one component, we must be able to use it both with get and with view. To achieve this, we pass
entt::every<ParentType>
instead of justParentType
toregistry.get
orregistry.view
, and get it back. Then we just iterateentt::every<ParentType>
to getParentType&
.3.3. Access with
entt::every
must work completely similar for one and for several components.erase
andremove
must delete all components inherited from given type. This might seem a bit controversial, because in some cases we need to delete component of exactly this type, so maybe in the future separate method should be added to do such things.insert
,get_or_emplace
are deleted for polymorphic components right now, andpatch
is also controversial one, as they deserve separate discussion.... This list will be expanded as the discussion progresses
Some implementation details
Wrappers (or containers, don't know, which name is better)
Main idea here is to store polymorphic components inside wrappers/containers (
polymorphic_component_container<Component>
). These wrappers are able to store component value of exactly given type and reference list to other components. They have storage for component value, list of references to other components and 2 bits for flags to store its state.Wrapper has methods for value construction/destruction, adding/removing references to other components from reference list and for getting reference to one component or
entt::every
to iterate all of them. When constructing a value, wrapper adds reference to this value into all wrappers for parent component types, when destructing - it will remove all this references. So it requires registry and entity to access those wrappers for parent types.Polymorphic components reference each other by pointer, so stable pointer is required,
in_place_delete
is true for all polymorphic types, and empty type optimization is also disabled, because wrappers are never empty.Changes in
basic_storage
Because polymorphic components are stored in wrappers and accessed in a slightly different way, separate
basic_storage
and iterator implementations are created for them. It changes implementation of get, emplace, erase and remove and also adds hidden methods forpolymorphic_component_container
likeemplace_ref
anderase_ref
.I have also added
get_as<T>
(and similar) methods for allbasic_storage
implementations and similaras<T>
methods for storage iterators. The purpose of those is resolvingentt::every
(and maybe some other type modifiers in the future). For non-polymorphic storage those methods are completely similar toget
/operator*
, they actually just call them with an extrastatic_assert
, so no performance overhead here.And here comes an uglier part:
emplace
anderase
now require registry for polymorphic storage, so right now they are just passed as an additional parameter, that is ignored for non-polymorphic component storages. Again, no performance overhead here, as the registry parameter is just optimized out, but the whole idea of such inverse dependence is pretty nasty. It is another open discussion on how to implement this in a more elegant way.Changes in
basic_view
Here things are simpler: I use
get_as<T>
/as<T>
methods I have created in storage and storage iterators to just forward everything in a right way. Because all of these methods are basically same asget
/operator*
used before for non-polymorphic types, no overhead is expected for them.There is also some changes to get unwrap
entt::every
to get storages for required component, but these are fully resolved at compile time.Changes in
basic_registry
Only changes here are passing registry to
emplace
/erase
/remove
and unwrappingentt::every
for views.Dive into
polymorphic_component_container
implementationAs was said before,
polymorphic_component_container
aka polymorphic component wrapper can store both component value and reference list. In a very basic implementation it can be done like this (and also thinking of it in this way may help a lot):But when I came to thinking about
erase
implementation the following problem kicked in. When erasing component, we must also erase all components listed inrefs
(those are child types). When deleting component, it must delete all of its references from parent wrappers, so we must know exact types of components inrefs
to do it or come up with another way. My idea was to store pointer to 'deleter' function along with component reference, such deleter will be placed when reference is added and called, when it is required to erase component by its reference. So wrapper implementation should look like this:Now lets move on to real implementation. It does not use
std::vector
norstd::optional
. Instead it hasdata
- buffer for component value or at least one pointer anduintptr_t pointer
, which can store pointer to one reference or pointer to list, depending on wrapper state. And to store wrapper state I use a solution with some pointer hacks - I rely on memory alignment, so 2 bits ofpointer
are always zero and can be used to store the state (it is achieved by aligningentt::inherit
by at least 4 bytes). This saves us one memory access when getting reference to the component and alsosizeof(pointer)
bytes of memory per wrapper, that would be otherwise added by padding. I thought it was worth it, so there is a lot of strange stuff going on inside wrapper, but if you consider this an overkill I of course will rethink it.Now lets look on the possible storage states: there are 2 bits, 2 flags that are mostly independent of each other, 1st flag - if wrapper has a value, 2nd flag - if wrapper has a reference list. Describing whole implementation here would take forever, I will try to do it separately, so here are some important notes:
component_ref_list_page_source
to optimize many small memory allocations of the same size, required for them. Note: right now template ofcomponent_ref_list_page_source
depends only on underlying allocator type, but maybe it should instead be instantiated per component type (the page size must be reduced greatly in this case).I am planning to clean up, and move some of this logic out of the wrapper class to abstract underlying memory layout a bit, right now the solution is messy.
...Implementation details are also to be expanded
The text was updated successfully, but these errors were encountered: