Weird for loop counter issue with threading


I am currently implementing a Cellular Automata algorithm for my world. Therefore i first have to create a way to randomly fill my map. Therefore i split up my map and create threads and let each one fill a part of the map. It is contained inside a method, so that’s all that’s necessary.

        void CellularAutomata()
            int ThreadCount = Environment.ProcessorCount; //Amount of cores. 4 in my case
            ThreadCount *= 4; // Multiplying by 4 to get higher amount of threads.
            int ThreadWorkSizeX = WorldProps.Size.x / ThreadCount; //Divide map x size by threadcount
            int ThreadWorkSizeY = WorldProps.Size.y / ThreadCount; // same for y
            Thread[] Threads = new Thread[ThreadCount]; //create simple array
            //Main thread nog mee laten werken
            for (int i = 0; i < ThreadCount; i++) //for loop to create threads
                UnityEngine.Debug.Log(i); // 0
                Threads *= new Thread(() => {*

UnityEngine.Debug.Log("Thread says " + (i)); // i is 1???
//Doing i-1 to correct this, but give unpredictable results.
for (int x = ( (i-1) * ThreadWorkSizeX); x < (( (i-1) * ThreadWorkSizeX) + ThreadWorkSizeX); x++)
for (int y = ( (i-1) * ThreadWorkSizeY); y < (( (i-1) * ThreadWorkSizeY) + ThreadWorkSizeY); y++)
if (WorldProps.RandomGen.Next(0, 100) < CellProps.FillPercentage)
TileTypeXZ[x, y] = 2;
TileTypeXZ[x, y] = 1;
//Inserting a debug.log here affects the results?

//Joining up again with my threads.
//(Yes i know, main thread not really doing anything yet, but i want this working first.)
for (int p = 0; p < ThreadCount; p++)

For some reason i gets incremented by 1 after i create a new Thread object. Anyone knows why?
There are no global variables or anything here. Just what’s in the method.
Also, when i insert a debug.log before starting my thread, it changes the way the map is filled whith random values.
EDIT: don’t mind the dutch comments here and there, their for myself :wink:

I’m still learning threading, but I’m pretty sure what’s happening here is that you’re referencing i, which is a value being incremented in the scope of the loop. All threads would therefore display whatever state i was in when they reported. Try setting a new value to match i and using that instead in the thread.

remember that threads are not always going to operate in order of their looped execution, also, unless you manually set up synchronicity.

I strongly advise reading this 6 part guide to multithreading

That happens when you don’t know what a closure is or when you forgot that little detail ^^. When you create a closure every variable which you use inside the method will be added to an internally created class object (the closure object) so the method can access that variable. Since your for loop continue incrementing “i” the thread will use the same variable i.

That’s because the loop variable of a for loop is declared once before the actual loop. To solve such problems, you should only access real local variables in your closure:

         for (int i = 0; i < ThreadCount; i++) //for loop to create threads
             int tmp = i; // local variable
             Threads *= new Thread(() => {*

UnityEngine.Debug.Log("Thread says " + (tmp));
for (int x = ( (tmp) * ThreadWorkSizeX); x < (( (tmp) * ThreadWorkSizeX) + ThreadWorkSizeX); x++)
This will work as the closure will close around our “tmp” variable which is local to the loop body and you will get a new tmp variable each iteration.
I would recommend to not use closures as thread functions. Closures can very easy produce sharing / concurrent access problems since you don’t have any locking in place.
Another thing that could cause a problem is your global “WorldProps.RandomGen” as all your threads access the same.
[Over here][1] i posted an example how you could manage multiple threading jobs. You should avoid concurrent access to the same variables at all costs.
Btw: Your “thread job” is actually so simple that you most likely won’t gain much (if any) performance. It might even run slower. The thread overhead and the additional coordinate calculations won’t help here ^^. The key to speed is minimizing [cache-misses][2]. Having more (active) threads than actual cpu cores will cause frequent [taskswitches][3] which will also result in a worse overall runtime.
[1]: Unity3D and C# - Coroutines vs threading - Questions & Answers - Unity Discussions
[2]: CPU cache - Wikipedia
[3]: Context switch - Wikipedia