UE_5.7 MassEntity Fragments, Memcpy and TMassFragmentTraits

Development Programming & Scripting
, ,
1

Hello,
I was upgrading my MegaBoids plugin ( MegaBoids - A general purpose group AI movement plugin for MassEntity | Fab ) for 5.7 and a significant change to MassEntity memory safety checks has me scratching my head a little bit.

The change has to do with regular fragments and the fact they are being copied around with Memcpy if they change archetype at runtime or if some other entities get moved within the same chunk. To ensure the fragment data remains valid, a check on trivially copyable types was added in 5.7 and a trait (struct TMassFragmentTraits<>) was also introduced to signify it was developer validated as being safe, when needed. This is all good for memory safety and data coherence when copying the fragments around. Fine.

These checks however raise issues for using TArray within a fragment. We also encounter this problem with using FInstancedStruct. Both are not trivially copyable because they create copies of their respective allocated memory when copied. However, since MassEntity doesn’t destroy the fragment when Memcpying them, they are also safe to Memcpy as far as I can tell since they will simply copy the allocated memory pointer until they are effectively destroyed, at which point the allocation will get Freed.

So my question is twofold.
First, is my above understanding right? Should we be able to use TArray or FInstancedStruct within a fragment safely by specifying ‘AuthorAcceptsItsNotTriviallyCopyable = true’? I see this is done in ‘FMassReplicationViewerInfoFragment‘ for TArray.
Second, how can one tell MassEntity to use the copy operator instead of Memcpy? I don’t think this is possible at the moment (and I don’t need it either) but would it be possible to reuse TMassFragmentTraits to declare a fragment type as requiring use of the copy operator? Something like ‘TMassFragmentTraits<>::UseCopyOperator’ could open the possibility to voluntarily use a less performant but necessary part of C++.

Thanks

2

I’m currently using TArray, TMap, TSet, TBitArray, TFunction, even TArray stores native C++ object along with vptr, in various Fragment types. I haven’t encounter bug yet. But i could see the potential issue here.

For example, you got object A which holds a pointer toward B. While B itself is allocated inside object A.

struct A
{
    A(){ ptr = new(RawData) B; }
    int RawData[128];
    B* ptr;                            // Unsafe
    B* GetPtr() {return (B*)RawData;}  // Safe
};

when you use memcpy to copy this object, ptr will point towards a valid object in invalid memory. It “works” until 10 minutes later someone else allocate the old memory and crash your program, which is a ticking bomb that hard to debug. A more common case is a lambda function which capture ”this”, inside a FMassFragment (or its member).

So just be careful creating pointer to FMassFragment or its memory.

As for container likes TArray<>, you should ensure AllocatorType works fine with memcpy(). FDefaultAllocator, TInlineAllocator, TFixedAllocator are all fine as far i could tell.

3

There is no doubt you absolutely do not want to reference anything within any fragment or the fragment itself through it’s memory address. Fragments get copied around all the time and there is no guarantee any address will be valid over time.
Typically my arrays will be inline allocated with a reasonable predetermined maximum size. As I said, this is what is done in MassEntity directly. However, I could not find any examples with FInstancedStruct, which I use for polymorphism. It’s not the best and I have another solution for this but it’s not implemented yet.
So really, my question is about storing pointers to memory outside of fragments within a fragment and how it supports trivial copying. Mostly, it is a request to extend the API to allow the use of copy operator, and perhaps the move operator too, for certain fragment types that are not trivially copyable.

1 Like
4

That is indeed a pretty brutal upgrade of the API
Coming from 5.4, it throws a lot of code out the window, forcing me to redo a lot.
This would have been a great thing to enforce since the beginning instead of allowing it to then remove the support along the way.
That’s a heavy price to pay to test experimental stuff