Grouped component

[ffreyer]

I did a bit of performance checking and noticed that SharedComponent is comparable to Component in terms of speed when getting values but much slower when setting. I assumed this is mostly because the duplication check, so I tried making another component that does not do this.

So basically GroupedComponent is SharedComponent but without any check for duplication. The normal syntax of component[entity] = value will always store a new value and generate a new group. You can add an entity to that group via component[entity] = parent_entity, which is somewhat like inheritance. However there is no "parent" - removing an entity will only remove that entity and only clear the value if it's the last entity in its group.

I also added an Entity(::AbstractLedger, parent::Entity, components...) method which "inherits" all components from parent, with passed components taking priority. (I.e. if you pass a Color the new entity will have that color, not the one from the parent.)

My hope is that this will be helpful for entity collections with shared attributes. In the context of Makie, for example, transforming scatter(rand(Point3f0, 1000), color = :red) into 1000 entities, without duplicating the color 1000 times.

Performance test I ran:
https://gist.github.com/ffreyer/0b56242048b6ca18bde0caa206e24847

This probably needs some more changes... For example color[e] = GroupedColor(rand(RGBA{Float32})) should create new groups but it adjusts the grouped value instead...

[ffreyer]

I suppose for setindex! there needs to be a way to differentiate "change the grouped value" and "change this entities value".

[louisponet]

This is quite nice! I like the idea of not doing the duplication check in SharedComponent for certain cases.

Couple of questions:

• couldn't we implement that functionality into SharedComponent in the same way?
• I was playing around a bit with AbstractEntity, maybe there could be another way of doing this with ChildEntity that has his own id and parent id. Then e.g. getindex would check whether parent is in the compoment, if not it searches for its own Id.
• I would like to keep the terminology Group for the Groups of compoments with aligned entities

[ffreyer]

This is quite nice! I like the idea of not doing the duplication check in SharedComponent for certain cases.

Couple of questions:

• couldn't we implement that functionality into SharedComponent in the same way?

We can. This pretty much just changes what setindex! and pop! do if I'm not mistaken. I guess the question then is how to differentiate "add if unique" from "just add it". Maybe we could have an explicit make_unique!?

• I was playing around a bit with AbstractEntity, maybe there could be another way of doing this with ChildEntity that has his own id and parent id. Then e.g. getindex would check whether parent is in the compoment, if not it searches for its own Id.

That could work too, though with just the entity you wouldn't be able to tell if the parent has a parent and which entity that might be. I also think that inheritance should be more of a per component (and entity) thing than a per entity thing. Otherwise you can't do "inherit a, b, c from X and d, e from Y"...

• I would like to keep the terminology Group for the Groups of compoments with aligned entities

Ok. I think this could be merged relatively easily with SharedComponent, we just need to figure out how to differentiate "do if unique" from "just do it"...

suppose for setindex! there needs to be a way to differentiate "change the grouped value" and "change this entities value".

Maybe we could have a wrapper type for entity to signal that it should adjust the group. E.g. component[group(entity)] = value?

[ffreyer]

I adjusted things to work how I wanted them to work:

• component[entity] = parent should replace the group of the given entity with that of parent if it exists. If that was the only reference to a value, that value should be removed
• component[entity] = value should replace that value only for the given entity. I.e. if it is grouped a new group should be created
• component[ParentGroup(entity)] = value should replace the value for all grouped entities

That required some checks on whether a group index is unique, which decreased performance. I assume that can be fixed in a memory vs performance trade of.

julia> print_timer()
 ──────────────────────────────────────────────────────────────────
                           Time                   Allocations      
                   ──────────────────────   ───────────────────────
 Tot / % measured:      2.03s / 43.1%            462MiB / 79.3%    

 Section   ncalls     time   %tot     avg     alloc   %tot      avg
 ──────────────────────────────────────────────────────────────────
 Grouped    1.00k    499ms  56.9%   499μs    183MiB  50.0%   188KiB
 Flat       1.00k    378ms  43.1%   378μs    183MiB  50.0%   188KiB
 ──────────────────────────────────────────────────────────────────

Yup, with group_size:

julia> print_timer()
 ──────────────────────────────────────────────────────────────────
                           Time                   Allocations      
                   ──────────────────────   ───────────────────────
 Tot / % measured:      767ms / 97.5%            368MiB / 100%     

 Section   ncalls     time   %tot     avg     alloc   %tot      avg
 ──────────────────────────────────────────────────────────────────
 Grouped    1.00k    381ms  51.0%   381μs    183MiB  50.0%   188KiB
 Flat       1.00k    366ms  49.0%   366μs    183MiB  50.0%   188KiB
 ──────────────────────────────────────────────────────────────────

[louisponet]

Sorry for my very delayed response, I like this approach a lot, thank you! I will have a look at some things and play around with it to evaluate it and come back to you!

[louisponet]

This is super nice work I like it a lot! Thanks for your effort.

If I understand it correctly, the root "parent" in this case is really just the data value itself, not any given particular entity. All entities that have that datavalue can be used as parents to a new Entity equally validly.
If I understood that correctly, I think this is almost like a better version of SharedComponent, where you'd get the latter's functionality upon calling make_unique! if you so please.

Do you think the current SharedComponent still has a place? If not I think I'd prefer also that naming over GroupedComponent, because I somehow still think that a component group should signify components with identical entity orders and datalayout.

Anyway, as far as I can see except for those couple of @inbounds, this is pretty much ready to be merged, thanks again!

[louisponet]

cool, I did not know this - Bool turns into - 0 or 1 :D

[ffreyer]

Yea, you can skip branching with that sometimes

[louisponet]

Oh Before I forget, I added some benchmarks etc to the master, along with that new iteration scheme, could you rebase your branch on that if it's not too much of a hassle?

[ffreyer]

If I understand it correctly, the root "parent" in this case is really just the data value itself, not any given particular entity. All entities that have that datavalue can be used as parents to a new Entity equally validly.

It's been quite a while since I worked on this so I'm not sure what exactly I did here. But I think the way it's set up every entity belonging to the same group can act as parent to that group. And the group is just an index pointing to some datavalue.

If I understood that correctly, I think this is almost like a better version of SharedComponent, where you'd get the latter's functionality upon calling make_unique! if you so please.

I think that was my original goal with this. Make a SharedComponent with better setindex! performance by relaxing some constraints.

Do you think the current SharedComponent still has a place? If not I think I'd prefer also that naming over GroupedComponent, because I somehow still think that a component group should signify components with identical entity orders and datalayout.

I think the way I build this it can do everything a SharedComponent does with a manual make_unique!() but can also be faster if you skip that or handle it explicitly yourself. Personally I'd say this is package that aims to be fast over convenient so I'd take this over the current SharedComponent.

Anyway, as far as I can see except for those couple of @inbounds, this is pretty much ready to be merged, thanks again!

Ok, I'll try to update things now