Search Unity

  1. Megacity Metro Demo now available. Download now.
    Dismiss Notice
  2. Unity support for visionOS is now available. Learn more in our blog post.
    Dismiss Notice

[SOLVED] Concurrent write to NativeQueue from another (injected) system messes up dependency

Discussion in 'Entity Component System' started by iam2bam, Dec 21, 2018.

  1. iam2bam

    iam2bam

    Joined:
    Mar 9, 2016
    Posts:
    36
    If I inject Sys_Second inside Sys_First, and use Sys_Second NativeQueue for concurrent writing, and then try to read the NativeQueue at Sys_Second it throws an exception, safety handles and input dependencies go nuts.

    1) If I call Complete() on Sys_First.OnUpdate and return an empty JobHandle, it works.
    2) If I call Complete() on Sys_Second.OnUpdate's inputDeps JobHandle (which also comes with IsCompleted=true), it doesn't.

    Sys_Second has [UpdateAfter(typeof(Sys_First))] so the dependency is correct. But inputDeps come as already completed although I get an exception that I must complete the previous job.

    EDIT: The PROBLEM was that dependencies were not connected because systems didn't share any component types. It was SOLVED by a dummy call to
    GetComponentGroup(typeof(Dat_Test));
    inside Sys_Second.

    I think injecting a system into another should account for something being shared between them, though...

    Thanks 5argon!

    I think I've traced the bug up to JobComponentSystem.GetDependency previous to Sys_Second.OnUpdate call, which passes some null pointers to m_SafetyManager.GetDependency. So it sparked the idea that dependencies are not correctly injected with these cross-system containers, but I'm not so knowledgable of the internals and want to know if anyone can shed some light on it.

    For the time being, can workaround with solution (1). But not getting the inputDeps correctly seems to be a problem that needs solving.

    Log and exception:
    Minimal test code:
    Code (CSharp):
    1. using UnityEngine;
    2. using Unity.Jobs;
    3. using Unity.Entities;
    4. using Unity.Collections;
    5. using Unity.Burst;
    6.  
    7. namespace QueueBug {
    8.  
    9.     struct Dat_Test : IComponentData {
    10.         public byte bogus;
    11.     }
    12.  
    13.     public class Sys_Init : JobComponentSystem {
    14.         [Inject] EntityManager _emgr;
    15.         protected override void OnCreateManager() {
    16.             var archetype = _emgr.CreateArchetype(typeof(Dat_Test));
    17.             var ents = new NativeArray<Entity>(100000, Allocator.Temp);
    18.             _emgr.CreateEntity(archetype, ents);
    19.             ents.Dispose();
    20.         }
    21.     }
    22.  
    23.     [UpdateAfter(typeof(Sys_Init))]
    24.     public class Sys_First : JobComponentSystem {
    25.         [Inject] Sys_Second sysSecond;
    26.  
    27.         protected override void OnCreateManager() {
    28.             Debug.Log("First Create");
    29.         }
    30.  
    31.         [BurstCompile]
    32.         struct Job : IJobProcessComponentDataWithEntity<Dat_Test> {
    33.             public NativeQueue<Entity>.Concurrent _entsMT;
    34.             public void Execute(Entity entity, int index, ref Dat_Test test) {
    35.                 _entsMT.Enqueue(entity);
    36.             }
    37.         }
    38.  
    39.         protected override JobHandle OnUpdate(JobHandle inputDeps) {
    40.             Debug.Log("First Update");
    41.             var job = new Job {
    42.                 _entsMT = sysSecond._ents.ToConcurrent()
    43.             }.Schedule(this, inputDeps);
    44.  
    45.             //FIXME: Unity ECS bug? returning job as dependency won't end up as inputDeps in Sys_Second...
    46.             //This works:
    47.             //job.Complete();
    48.             //return new JobHandle();
    49.      
    50.             //This doesn't work as expected
    51.             return job;
    52.         }
    53.     }
    54.  
    55.     [AlwaysUpdateSystem]    //<-- Important
    56.     [UpdateAfter(typeof(Sys_First))]
    57.     public class Sys_Second : JobComponentSystem {
    58.         public NativeQueue<Entity> _ents;
    59.  
    60.         protected override void OnCreateManager() {
    61.             Debug.Log("Second Create");
    62.             _ents = new NativeQueue<Entity>(Allocator.Persistent);
    63.         }
    64.  
    65.         protected override void OnDestroyManager() {
    66.             base.OnDestroyManager();
    67.             _ents.Dispose();
    68.         }
    69.  
    70.         protected override JobHandle OnUpdate(JobHandle inputDeps) {
    71.             Debug.Log("Second Update");
    72.             Debug.Log($"inputDeps completed? {inputDeps.IsCompleted}");
    73.             inputDeps.Complete();  // <--- FIXME: This should wait for previous jobs, but doesn't!
    74.  
    75.             Entity ent;
    76.             if(_ents.TryDequeue(out ent)) {
    77.                 do {
    78.                     Debug.Log("OK!");
    79.                 } while(_ents.TryDequeue(out ent));
    80.             }
    81.             return new JobHandle();
    82.         }
    83.     }
    84. }
    85.  
    Also tried putting the queue in Sys_First, and injecting it to Sys_Second to get it. Same problem.
     
    Last edited: Dec 21, 2018
  2. 5argon

    5argon

    Joined:
    Jun 10, 2013
    Posts:
    1,555
    Custom allocated native containers is not included in the "reader-writer" of the system, only memory in the ECS database that tied to a particular ECS type.

    To make the 2nd system's incoming inputDeps contains the first system's job handle the 2nd system needs to have some common type with the first. In this case the 2nd system do not have any jobs (as you returned new JobHandle, or you could just return the inputDeps which would be empty, since this system has no registered reader-writer type) so ECS think the 2nd system is not depending on any data.

    To make manually allocated memory works you can remember the handle in the 1st system's class field then use public getter to get and complete it from the 2nd system.
     
  3. iam2bam

    iam2bam

    Joined:
    Mar 9, 2016
    Posts:
    36
    Thanks for your answer!

    So, if I understand correctly, a dummy call to, e.g. GetComponentGroup() from Sys_Second would register that data type and fix this?

    Maybe by injecting a system into another some sharing of data should be assumed...

    If I understand correctly, I tried it and it didn't work either: Sys_First has the queue and concurrent job, Sys_Second access the queue in Sys_First and tries to dequeue.
    Because again, there are no dependencies between systems, can't wait on input deps and a race condition arises.
     
  4. iam2bam

    iam2bam

    Joined:
    Mar 9, 2016
    Posts:
    36
    Wow, yes, that dummy call totally worked. You saved me so much time, thank you.

    Code (CSharp):
    1.     public class Sys_Second : JobComponentSystem {
    2.         public NativeQueue<Entity> _ents;
    3.  
    4.         protected override void OnCreateManager() {
    5.             Debug.Log("Second Create");
    6.             _ents = new NativeQueue<Entity>(Allocator.Persistent);
    7.  
    8.             //This will register a mutual component type between Sys_First and this one
    9.             //so dependencies can work...
    10.             var dummy = GetComponentGroup(typeof(Dat_Test));
    11.         }
     
    Last edited: Dec 21, 2018
    5argon likes this.
  5. 5argon

    5argon

    Joined:
    Jun 10, 2013
    Posts:
    1,555
    More efficient way to forcefully add things to reader-writer is using chunk iteration method `GetArchetypeChunkComponentType<T>(isReadOnly : false/true)`, it adds the just type dependency but does not affect the system's update condition. If you use get CG I think after that when the world does not have Dat_Test your system's Update will not run?

    This one I meant the handle is not just returned as a part of the deps (which it will not arrive in the system you hope to receive), you also keep it. By being specific you don't need to add any fake dependency. Also you will minimally complete only this one job. With fake dependency it completes any other JCS which has a job that works on that type that updates earlier.

    Code (CSharp):
    1. private JobHandle latestJob;
    2. //Call from any other system that does not know about the queue
    3. public void CompleteLatestJob() => latestJob.Complete();
    4.  
    5. protected override JobHandle OnUpdate(JobHandle inputDeps) {
    6.     Debug.Log("First Update");
    7.     latestJob = new Job {
    8.         _entsMT = sysSecond._ents.ToConcurrent()
    9.     }.Schedule(this, inputDeps);
    10.  
    11.     //FIXME: Unity ECS bug? returning job as dependency won't end up as inputDeps in Sys_Second...
    12.     //This works:
    13.     //job.Complete();
    14.     //return new JobHandle();
    15.  
    16.     //This doesn't work as expected
    17.     return latestJob;
    18. }
     
  6. iam2bam

    iam2bam

    Joined:
    Mar 9, 2016
    Posts:
    36
    Good to know that, thanks.
    It will update because of the attribute [AlwaysUpdateSystem] that I intend to set for any generative system.

    Now I get it. It makes sense but if more than one system depends on its completion it might get messy later on. I rather just do a bogus call to set data dependency.

    I hope later on injecting a system will be considered also for dependency relationships.