Skip to content

Conversation

@martindevans
Copy link
Contributor

Added basic support for Myriad.ECS.

There are a couple of issues here I want to raise:

Myriad doesn't support adding/removing components or creating/deleting entities without using a CommandBuffer. Right now I haven't implemented those benchmarks (especially since there's a separate category for CommandBuffer based operations). Do you think I should implement them, using a CommandBuffer?

Alternatively, we could expand the category of CommandBuffer operations to cover all these things - right now there's just one benchmark that does adding and removing components with a buffer, which feels a bit weird. This could be expanded into:

  • Buffered add component to existing entity
  • Buffered remove component
  • Buffered create entity with N components
  • Buffered destroy entity

Another thing, this isn't an issue with Myriad specifically, is that the entity counts seem very small. For example the query benchmarks (QueryT1, QueryT5, QueryFragmentedT1) all operate over just 100 entities. Can I suggest parameterising this and running it at e.g. 100 and 10,000 entities? That way we can see better how the different options scale.

@friflo
Copy link
Owner

friflo commented Jul 28, 2024

hi,

thx for PR.

Myriad doesn't support adding/removing components or creating/deleting entities without using a CommandBuffer. Right now I haven't implemented those benchmarks (especially since there's a separate category for CommandBuffer based operations). Do you think I should implement them, using a CommandBuffer?

I leave this decision up to you. For now I will add a footnote. Somtehing like
[^2] Myriad doesn't support adding/removing components or creating/deleting entities without using a CommandBuffer.

Alternatively, we could expand the category of CommandBuffer operations to cover all these things

Make sense. We can add

  • CommandBuffer - create entity with 1 / 3 components (so they can be compared to unbuffered version)
  • CommandBuffer - delete entity with 5 components (so they can be compared to unbuffered version)

I dont't want split Add & Remove component. Two reasons in this case and in general:

  • I want to ensure benchmark clarity. So not too many benchmarks with are only slightly different.
  • Running all benchmarks should not take too long. It takes already one hour. More benchmarks - more time.

Can I suggest parameterising this and running it at e.g. 100 and 10,000 entities?

Okay - why not. But doing this was not my original intention. As Doraku's bechmarks already focus large data sets.

@friflo friflo merged commit 73492ce into friflo:main Jul 28, 2024
@friflo
Copy link
Owner

friflo commented Jul 28, 2024

Will update the benchmark results on Tuesday.

@martindevans martindevans deleted the myriad.ecs branch July 28, 2024 20:43
@martindevans
Copy link
Contributor Author

Thanks for the fast turnaround!

I leave this decision up to you.

In that case I'll leave it as-is until there are more CommandBuffer focused benchmarks. The footnote is a good idea 👍

I dont't want split Add & Remove component.

Those are good points, I don't think keeping them together will obscure the results very much and more benchmarks is always a problem for projects like this!

@friflo
Copy link
Owner

friflo commented Aug 5, 2024

hi @martindevans

Myriad doesn't support adding/removing components or creating/deleting entities without using a CommandBuffer. Right now I haven't implemented those benchmarks (especially since there's a separate category for CommandBuffer based operations). Do you think I should implement them, using a CommandBuffer?

Just implemented the following benchmarks for Myriad

  • AddRemoveComponents
  • CreateEntity
  • DeleteEntity

While simplifying benchmarks I got an overall view of all benchamrks.
So I came to the opinion to implement the missing benchmarks for Myriad.
Otherwise comparison is not fair. I did the same for Morpeh which also requires a command buffer.

I also added a note to Benchmark constrains in the readme.

  • An ECS framework must implement all Basic benchmarks. Reasons
    • Operations used in basic benchmarks are required to build a rudimentary application.
    • Avoid cherry picking of benchmarks with good performance.

@martindevans
Copy link
Contributor Author

martindevans commented Aug 5, 2024

Fair enough. Personally I would split out the tests into buffered and non-buffered groups, so there's a more direct comparison. I know there's always a desire to keep the number of benchmarks minimal in projects like though, so I understand if you'd rather not do that!

Aside from buffered, I do think there's an issue in the way the entity creation benchmark is setup at the moment which could easily be fixed.

Many of the frameworks create an entity directly in an archetype (e.g. Friflo, Arch). Presumably just giving those new entities the default value for the component types (although, maybe giving them junk values, which would be even worse). Of course in a real-world environment creating a large number of default entities isn't very useful and you'll always want to set component values!

With the current setup you could get a really fantastic benchmark score by locating the correct archetype (during setup, so that's free) and then the actual benchmark would just bump the entity count by 100! Not very useful though 😆

In my opinion it would be better to specify values that all of those components have to have. Since every component contains an int, maybe just specify that it has to the the loop index.


As a brief explanation of why Myriad doesn't generally support non-buffered operations. I used to use Arch as my ECS, if you use non-buffered operations during a query it can work but it's very risky and might break the query or corrupt the entire world state. It's a frequent issue that comes up on the Arch Discord. When implementing Myriad I decided to just sidestep the entire issue by not supporting non-buffered operations.

friflo added a commit that referenced this pull request Aug 5, 2024
… in applications.

Follow up of feedback in
#4 (comment)

Before components had default values.
@friflo
Copy link
Owner

friflo commented Aug 5, 2024

thx for your response.

Bulk create entities

I agree created entities must be created with individual component values.
That is what you do in an application.
So I added this in 8ca6616
Now only 100 entities are created. Before 100.000.

Btw: fennecs also support bulk creation.

Many of the frameworks create an entity directly in an archetype (e.g. Friflo Arch) Presumably just giving those new entities the default value for the component types (although, maybe giving them junk values, which would be even worse).

In case of friflo components are defaulted. For Arch I don't know.

I guess I will add a non bulk create entities.
Nevertheless having a bulk create is a useful feature. It is basically the counterpart of a query.
The counterpart of creating a single entity is get/set component.

Use required command buffer in Basic benchmarks

The Basics benchmarks I don't want to change. Without these benchmarks / operations a world will always be empty.

@martindevans
Copy link
Contributor Author

I added this in 8ca6616

The Myriad one there executes the buffer twice (once in the CreateEntities extension, and then again as part of the benchmark). I'll submit a PR to fix it in a sec :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants