Wrote my first JOB to detect closest enemies for each individual agent (200+)

Hi team,

I have 200+ agents at a time, 100 per each time. Each agent needs a list of the ten closest enemies to it. I figured that a job that iterated through a loop of all agents, filter them, and take a Vector3 distance would work. I believe it is working, but as per my profiler, it’s taking 340ms on each 6 of my threads. It bottlenecks when I do this, and I think I’m taking 4 frames to do this.

Can anyone provide feedback to me on my code? It’s my first one and I THINK I have it working?


public class tickManager : MonoBehaviour
{
    public static tickManager instance;

    private List<Controller> agentsSearchingForTargetsThisFrame = new List<Controller>();
    private List<structTargettingJob> listStructTargettingJob  = new List<structTargettingJob>();
    structTargettingJob tempTargettingJob = new structTargettingJob();
    private int counter;
    private int totalCount;

    //Arrays for the Targetting Job
    public float[] arrayTargettingRadius;
    public enumTeams[] arrayTeam;
    public Vector3[] arrayTargetterPosition;
    public int[] arrayPotentialTargetsIndex;
    public Vector3[] arrayGameObjectPositions;
    public enumTeams[] arrayTeamsOfPotentialTargets;

    JobHandle jobHandleTargetting;
    private bool targettingJobRunning = false;
    private int targettingJobFrameCounter;



    void Start()
    {
        initSingleton();
        initTargettingJobArrays();
    }

    private void initTargettingJobArrays()
    {
        totalCount = battleManager.instance.arrayAllTroopsInBattle.Length;
        
        arrayTargettingRadius = new float[totalCount];
        arrayTeam = new enumTeams[totalCount];
        arrayTargetterPosition = new Vector3[totalCount];
        arrayPotentialTargetsIndex = new int[totalCount];
        arrayGameObjectPositions = new Vector3[totalCount];
  
    }

    private void Update()
    {
        //Targetting - if job isn't running, increment timers and add people for new job every x seconds.
        if (!targettingJobRunning)
        {
            addTargetScansForThisFrameAndPrepJob();
            targetScanThisFrame();
            removeTargetScansForThisFrame();
        }
        else   //Since the job is running, max it out at 4 frames since using tempjob allocation
        {
            targettingJobFrameCounter++;
            if(targettingJobFrameCounter >=4)
            {
                jobHandleTargetting.Complete();
                targettingJobRunning = false;
                targettingJobFrameCounter = 0;

                //Now to assign them to each Controller

                for(int i = 0; i < totalCount; i++)
                {
                    //go through each game object and update their own arrays of ten closest enemies, etc. TBD.


                }
            }
        }
       

    }

   

    



    //struct used in the targetting job
    struct jobStructIndexToCalculatedTargets
    {
        public int TargetterUnitIndex;
        public NativeArray<(float, int)> ArrayCalculatedTargets;// unit, list of distances and other units
    }

    
    struct structTargettingJob : IJobParallelFor
    {
        [DeallocateOnJobCompletion]
        [ReadOnly] public NativeArray<float> jobArrayTargettingRadius;
        [DeallocateOnJobCompletion]
        [ReadOnly] public NativeArray<enumTeams> jobArrayTeam;
        [DeallocateOnJobCompletion]
        [ReadOnly] public NativeArray<Vector3> jobArrayTargetterPosition;
        [DeallocateOnJobCompletion]
        [ReadOnly] public NativeArray<int> jobArrayPotentialTargetsIndex;
        [DeallocateOnJobCompletion]
        [ReadOnly] public NativeArray<Vector3> jobArrayGameObjectPositions;
        [DeallocateOnJobCompletion]
        [ReadOnly] public NativeArray<enumTeams> jobArrayTeamsOfPotentialTargets;
         
        public jobStructIndexToCalculatedTargets structTargetterToTargets;
        public float tempDistance;
        public int counterCalculatedTargets;

        

        public void Execute(int i)
        {
            //Goes through everything and builds the index arrays of distances to each object by index, within range.  Unsorted.
            for(int j = 0; j < jobArrayPotentialTargetsIndex.Length; j++)
            {
                
                //Inner loop
                counterCalculatedTargets = 0;
                
                for (int k = 0; k < jobArrayPotentialTargetsIndex.Length; k++)  //J is 'me', K is 'others'
                {

                    //If this is me or my team, skip
                    if (jobArrayPotentialTargetsIndex[j] == k || jobArrayTeamsOfPotentialTargets[j] == jobArrayTeamsOfPotentialTargets[k]) continue;
                    
                    tempDistance = Vector3.Distance(jobArrayGameObjectPositions[k], jobArrayTargetterPosition[j]);
                
                    //If outside my targetting range? skip
                    if (tempDistance > jobArrayTargettingRadius[j]) continue;

                    //Make a list of closest
                    structTargetterToTargets.TargetterUnitIndex = j;
                    structTargetterToTargets.ArrayCalculatedTargets[counterCalculatedTargets] = (tempDistance, k);
                    counterCalculatedTargets++;
                }
                //Assign values to the J index of closest targets in range, and sort
                structTargetterToTargets.ArrayCalculatedTargets.Sort();
            }
            

        }

    }

