Skip to content
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

Use overlapped union of sequential layout via Unsafe class? #57

Closed
dzmitry-lahoda opened this issue Aug 10, 2019 · 3 comments
Closed

Use overlapped union of sequential layout via Unsafe class? #57

dzmitry-lahoda opened this issue Aug 10, 2019 · 3 comments

Comments

@dzmitry-lahoda
Copy link

fixed (byte* memoryP = memory)

  • ability to process commands in parallel(from any location, non sequential) in future
  • probably better alignment of data -> better performance
  • packing with offsets seems premature optimization for the case, does not look that there will be much of commands per frame or so.
  • you can choose larger struct and put all into it like Champion "Discriminated Unions" dotnet/csharplang#113 (comment) , this seems may be design of C# 9
  • it is possible to write generator for overlapped union if needed either, until than manual typing may suffice
  • putting commands with GC object as first argument may incorporate overlapped managed structs either
  • use https://www.nuget.org/packages/System.Runtime.CompilerServices.Unsafe/ to avoid unsafe compile directive and alien(star) code
  • at least one (real time third person online shooter) game uses on marked uses such design, it is switch heavy for custom commands-events (this are marker interface, but optimization will be overlapped union).
  • it also uses pooled span list from https://github.com/jtmueller/Collections.Pooled
@Doraku
Copy link
Owner

Doraku commented Aug 13, 2019

  • ability to process commands in parallel(from any location, non sequential) in future

The purpose of the command recorder is to allow sequential deferred execution of actions saved in parallel so I am not sure what you mean by processing commands in parallel?

  • probably better alignment of data -> better performance

This goes with the overlapped union I guess. The problem I see with this is the recording of Set, since you need to record the value which will be set, and obviously we can't know the max size of a command from the start, and even so a lot of space would be lost.

  • packing with offsets seems premature optimization for the case, does not look that there will be much of commands per frame or so.

I went with offset for code reuse actually. Since you can create new entities on the recorder but you don't know their ids beforehand, I didn't find a better solution that leave space for the entity when it will be really created on the buffer and save the offset to later retrieve it. For already existing entities, I just save them and use the offset the same way so there is no difference for each case.

  • putting commands with GC object as first argument may incorporate overlapped managed structs either

not sure what you mean here.

All that unsafe code is ugly but apart the benefit of dropping the unsafe directive I am not sold on the Unsafe class usage, especially if it imply some overhead. I will do some testing and see if it's just bad supposition on my part.

I hesitated to use ArrayPool but decided against for now. Recycling of array sure is nice for initialization speed up but they remained in memory even if you don't need them anymore. There is already a lot of static array used here and there so I didn't want to stress the retained memory further.

@dzmitry-lahoda
Copy link
Author

dzmitry-lahoda commented Aug 15, 2019

The purpose of the command recorder is to allow sequential deferred execution of actions saved in parallel so I am not sure what you mean by processing commands in parallel?

I mean that may be in some future time commands may be processed in parallel. So current design is limiting.

This goes with the overlapped union I guess. The problem I see with this is the recording of Set, since you need to record the value which will be set, and obviously we can't know the max size of a command from the start, and even so a lot of space would be lost.

The design of Rust unions to have max size of struct from possible. It is possible to do exactly same in C# manually via field layout and Unsafe casts. All types of commands are know from start. So may choose max.

I doubt about much space given limited amount of commands generated per frame. But uniform size of command (choose max) probably will give more capabilites to evolve in future. Size of components matter more.

not sure what you mean here.

First field of union could be object to allow class based commands and non class based in same container. CLR prevents to store struct data in same location as object reference (Field offset of object could be 0). Will lost 64 bit to store null so.

All that unsafe code is ugly but apart the benefit of dropping the unsafe directive I am not sold on the Unsafe class usage, especially if it imply some overhead. I will do some testing and see if it's just bad supposition on my part.

I have found Unsafe to be faster is some scenarios. I think it is because CRL has proper type information and may generate better code (inlike with pointers)

I hesitated to use ArrayPool but decided against for now. Recycling of array sure is nice for initialization speed up but they remained in memory even if you don't need them anymore. There is already a lot of static array used here and there so I didn't want to stress the retained memory further.

Unity has its own allocators https://github.com/dzmitry-lahoda/memory-pool-unity-unsafe. I see people experiment with custom allocators in C# either. So to have reusable ECS probably memory allocations should be abstracted somehow.

@Doraku
Copy link
Owner

Doraku commented Nov 19, 2022

Cleaning old issues as this is not something that's going to change now without a pretty good reason.

@Doraku Doraku closed this as completed Nov 19, 2022
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

No branches or pull requests

2 participants