Search Unity

Question How to sort a NativeList which is used in a previous job?

Discussion in 'Entity Component System' started by TJHeuvel-net, Nov 16, 2022.

  1. TJHeuvel-net

    TJHeuvel-net

    Joined:
    Jul 31, 2012
    Posts:
    838
    I have a list of indices i'm filtering, and after that i would like to sort them. Something like:
    Code (CSharp):
    1.     void OnEnable()
    2.     {
    3.         var rng = new Unity.Mathematics.Random(123);
    4.         NativeList<int> indices = new NativeList<int>(10, Allocator.TempJob);
    5.         for (int i = 0; i < 10; i++)
    6.             indices.Add(rng.NextInt(100));
    7.  
    8.         Debug.Log($"Before: {string.Join(',', indices.AsArray())}");
    9.  
    10.         var handle = new FilterOdd().ScheduleFilter(indices, 32);    
    11.         handle = indices.SortJob().Schedule(handle);
    12.  
    13.         handle.Complete();
    14.  
    15.         Debug.Log($"After: {string.Join(',', indices.AsArray())}");
    16.  
    17.         indices.Dispose();
    18.     }
    19.  
    20.  
    21.     struct FilterOdd : IJobParallelForFilter
    22.     {
    23.         public bool Execute(int index) => index % 2 == 0;
    24.     }
    This throws the following error:

    Which i dont really understand, since i pass in the previous handle. The error also doesnt even mention any of the sort jobs.

    When i sort `indices.AsDeferredArray` instead the results simply are invalid, not sorted. It does work when i call complete in between the jobs, but obviously i'd rather not.

    Using the non-job variant, e.g. `indices.Sort` does work as expected.
     
    Last edited: Nov 16, 2022
  2. TJHeuvel-net

    TJHeuvel-net

    Joined:
    Jul 31, 2012
    Posts:
    838
    A kind user on Discord helped me! I'm still a bit puzzled how it manages to do this, but constructing the job *before* filtering does make it work. E.g.:

    Works, but this:
    Doestnt. I'm not sure why, and it makes my code a bit harder to read. Ah well.
     
  3. JesOb

    JesOb

    Joined:
    Sep 3, 2012
    Posts:
    1,109
    It looks like a bug
    do your report it?
     
  4. kdchabuk

    kdchabuk

    Joined:
    Feb 7, 2019
    Posts:
    50
    SortJob calls GetUnsafePtr(), which checks the safety handle at job creation. (Dependencies aren't known until the job is scheduled.) Since the filter job is already scheduled, the safety check throws an error.
     
    TJHeuvel-net likes this.
  5. TJHeuvel-net

    TJHeuvel-net

    Joined:
    Jul 31, 2012
    Posts:
    838
    Thanks for the explanation! Seems like a slightly overzealous safety, but i take those over race conditions any day of the week.
     
    kdchabuk likes this.
  6. kdchabuk

    kdchabuk

    Joined:
    Feb 7, 2019
    Posts:
    50
    Upon another look, I think
    SortJob<T>(this NativeList<T> list)
    with a NativeList is likely to result in bugs, because it also gets list.Length() immediately, at job creation. That's probably not intended, such as if you populate the list inside the first job. Instead, you want a job similar to this:

    Code (CSharp):
    1. public struct SortJob<T> : IJob where T : unmanaged, IComparable<T>
    2. {
    3.     public NativeList<T> list;
    4.  
    5.     public void Execute()
    6.     {
    7.         list.Sort();
    8.     }
    9. }
    edit: However, the built-in struct SortJob is internally parallelized so it is better when you do know the length in advance.

    edit2: In fact,
    Unity.Collections.SortJob<T>(this NativeList<T> list)
    totally bypasses the safety system after the job is created. There is no safety check that a SortJob doesn't overwrite another job, so long as it is created first. I wouldn't use it.
     
    Last edited: Nov 17, 2022
    TJHeuvel-net likes this.
  7. TJHeuvel-net

    TJHeuvel-net

    Joined:
    Jul 31, 2012
    Posts:
    838
    I just came back to ask exactly this, because i fell straight into this trap. Thanks so much!

    I was indeed seeing exactly this behaviour, for example:

    1. Make a list with indices 0..8
    2. Filter odd entries
    3. Sort

    Observe sort is being called for odd, removed, indices.

    Code (csharp):
    1.  
    2. void OnEnable()
    3.    {
    4.        NativeList<int> indices = new NativeList<int>(10, Allocator.TempJob);
    5.      
    6.        for (int i = 0; i < 8; i++)
    7.            indices.Add(i);
    8.        Debug.Log($"Start indices: {string.Join(',', indices.AsArray())}");
    9.        var sortJob = indices.SortJob(new Comparer());
    10.        JobHandle handle = default;
    11.      
    12.        handle = new FilterOddJob().ScheduleFilter(indices, 32);
    13.        handle.Complete();
    14.        Debug.Log($"After filtering: {string.Join(',', indices.AsArray())}");
    15.  
    16.        handle = sortJob.Schedule(handle);      
    17.        handle.Complete();
    18.  
    19.        indices.Dispose();
    20.    }
    21.  
    22.    [BurstCompile]
    23.    struct FilterOddJob : IJobParallelForFilter
    24.    {
    25.        public bool Execute(int index) => index % 2 == 0;
    26.    }
    27.  
    28.    struct Comparer : IComparer<int>
    29.    {
    30.        public int Compare(int x, int y)
    31.        {
    32.            Debug.Log($"Comparing {x} with {y}!");
    33.            return x.CompareTo(y);
    34.        }
    35.    }
    36.  


    I have also reported this as a bug, but perhaps its a new feature request. A DeferredSortJob.
     
    Last edited: Nov 17, 2022
    kdchabuk likes this.
  8. tertle

    tertle

    Joined:
    Jan 25, 2011
    Posts:
    3,759
    Do away with the built in sort job and just schedule your own IJob and pass it the list and call list.AsArray().Sort()
     
    Timboc and hadynlander like this.
  9. iamarugin

    iamarugin

    Joined:
    Dec 17, 2014
    Posts:
    883
    Wasted some time on this to. I don't understand why this was implemented this way. It is so annyoing.
     
    arkano22 and hadynlander like this.