    private void addTargetScansForThisFrameAndPrepJob()
    {
        //Debug.Log("entering addTargetSCansForThisFrame");
        counter = 0;
        listStructTargettingJob.Clear();
        

        //Go through all living or dead agents in the scene
        for (int i = 0; i < totalCount; i++)
        {
        
            //if dead skip
            if (battleManager.instance.arrayAllTroopsInBattle[i].healthstruct.health <= 0) 
                continue;

            //increment timer
            battleManager.instance.arrayAllTroopsInBattle[i].targetting.timeSinceLastTargettingScan += Time.deltaTime;
            
            //Build the arrays of information that I will pass into the Job
            if(battleManager.instance.arrayAllTroopsInBattle[i].targetting.timeSinceLastTargettingScan >= battleManager.instance.arrayAllTroopsInBattle[i].targetting.targettingRate) 
            {

                arrayTargettingRadius[counter] = battleManager.instance.arrayAllTroopsInBattle[counter].targetting.maxTargetDistance;
                arrayTeam[counter] = battleManager.instance.arrayAllTroopsInBattle[counter].team;
                arrayTargetterPosition[counter] = battleManager.instance.arrayAllTroopsInBattle[counter].transform.root.position;
                arrayPotentialTargetsIndex[counter] = counter;
                
                agentsSearchingForTargetsThisFrame.Add(battleManager.instance.arrayAllTroopsInBattle[i]);
                battleManager.instance.arrayAllTroopsInBattle[i].targetting.timeSinceLastTargettingScan = 0;
                
                counter++;
            }
         }

        //Build position array out of loops
        for (int j = 0; j < counter; j++)
        {
            arrayGameObjectPositions[j] = battleManager.instance.arrayAllTroopsInBattle[j].transform.root.position;
        }
    }

    private void removeTargetScansForThisFrame()
    {
        agentsSearchingForTargetsThisFrame.Clear();
    }
    private void targetScanThisFrame()
    {
        //CREATE JOB
        if(agentsSearchingForTargetsThisFrame.Count > 0)
        {
            Debug.Log("PING!: " + agentsSearchingForTargetsThisFrame.Count);
            scheduleJobFindNearestTargets();
        }
    }


    private void scheduleJobFindNearestTargets()
    {
        //Initialize the job data

        tempTargettingJob = new structTargettingJob() 
        {

            jobArrayTargettingRadius = new NativeArray<float>(totalCount, Allocator.TempJob),
            jobArrayTeam = new NativeArray<enumTeams>(totalCount, Allocator.TempJob),
            jobArrayTargetterPosition = new NativeArray<Vector3>(totalCount,Allocator.TempJob),
            jobArrayPotentialTargetsIndex = new NativeArray<int>(totalCount,Allocator.TempJob),
            jobArrayGameObjectPositions = new NativeArray<Vector3>(totalCount, Allocator.TempJob),
            jobArrayTeamsOfPotentialTargets = new NativeArray<enumTeams>(totalCount, Allocator.TempJob),
            structTargetterToTargets = new jobStructIndexToCalculatedTargets()
                {
                    TargetterUnitIndex = 0,
                    ArrayCalculatedTargets = new NativeArray<(float, int)>(totalCount, Allocator.TempJob),
                }
        };


        //Build the arrays for the job
        for (int i = 0; i > agentsSearchingForTargetsThisFrame.Count; i++) {
            
            tempTargettingJob.jobArrayTargettingRadius[i] = arrayTargettingRadius[i];
            tempTargettingJob.jobArrayTeam[i] = arrayTeam[i];
            tempTargettingJob.jobArrayTargetterPosition[i] = arrayTargetterPosition[i];
            tempTargettingJob.jobArrayPotentialTargetsIndex[i] = arrayPotentialTargetsIndex[i];
            tempTargettingJob.jobArrayGameObjectPositions[i] = arrayGameObjectPositions[i];
            tempTargettingJob.jobArrayTeamsOfPotentialTargets[i] = arrayTeamsOfPotentialTargets[i];

        }

        
        jobHandleTargetting= new JobHandle();
        jobHandleTargetting = tempTargettingJob.Schedule(agentsSearchingForTargetsThisFrame.Count, 1);
        targettingJobRunning = true;
        

    }

    
    private void initSingleton()
    {
        if (instance == null)
        {
            instance = this;
        }
        else if (instance != this)
        {
            Destroy(this);
        }

        DontDestroyOnLoad(this);
    }

}

