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

Instead of using arrays directly, use a wrapped collection for StructHeap and IdArray #41

Closed
vkensou opened this issue Dec 13, 2024 · 4 comments

Comments

@vkensou
Copy link

vkensou commented Dec 13, 2024

I am using Friflo in Unity, and I want to use job system with it. But Unity can only use value types (NativeArray etc.) in the job system. So I must modify a lot of code. But if Friflo uses a wrapped collection, then I only need to modify a little code.

And this proposal depends on the Dispose proposal: #40

@friflo
Copy link
Owner

friflo commented Dec 13, 2024

Hm,
Right StructHeap / StructHeap<> are reference types. They also use fields with reference types internally. E.g.

class StructHeap<T> {
    internal T[] components;
    ...
}

I assume these components is what you want inside a Unity Job.
I assume you may also want to access the Archetype.entityIds inside a UnityJob.

class Archetype {
    internal int[] entityIds;
    ...
}

Friflo ECS must not have any dependency to Unity types like NativeArray<>.
If doing this the ECS cannot be used in any other platform than Unity.

So the only way I see to use components and entityIds inside a Unity Job is to wrap them in a NativeArray<> when passing them to a Unity Job. E.g.

    var nativeComponents = new NativeArray(components, Allocator.None)

Related to this also Chunks<>, QueryChunks<> and ChunkEnumerator<> are using a few reference internally. So these type can also not be used inside a Unity Job.
Not sure but I assume you want these types inside a Unity Job too.

Note:
In this project I cannot change these types to value types. It would mean to re implement the whole internal implementation. The effort doing this require many months. It also will degrade performance required for lookup of components of a specific type.

Can you show a code snippet how you use components inside a Unity Job?

@vkensou
Copy link
Author

vkensou commented Dec 13, 2024

You reminded me.
btw new NativeArray(components, Allocator.None) will throw an exception. new NativeArray must not use Allocator.None.
But I find there is an api: NativeArrayUnsafeUtility.ConvertExistingDataToNativeArray. It seems useful.

My code is simple:

foreach (var (components1, components2, components3, components4, entities) in Query.Chunks)
{
    var computeJob = navWorld.GetComputeVelocityJob(components1.Slice, components2.Slice, components3.Slice, components4.Slice, invTimeStep);
    job = JobHandle.CombineDependencies(job, computeJob.Schedule(entities.Length, 16, new JobHandle()));
}

public struct ComputeVelocityJob : IJobParallelFor
{
    [ReadOnly]
    public NativeSlice<PathDirect> pathDirects;
    [ReadOnly]
    public NativeSlice<RVOAgent> agents;
    [ReadOnly]
    public NativeSlice<Radius> radiuses;
    public NativeSlice<NavVelocity> velocities;
}

What I meant was not to change friflo to directly use NativeArray, but to wrap the Array instead. This would allow me to only modify a small part of the code myself.

But I'll test NativeArrayUnsafeUtility.ConvertExistingDataToNativeArray first. If it works, we won't need to make any changes to friflo.

@vkensou
Copy link
Author

vkensou commented Dec 13, 2024

Unfortunately, the NativeArray created by NativeArrayUnsafeUtility.ConvertExistingDataToNativeArray cannot be passed into Jobs. Unity throws an error: XXX is allocated with Temp memory. Temp memory containers cannot be used when scheduling a job, use TempJob instead.

These code could work!

static unsafe NativeArray<T> As<T>(ReadOnlySpan<T> span) where T : unmanaged
{
    fixed (T* p = span)
    {
        var na = NativeArrayUnsafeUtility.ConvertExistingDataToNativeArray<T>(p, span.Length, Allocator.None);
#if ENABLE_UNITY_COLLECTIONS_CHECKS
        var safe = AtomicSafetyHandle.Create();
        NativeArrayUnsafeUtility.SetAtomicSafetyHandle(ref na, safe);
#endif
        return na;
    }
}

public static void Dispose<T>(NativeArray<T> nativeArray) where T : unmanaged
{
#if ENABLE_UNITY_COLLECTIONS_CHECKS
    var safe = NativeArrayUnsafeUtility.GetAtomicSafetyHandle(nativeArray);
    AtomicSafetyHandle.Release(safe);
#endif
}

@vkensou
Copy link
Author

vkensou commented Dec 19, 2024

I write an extension method for Chunk and ChunkEntities to convert span to NativeArray. So, this proposal is not neccesary.

@vkensou vkensou closed this as completed Dec 19, 2024
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