Please help with my conversion of this IJob to IJobParallelFor

Hey all,

I’ve got this hierarchy system where entities reference a parent entity that has a Position component, and then set their own Position component based on that. But I’m having trouble turning the job into a parallel one.

using System.Collections;
using System.Collections.Generic;
using Unity.Burst;
using Unity.Collections;
using Unity.Entities;
using Unity.Jobs;
using Unity.Mathematics;
using Unity.Transforms;
using UnityEngine;

public class HierarchyPositionSystem : JobComponentSystem
{
    struct HierarchyPositionGroup
    {
        [ReadOnly] public ComponentDataArray<TransformParent> parents;
        [ReadOnly] public ComponentDataArray<LocalRotation> localRotations;
        [ReadOnly] public ComponentDataArray<RotationSpring> springs;
        [ReadOnly] public EntityArray entities;
        public ComponentDataArray<Position> positions;
        public ComponentDataArray<Rotation> rotations;
        public ComponentDataArray<PreviousPosition> prevPositions;
        public int Length;
    }

    [Inject] private HierarchyPositionGroup hierarchyDataGroup;
    [Inject] ComponentDataFromEntity<Position> positionGroup;
    [Inject] ComponentDataFromEntity<Rotation> rotationGroup;

    [BurstCompile]
    struct HierarchyPositionJob : IJobParallelFor
    {
        [ReadOnly] public ComponentDataFromEntity<Position> allPositions;
        [ReadOnly] public ComponentDataFromEntity<Rotation> allRotations;
        [ReadOnly] public ComponentDataArray<TransformParent> parents;
        [ReadOnly] public ComponentDataArray<LocalRotation> localRotations;
        [ReadOnly] public ComponentDataArray<RotationSpring> springs;
        [ReadOnly] public EntityArray entities;

        public ComponentDataArray<Position> positions;
        public ComponentDataArray<Rotation> rotations;
        public ComponentDataArray<PreviousPosition> prevPositions;

        public void Execute(int i)
        {
            var childEntity = entities[i];
            var parentEntity = parents[i].Value;
            var parentPosition = allPositions[parentEntity].Value;
            Quaternion parentRotation = allRotations[parentEntity].Value;
            Quaternion childRotation = localRotations[i].Value;
            Vector3 anchor = Vector3.up;
            Vector3 offset = springs[i].position;
            Quaternion offsetQ = Quaternion.FromToRotation(anchor, offset);
            childRotation *= offsetQ;
            childRotation *= parentRotation;
            rotations[i] = new Rotation
            {
                Value = childRotation
            };
            float3 up = childRotation * Vector3.up;
            prevPositions[i] = new PreviousPosition() { Value = positions[i].Value };
            positions[i] = new Position(parentPosition + up);            
        }
    }

    protected override JobHandle OnUpdate(JobHandle inputDeps)
    {
        var job = new HierarchyPositionJob()
        {
            positions = hierarchyDataGroup.positions,
            prevPositions = hierarchyDataGroup.prevPositions,
            rotations = hierarchyDataGroup.rotations,
            parents = hierarchyDataGroup.parents,
            localRotations = hierarchyDataGroup.localRotations,
            springs = hierarchyDataGroup.springs,
            entities = hierarchyDataGroup.entities,
            allPositions = positionGroup,
            allRotations = rotationGroup
        };
        return job.Schedule(hierarchyDataGroup.Length,32, inputDeps);
    }
}

I got the problem of not being able to write to ComponentDataArray, so I made those read only and figured I could have a separate array of positions related only to the entities in the job. But Unity complains that these two arrays are the same. As far as I know they should be different, right? The ComponentDataArray relates to the entities in this job, and the ComponentDataArrayFromEntity has all the entities in it…?

ComponentDataFromEntity returns you data by entity from same chunk as they located in memory for ComponentDataArray, it’s… indexer by entity from chunks. And technically your positions from “allPositions” is same as positions if you get this by entity from “positions”, it’s race condition, you mark position as readonly and can change that from getting by entity.

I get it, that’s fair enough. Thanks. So probably this job can’t be parallelised?

you can look at what TransformSystem does in the package.

basically it iterates on only the roots in the parallelFor, then updates each tree in its own thread, using [NativeDisableParallelForRestriction] to bypass the safety system.
just be sure you check for possible race conditions.

Unity’s internal transform hierarchy is parallelized by root transform (see the talk [here](http://according to an optimization talk on last Unite), starting at 3:45). So you could try to do a similar approach, schedule one of these jobs per hierarchy.

An alternative would be to do each layer in parallel. So you schedule a parallel job for each of the roots, one for each with one parent, one for each with two, and so on. Then you’d set up dependencies so you run the jobs root down. That’d guarantee the correct results as well. Right now you can end up with executing a child before it’s parent, which will give the wrong result on that frame (unless you’ve sorted your data to fix that).

I assume that Unity’s tried both approaches, and are going with the each-root-in-parallel approach instead of each-layer-in-parallel for good reasons, but it might be worthwhile to try both for your case.

EDIT: Sniped!

you can’t easily filter for layer, unless you add a pre-process step that walks the entire chain for each entity and then sync/modify a ISharedComponentData. that would be heavy.

True.

If the parent/child relationship is relatively stable, it might still be worth it. If it updates often, then it’s a really bad solution, because it has the same problems with internal dependencies as the original problem.

OK, thanks all! Lots to look at here.