Initial thoughts on the job struct:

  1. No [BurstCompile]?
  2. What does i do?
  3. I don’t think your code is working, because line 132 is a safety violation. If it isn’t flagging the system, then that line is never being run.
1 Like

Too much code.

200 agents should perform that task in single threaded C# within a frame. Looking at the verbosity of that code I‘d say you should first simplify and clean it up.

You are probably wasting a lot of performance just on repeatedly accessing the same instance and the same array and possibly even the same transform again and again. And I bet this is also why you don‘t use BurstCompile - it requires you to restrict the Jobs code to value types only. No instance, no transform, no managed C# types at all.

So without going into specifics, I‘d say you need to rewrite this from scratch in an organized manner. All the data the job works on belongs inside each job, rather than reaching out to some “instance”.

Compare your code (cleaned up a little):

private void addTargetScansForThisFrameAndPrepJob()
{
	counter = 0;
	listStructTargettingJob.Clear();

	for (int i = 0; i < totalCount; i++)
	{
		if (battleManager.instance.arrayAllTroopsInBattle[i].healthstruct.health <= 0) 
			continue;

		battleManager.instance.arrayAllTroopsInBattle[i].targetting.timeSinceLastTargettingScan += Time.deltaTime;
		
		if(battleManager.instance.arrayAllTroopsInBattle[i].targetting.timeSinceLastTargettingScan >= battleManager.instance.arrayAllTroopsInBattle[i].targetting.targettingRate) 
		{
			arrayTargettingRadius[counter] = battleManager.instance.arrayAllTroopsInBattle[counter].targetting.maxTargetDistance;
			arrayTeam[counter] = battleManager.instance.arrayAllTroopsInBattle[counter].team;
			arrayTargetterPosition[counter] = battleManager.instance.arrayAllTroopsInBattle[counter].transform.root.position;
			arrayPotentialTargetsIndex[counter] = counter;
			
			agentsSearchingForTargetsThisFrame.Add(battleManager.instance.arrayAllTroopsInBattle[i]);
			battleManager.instance.arrayAllTroopsInBattle[i].targetting.timeSinceLastTargettingScan = 0;
			
			counter++;
		}
	 }

	for (int j = 0; j < counter; j++)
		arrayGameObjectPositions[j] = battleManager.instance.arrayAllTroopsInBattle[j].transform.root.position;
}

With the same code just applying DRY principle (ie good style, readable, maintainable):

private void addTargetScansForThisFrameAndPrepJob()
{
	counter = 0;
	listStructTargettingJob.Clear();
	var allTroopsInBattle = battleManager.instance.arrayAllTroopsInBattle;

	for (int i = 0; i < totalCount; i++)
	{
		var theTroop = allTroopsInBattle[i];
		if (theTroop.healthstruct.health <= 0) 
			continue;

		theTroop.targetting.timeSinceLastTargettingScan += Time.deltaTime;
		
		if(theTroop.targetting.timeSinceLastTargettingScan >= theTroop.targetting.targettingRate) 
		{
			var counterTroop = allTroopsInBattle[counter];
			arrayTargettingRadius[counter] = counterTroop.targetting.maxTargetDistance;
			arrayTeam[counter] = counterTroop.team;
			arrayTargetterPosition[counter] = counterTroop.transform.root.position;
			arrayPotentialTargetsIndex[counter] = counter;
			
			agentsSearchingForTargetsThisFrame.Add(theTroop);
			theTroop.targetting.timeSinceLastTargettingScan = 0;
			
			// write-back struct
			allTroopsInBattle[counter] = counterTroop;
			
			counter++;
		}
		
		// write-back struct
		allTroopsInBattle[i] = theTroop;
	 }

	for (int j = 0; j < counter; j++)
		arrayGameObjectPositions[j] = allTroopsInBattle[j].transform.root.position;
}

This may also be faster, too, depending on whether the compiler doesn’t actually realize that you’re accessing the same thing over and over again.

1 Like

Why not use “i” in Execute?

1 Like

Hi! Thanks for replying and going through my mess here!

  1. I have a tuple in there sooo yeah. It didn’t like it for burst. I’m going to have to figure a way to keep track of things without it.

