Use array of threads

Question

I'm new to threads so it might be an easy one for you, but I've spent some hours trying to figure it out.

Let's say I have a function

public double Gain(List<int> lRelevantObsIndex, ushort uRelevantAttribute)

which needs some time to finish, but is a read only func.

I have an array of ushort[] values, and I want to get the ushort value that achieves the minimum value of the Gain function.

Here is what I've got so far, but it's not working:

lRelevantObsIndex is a read only index.

lRelevantAttributes is the list of ushort values.

        //Initialize the threads
        double[] aGains = new double[lRelevantAttributes.Count];
        Thread[] aThreads = new Thread[lRelevantAttributes.Count];
        for (int i = 0; i < lRelevantAttributes.Count; i++)
        {
            aThreads[i] = new Thread(() => aGains[i] = Gain(lRelevantObsIndex, lRelevantAttributes[i]));
            aThreads[i].Start();
        }

        //Join the threads
        for (int i = 0; i < lRelevantAttributes.Count; i++)
            aThreads[i].Join();

        //The easy part - find the minimum once all threads are done
        ushort uResult = 0;
        double dMinGain = UInt16.MaxValue;
        for (int i = 0; i < lRelevantAttributes.Count; i++)
        {
            if (aGains[i] < dMinGain)
            {
                dMinGain = aGains[i];
                uResult = lRelevantAttributes[i];
            }
        }

        return uResult;

I know this is a simple multithreading question - but still need your brains since I'm new to this.

Can you be more specific when you say it's "not working"? In what way is it not working? What is the behavior? — TypeIA, Jan 02 '14 at 16:57
@dvnrrs, sure, it throws an exception in the first for loop. it's trying to access array[i] and i is set to 30. I guess it happens because of bad use of threads. — Mockingbird, Jan 02 '14 at 17:02
OK, yep, that is the result of the "access to modified closure" problem described in the answers below. — TypeIA, Jan 02 '14 at 17:04
What you actually need is more like `Parallel.ForEach(lRelevantAttributes, ...)`. I doubt that creating separate threads is economical here (did you measure anything?). It certainly is the hard way to do it. — H H, Jan 02 '14 at 17:56

score 4 · Answer 1 · edited May 23 '17 at 12:03

This one is somewhat tricky: your for loop uses a modified value here (a so-called access to modified closure)

for (int i = 0; i < lRelevantAttributes.Count; i++)
{
    aThreads[i] = new Thread(() => aGains[i] = Gain(lRelevantObsIndex, lRelevantAttributes[i]));
    aThreads[i].Start();
}

At the time the thread starts, i will be different in your lambda, accessing a wrong item. Modify your loop as follows:

for (int ii = 0; ii < lRelevantAttributes.Count; ii++)
{
    var i = ii; // Now i is a temporary inside the loop, so its value will be captured instead
    aThreads[i] = new Thread(() => aGains[i] = Gain(lRelevantObsIndex, lRelevantAttributes[i]));
    aThreads[i].Start();
}

This will fix the problem, because lambdas will capture the current value of the temporary variable i on each iteration of the loop.

This is exactly what the problem is; I've hit this one more than I care to admit. Remember folks - lambdas aren't evaluated in your loops, they close over any variables and are evaluated when you actually invoke them; in this case, on some set of background threads. — antiduh, Jan 02 '14 at 17:03

score 2 · Accepted Answer · answered Jan 02 '14 at 16:59

I'm not sure if this is your problem, but it is a problem:

for (int i = 0; i < lRelevantAttributes.Count; i++)
{
    aThreads[i] = new Thread(() => aGains[i] = Gain(lRelevantObsIndex, lRelevantAttributes[i]));
    aThreads[i].Start();
}

When a lambda refers to a loop variable, the binding is delayed, so that when your lambda actually runs, it takes the value of i at the time the lambda runs, not the value it had when the lambda was created. To fix this, declare a secondary variable inside the loop, and use that in the lambda:

for (int i = 0; i < lRelevantAttributes.Count; i++)
{
    int j = i;
    aThreads[i] = new Thread(() => aGains[j] = Gain(lRelevantObsIndex, lRelevantAttributes[j]));
    aThreads[i].Start();
}

GSerjo · Answer 3 · 2014-01-02T23:17:09.667

You can do the same on Task

    [Fact]
    public void Test()
    {
        List<Task<int>> tasks = Enumerable.Range(0, 5) //- it's equivalent how many threads
                                          .Select(x => Task.Run(() => DoWork(x)))
                                          .ToList();
        int[] result = Task.WhenAll(tasks).Result; //- Join threads

        result.ToList().ForEach(Console.WriteLine);
    }

    private int DoWork(int taskId)
    {
        return taskId;
    }

Result output:

Use array of threads

3 Answers3