CODEWITHSUNDEEP
c#multithreading
fozylet
fozylet

Reputation: 1289

Tasks in C# problem - argument not passed correctly in a for loop

I have a method where it need to make multiple service calls and consolidate the result. Was trying to use Task for this and the results were not right, so I did the below two tests.

Using Tasks

List<Task<string>> tasks = new List<Task<string>>();
Stopwatch stopwatch = new Stopwatch();
stopwatch.Start();
for (int i = 0; i < 20; i++)
{
    tasks.Add(Task.Factory.StartNew(() =>
        {
            long started = stopwatch.ElapsedMilliseconds;
            Random wait = new Random();
            int waited = wait.Next(500, 3000);
            Thread.Sleep(waited);
            return string.Format("Index #{0} started at {1}ms and waited {2}ms", i, started, waited);
        }));
}

Task.WaitAll(tasks.ToArray());
foreach (Task<string> task in tasks)
{
    Console.WriteLine(string.Format("[Task {0,2}] {1}", task.Id, task.Result));
}

Using Parallel

Stopwatch stopwatch = new Stopwatch();
stopwatch.Start();
int[] inputs = new int[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19 };
Parallel.ForEach(inputs, i =>
{
    long started = stopwatch.ElapsedMilliseconds;
    Random wait = new Random();
    int waited = wait.Next(500, 3000);
    Thread.Sleep(waited);
    string result = string.Format("Task {0,2} started at {1} ms and waited {2} ms", i, started, waited);
    Console.WriteLine(result);
});

stopwatch.Stop();
Console.WriteLine("Total time taken: " + stopwatch.ElapsedMilliseconds.ToString());

As you can see below, the task index gets printed as 20 all the time (though 0 to 19 was passed). Also, Parallel is taking more time than tasks. Below are the results, and obviously am doing something wrong in Tasks and is unable to figure out what :(

Task Output

[Task  1] Index #20 started at 0ms and waited 875ms
[Task  2] Index #20 started at 0ms and waited 875ms
[Task  3] Index #20 started at 0ms and waited 875ms
[Task  4] Index #20 started at 0ms and waited 875ms
[Task  5] Index #20 started at 0ms and waited 875ms
[Task  6] Index #20 started at 0ms and waited 875ms
[Task  7] Index #20 started at 855ms and waited 1477ms
[Task  8] Index #20 started at 886ms and waited 1965ms
[Task  9] Index #20 started at 886ms and waited 1965ms

Parallel Output

Task  6 started at 1046 ms and waited 636 ms
Task 11 started at 1561 ms and waited 758 ms
Task  0 started at 16 ms and waited 2891 ms
Task  5 started at 16 ms and waited 2891 ms
Task 15 started at 17 ms and waited 2891 ms
Task  1 started at 17 ms and waited 2891 ms
Task 10 started at 17 ms and waited 2891 ms

With actual method too I've the same experience where the last item is being returned multiple times rather than individual results.

Would be of great help if you can guide me in the right direction.

Note: The outputs are partial. Actual output has 20 items each.

Upvotes: 3

Views: 1136

Answers (3)

Just another metaprogrammer
Just another metaprogrammer

Reputation: 11577

Parallel is primarily intended to allow you to scale your algorithms to use the available CPU resources on your machine. If the problem isn't CPU bound (ie calling WebServices) Parallel is not the right abstraction.

So in your case it seems Task is the right choice.

Upvotes: 0

Enigmativity
Enigmativity

Reputation: 117175

You need to form a closure over i. Here's the easiest way:

for (int x = 0; x < 20; x++)
{
    var i = x;
    tasks.Add(...) // keep using `i` inside here & never `x`
}

That should fix it.

Upvotes: 2

Radu094
Radu094

Reputation: 28434

When using the Tasks approach, you have a closure there on the 'i' which is why all the tasks IDs come out as 20 (ie. by the time the tasks actually execute, all 20 of them have been scheduled and thus i == 20 when the method executes).

Upvotes: 1

Related Questions