2). Nothing because I didn’t use it because I want sure what it did… But looking at it now looks like I’m adding a for loop for nothing since I can use i.

3). Yeah, I’ll have to find a way to track the game objects index in a list after it’s sorted by distance, without using a tuple. Maybe Instead of an index of a master array of game objects to track, I can track by the GUID of the object Instead…

Thanks, lots of good food for thought!

Thank you for taking time to go through my mess here and your thoughtful and kind response!! Thank you!

This makes sense! So making a copy locally is better than accessing a main copy of it from a Singleton elsewhere. I’m a relative beginner so I was following the “declaring unnecessary variables is bad and just reuse whatever you have”, but obviously I don’t know enough to understand when and when not to. Thank you! I’ll try this out and see where I can simplify my logic!

I’m obviously a beginner so I thank you for taking the time to help me learn!

Heh because I don’t know what I’m doing lol

I’m definitely adding an unnecessary for loop on there.

Thanks for going through my mess here and taking time out of your day! I’ll definitely use i! Thank you!

This also stands out. You make an array of positions despite already having an array of troops which give you those positions.

Also, stop naming arrays with “array” in the name. It’s bad practice because maybe sometime in the future it’ll be a List or a HashSet or something else. Same reason we don’t add types to any variable. Would be awful to have vector3Position and quaternionRotation or floatTimeOut or intIndex. :wink:

1 Like

Good point! Heh I thought I was making easier for myself so I know if I have a map or a set or whatever in the name but obviously not! I can get that anyway easily enough. Thanks for the tips here!

This is definitely a great exercise. Standard monobehavioir approaches are very forgiving, and this is really teaching me how to properly do things. Thank you again!

1 Like

Make it an explicit struct. Then it will be fine.

While I agree with pretty much everything else said here, this one in particular I disagree with. In DOD, you are often working with a mix of a collection of things and single things. And I’ve found specifiers like this can be a great way to help keep those things straight. Also in DOD, you will often find yourself with a lot more variables to keep track of, so naming things that makes the most sense to you should come at a higher priority than anything else. If you change the type, you change the name. IDEs have tools for this.

Yeah but IDEs also show you the type!

At least the good ones do! :wink:
image

I turn off those particular annotations, so I’d have to hover to see the type. I don’t like non-code being displayed in-between code. Plus it messes with my quick-parsing I do with vertical alignment. My point is that everyone has different ways of working, and that particular practice I don’t think should be blindly adopted, especially if it goes against initial intuition.

1 Like

Hi team! Thanks for your help so far. I have a fundamental question which I think is causing much of my confusion.

So I currently have it set up so that I have one job. That one job will iterate through a list of all game objects and calculate for each game object the distance between it and all others. Basically two loops.

The fundamental question is is that I’m not sure in terms of how the job system assigns it to other threads where I should have it as one job and the system is smart enough to know which parts of that single job,where it has a bunch of loops, it will assign to other threads.

Or since there’s only one job, there’s only one job to do so there’s no multi-threading since individual jobs are what’s divided amongst worker threads.

I do see all threads working and doing the job at the 350ms - since one job are they splitting it or doing the same thing?

I hope that makes sense. I’m barely able to understand it as it is lol

OverlapSphere (or OverlapCircle if working in 2D) may be more performant than jobs. Using OverlapSphere means you can also place each team on its own layer so then it only needs to return the relevant opposing entities to further process/filter. So basically you’ll be using the physics engine and its spatial partitioning to help you with the filtering.

1 Like

For an IJobParallelFor.Schedule (and IJobFor.ScheduleParallel which you probably want to use instead), it uses the concept of work stealing. When you schedule the job, you pass in the total number of elements (the number of times Execute gets called) as well as an innerLoopBatchCount. The latter determines how many elements each thread takes before looking to grab more work to do.

So I started with overlapsherecommand, but it doesn’t give you any sort of distance information, just what’s in the bubble, so on top of the physics command, I’d have to sort things anyway.

If I’m sorting things anyway, I may as well do it this way, I figured. Maybe not for the best lol.

Also I’m using this as an exercise to learn HOW to do jobs. Then yeah I’ll profile and see what’s better, but for now, yeah I’m learning a lot!

Interesting. Ok I think I get that! I got lots of work to do tomorrow night (can’t tonight sadly).

Thank you for explaining it!

So to confirm: one job that basically is a giant for loop, and I tell the threads how many loops each either should take before getting more?

I think I got it if so! Thanks!

Ignoring technicalities, I think you grasped the concept.

1 Like

LOL pretty sure that was on my report card every year

Thanks for the help here, already I have great things to implement to make this better, and I understand why! Which is the really thing here.

Happy weekend!!

1 Like