Search Unity

Multi threading code screwing up

Discussion in 'Scripting' started by AleksandarCic, Apr 8, 2021.

  1. AleksandarCic

    AleksandarCic

    Joined:
    Sep 5, 2020
    Posts:
    9
    Hello. I've recenetly tried making a multi threading system without using the job system since it's probably dead at this point so a custom system whould be better suited. So ive tried implementing it into some script just for testing purposes and it has been writing wierd values for some reason. Even after implementing a lock system it still screwed up.

    Code (CSharp):
    1.  
    2. private object locker = new object();
    3. private float[,] heights;
    4. //has some initialisation code i'll try to keep this to a minimum
    5.     public void GenerateNoiseLine(int z)
    6.     {
    7.         for (int x = 0; x < hmwidth; x++)
    8.         {
    9.             lock (locker)
    10.             {
    11.                 try
    12.                 {
    13.                     heights[x, z] = noiseHeight;
    14.                 }
    15.                 catch
    16.                 {
    17.                     print(x + " " + z);
    18.                 }
    19.  
    20.             }
    21.         }
    22. //this function is being ran on a thread and is given the argument by some manager function
    23.  
    24.     }
    25.  

    For some reason sometimes when this function is being ran it just bugs out and everything that is writes in the heights array just turns out to be float.max. I think the problem lays in the fact these threads are probably not in sync and override each others results,but having a lock should fix it but here it doesnt.
    Is there any way to prevent these/this error/errors?
     
  2. Yoreki

    Yoreki

    Joined:
    Apr 10, 2019
    Posts:
    2,605
    Are you making sure that all of your threads running this code use the same locker object? It is also worth mentioning that the lock wont work when an exception occurs, but since you print something in the catch clause i think youd notice that.

    Generally i do not believe the problem lies with the lock tho. You effectively told us that sometimes the values you put into your array (noiseHeight) are too high. So id first check these values.
    Even if there was a concurrency problem in the code sniplet shown, the resulting values should rather look random, or in case you are adding or subtracting something, they would seem some amount off. This is the case since on a memory level reading and writing from memory are atomic, but a line of code usually consists of at least reading something, and something else, then calculating something, and putting it somewhere else. Even a small line of code is thus, usually, not atomic, meaning it can happen that the active thread is switched mid-execution. This leads to some threads potentially working with one or more outdated values, which they then write back to memory after their calculation is completed. But given all that i cant really imagine how they would consistantly arrive at float.max for the entire array.

    Edit: Unless multiple threads write to the same [x, y], you should not need a lock at all. And if you want to avoid writing to the same [x, y]'s on different threads, just assign them a number and make them only calculate the value if x % threadcount == id, or something along those lines.

    Also:
    Why would you think that? Especially utilising DOTS, Jobs are fast, safe and mostly easy to work with. Not to mention you can expect them to run several factors faster than your own system when utilizing the burst compiler, if speed is a concern (and when using multi-threading, it is).
     
    Last edited: Apr 8, 2021
    PraetorBlue likes this.
  3. AleksandarCic

    AleksandarCic

    Joined:
    Sep 5, 2020
    Posts:
    9
    I removed the locks and what i orignaly forgot to mention is that i had another part of the code that checked if it was the new maximum/minimum value so i can do a reverse lerp just in case the values are a bit off.

    Anyway the values still bug out and its always something diffrent but always the same one-three threads.
     
  4. PraetorBlue

    PraetorBlue

    Joined:
    Dec 13, 2012
    Posts:
    7,909
    Where are you getting the impression that the job system is dead? It's very much being actively developed.

    Anyway your lock effectively does nothing. It prevents multiple threads from getting in there and writing at the same time, but it doesn't really matter because your threads are not doing anything like a read/increment/write. They're just writing blindly. So all your lock does is make your threads form an orderly queue to do their work. In fact this style of multithreading is not going to gain you any performance boost over simply doing this work in a single thread.

    I think you should revisit the job system, as it will guide you in the right direction.
     
    Joe-Censored likes this.