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
– TypeIA, Commented Jan 2, 2014 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
– Mockingbird, Commented Jan 2, 2014 at 17:02
OK, yep, that is the result of the "access to modified closure" problem described in the answers below. — TypeIA
– TypeIA, Commented Jan 2, 2014 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. — Henk Holterman
– Henk Holterman, Commented Jan 2, 2014 at 17:56

Community · Accepted Answer · 2017-05-23 12:03:44Z

4

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.

edited May 23, 2017 at 12:03

CommunityBot

11 silver badge

answered Jan 2, 2014 at 16:59

Sergey Kalinichenko

729k85 gold badges1.2k silver badges1.6k bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

antiduh Over a year ago

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.

TypeIA · Accepted Answer · 2014-01-02 16:59:56Z

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 · Accepted Answer · 2014-01-02 23:17:09Z

1

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:

edited Jan 2, 2014 at 23:17

answered Jan 2, 2014 at 17:13

GSerjo

4,7971 gold badge39 silver badges57 bronze badges

Collectives™ on Stack Overflow

Use array of threads

3 Answers 3

1 Comment

Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

1 Comment

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related