Search Unity

  1. Welcome to the Unity Forums! Please take the time to read our Code of Conduct to familiarize yourself with the forum rules and how to post constructively.
  2. Dismiss Notice

Curious if there is some way to fix this pattern

Discussion in 'Burst' started by Carpet_Head, Jun 6, 2022.

  1. Carpet_Head

    Carpet_Head

    Joined:
    Nov 27, 2014
    Posts:
    254
    You are only allowed to schedule a job if all native container fields are populated at the start of the job, however allocating on the main thread is slower than Allocator.Temp

    For this reason, Temp allocations must be made inside the job, however that means you have to pass them as an explicit argument to every single function that needs to access them. When you have a lot of data structures going around, this can get pretty tedious and hard to read. It's quite nice to have them as member variables of the job itself

    The quick solution is to wrap the real job in a parent job, and then allocate that memory in the parent job

    Code (CSharp):
    1.     [BurstCompile]
    2.     public struct ComputeVerticesJobWrapper : IJob
    3.     {
    4.         [ReadOnly]
    5.         private readonly NativeArray<float3> inputPoints;
    6.  
    7.         private readonly NativeHashMapList<int, foo> faces;
    8.  
    9.         private readonly float epsilon;
    10.  
    11.         public ComputeVerticesJobWrapper(NativeArray<float3> inputVertices, NativeHashMapList<int, foo> faces, float epsilonMultiplier = 1f)
    12.         {
    13.             inputPoints = inputVertices;
    14.             this.faces = faces;
    15.             epsilon = 0.0001f * epsilonMultiplier;
    16.         }
    17.  
    18.         public void Execute()
    19.         {
    20.             int numVertices = inputPoints.Length;
    21.  
    22.             NativeList<bool> litFaces = new NativeList<bool>(3 * numVertices / 2, Allocator.Temp);
    23.             NativeList<foobar> horizon = new NativeList<foobar>(numVertices, Allocator.Temp);
    24.             NativeList<foo> openSet = new NativeList<foo>(numVertices, Allocator.Temp);
    25.  
    26.             ComputeVerticesJob computeVerticesJob = new ComputeVerticesJob(inputPoints, faces, openSet, litFaces, horizon, epsilon);
    27.             computeVerticesJob.Execute();
    28.         }
    29.     }
    I wonder if it would be possible to have some way to allow us to initialise these at the start of job execution rather than before the job. For example

    [AllocatedAtJobBegin]
    private NativeList<foobar> horizon;
     
  2. vectorized-runner

    vectorized-runner

    Joined:
    Jan 22, 2018
    Posts:
    383
    Allocations shouldn't be the bottleneck though, why do you think it's slow?
     
  3. Carpet_Head

    Carpet_Head

    Joined:
    Nov 27, 2014
    Posts:
    254
    because I profiled it and the allocations are a bottleneck. We have to start many thousands of jobs based on users input

    Temp is a faster allocator than TempJob as it is per-thread
     
  4. R2-RT

    R2-RT

    Joined:
    May 8, 2019
    Posts:
    38
    I recall the saying "the fastest code is the code you don't execute": can't you make persistent allocation instead? So you don't pay for `Temp`/`TempJob` allocations at all?

    About the title question: only thing you can do is to switch to `UnsafeList<T>` (via NativeList<T>.GetUnsafeList()), which is not checked by jobs debugger for resources violations. It still does boundary checks etc. But obviously, it is less safe than `NativeList` (what can be acceptable for sake of performance).
     
  5. Carpet_Head

    Carpet_Head

    Joined:
    Nov 27, 2014
    Posts:
    254
    persistent allocations are extremely slow, so we can't use that. Unsafe could work, I suppose - though those are definitely by definition unsafe
     
  6. Per-Morten

    Per-Morten

    Joined:
    Aug 23, 2019
    Posts:
    109
    I think the point was to allocate memory beforehand and reuse that, rather than allocating memory all the time in the jobs themselves.
     
  7. calabi

    calabi

    Joined:
    Oct 29, 2009
    Posts:
    232
    I was struggling with this before but I found if you do this in the job struct then you can allocate the memory inside the execute method and it works fine.

    Code (CSharp):
    1.  
    2.         [NativeDisableContainerSafetyRestriction]
    3.         private NativeArray<CellData> PotentialMoveCells;
     
  8. DragonCoder

    DragonCoder

    Joined:
    Jul 3, 2015
    Posts:
    1,459
    Wonder what exactly the safety is that you loose for this particular usecase.
     
  9. calabi

    calabi

    Joined:
    Oct 29, 2009
    Posts:
    232
    My guess is none if your using that array exclusively in that job and and thread. The safety checks as far as I'm aware are for race conditions and there's no problems with that for this case.
     
    DragonCoder likes this.
  10. DragonCoder

    DragonCoder

    Joined:
    Jul 3, 2015
    Posts:
    1,459
    Race conditions and aliasing, from what I know too. So technically that should be a good solution.
     
  11. Carpet_Head

    Carpet_Head

    Joined:
    Nov 27, 2014
    Posts:
    254
    Hmm, I thought this causes you to lose burst's ability to assume things don't alias causing it to be slower