C# Anonymous delegates issue
-
This is well documented, yet I got into the issue and wasted about 1/2 hr. Consider the following program of summing 100000 numbers using 10 processors(threads). An extremely minor modification will slove this problem.
class Program
{
static void Main(string[] args)
{
int n = 100000, p = 10;int\[\] numbers = GenerateInts(n); int\[\] localSums = new int\[p\]; Thread\[\] threads = new Thread\[p\]; for(int i = 0; i < p; i++) { ThreadStart func = delegate { for (int j = (i \* n) / p; j < ((i + 1) \* n) / p; j++) { localSums\[i\] += numbers\[j\]; } }; threads\[i\] = new Thread(func); threads\[i\].Start(); } int sum = 0; for (int i = 0; i < p; i++) { threads\[i\].Join(); sum += localSums\[i\]; } Debug.Assert(sum == (n\*(n+1))/2); } private static int\[\] GenerateInts(int p) { int\[\] nums = new int\[p\]; for (int i = 0; i < p; i++) { nums\[i\] = i + 1; } return nums; }
}
Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it. -Brian Kernighan
-
This is well documented, yet I got into the issue and wasted about 1/2 hr. Consider the following program of summing 100000 numbers using 10 processors(threads). An extremely minor modification will slove this problem.
class Program
{
static void Main(string[] args)
{
int n = 100000, p = 10;int\[\] numbers = GenerateInts(n); int\[\] localSums = new int\[p\]; Thread\[\] threads = new Thread\[p\]; for(int i = 0; i < p; i++) { ThreadStart func = delegate { for (int j = (i \* n) / p; j < ((i + 1) \* n) / p; j++) { localSums\[i\] += numbers\[j\]; } }; threads\[i\] = new Thread(func); threads\[i\].Start(); } int sum = 0; for (int i = 0; i < p; i++) { threads\[i\].Join(); sum += localSums\[i\]; } Debug.Assert(sum == (n\*(n+1))/2); } private static int\[\] GenerateInts(int p) { int\[\] nums = new int\[p\]; for (int i = 0; i < p; i++) { nums\[i\] = i + 1; } return nums; }
}
Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it. -Brian Kernighan
Hi Rama Krishna, The issue in the above code is that, you are directly using the loop counter variable "i" in the anonymous delegate (anonymous method). Since this variable is captured by the anonymous delegate, the C# compiler will create a new class to hold this variable and will create an object of this new class above the for-loop containing the anonymous delegate, and replaces the loop counter variable usage with its instance data member "i". Due to this the same instance data member is used by all the anonymous methods created for the 10 threads. So the results will be pretty unpredictable when the threads start running. Also since the instance data member "i" is used as loop counter variable, by the time the loop ends its value would be 10 instead of 9 (as one would expect). For this reason when the threads actually start executing, they will try to access the 10th index of "localSums" array and the application will throw "IndexOutOfRange" exception. Below is the modified code that runs correctly.
using System;
using System.Collections.Generic;
using System.Text;
using System.Threading;
using System.Diagnostics;namespace TestAnonymous
{
class Program
{
static void Main(string[] args)
{
int n = 100000, p = 10;
int[] numbers = GenerateInts(n);
int[] localSums = new int[p];
Thread[] threads = new Thread[p];
for (int i = 0; i < p; i++)
{
int iTemp = i;
ThreadStart func = delegate
{
for (int j = (iTemp * n) / p; j < ((iTemp + 1) * n) / p; j++)
{
localSums[iTemp] += numbers[j];
}
};
threads[iTemp] = new Thread(func);
threads[iTemp].Start();
}
int sum = 0;
for (int i = 0; i < p; i++)
{
threads[i].Join(); sum += localSums[i];
}
Console.WriteLine(sum);
Debug.Assert(sum == (n * (n + 1)) / 2);
}
private static int[] GenerateInts(int p)
{
int[] nums = new int[p];
for (int i = 0; i < p; i++)
{
nums[i] = i + 1;
}
return nums;
}
}
}Observe the highlighted variable "iTemp". What we a
-
Hi Rama Krishna, The issue in the above code is that, you are directly using the loop counter variable "i" in the anonymous delegate (anonymous method). Since this variable is captured by the anonymous delegate, the C# compiler will create a new class to hold this variable and will create an object of this new class above the for-loop containing the anonymous delegate, and replaces the loop counter variable usage with its instance data member "i". Due to this the same instance data member is used by all the anonymous methods created for the 10 threads. So the results will be pretty unpredictable when the threads start running. Also since the instance data member "i" is used as loop counter variable, by the time the loop ends its value would be 10 instead of 9 (as one would expect). For this reason when the threads actually start executing, they will try to access the 10th index of "localSums" array and the application will throw "IndexOutOfRange" exception. Below is the modified code that runs correctly.
using System;
using System.Collections.Generic;
using System.Text;
using System.Threading;
using System.Diagnostics;namespace TestAnonymous
{
class Program
{
static void Main(string[] args)
{
int n = 100000, p = 10;
int[] numbers = GenerateInts(n);
int[] localSums = new int[p];
Thread[] threads = new Thread[p];
for (int i = 0; i < p; i++)
{
int iTemp = i;
ThreadStart func = delegate
{
for (int j = (iTemp * n) / p; j < ((iTemp + 1) * n) / p; j++)
{
localSums[iTemp] += numbers[j];
}
};
threads[iTemp] = new Thread(func);
threads[iTemp].Start();
}
int sum = 0;
for (int i = 0; i < p; i++)
{
threads[i].Join(); sum += localSums[i];
}
Console.WriteLine(sum);
Debug.Assert(sum == (n * (n + 1)) / 2);
}
private static int[] GenerateInts(int p)
{
int[] nums = new int[p];
for (int i = 0; i < p; i++)
{
nums[i] = i + 1;
}
return nums;
}
}
}Observe the highlighted variable "iTemp". What we a
I had a similar issue before, and the fix was the same, creating a new local var, and assigning to it.
**
xacc.ide-0.2.0.57 - now with C# 2.0 parser and seamless VS2005 solution support!
**
-
This is well documented, yet I got into the issue and wasted about 1/2 hr. Consider the following program of summing 100000 numbers using 10 processors(threads). An extremely minor modification will slove this problem.
class Program
{
static void Main(string[] args)
{
int n = 100000, p = 10;int\[\] numbers = GenerateInts(n); int\[\] localSums = new int\[p\]; Thread\[\] threads = new Thread\[p\]; for(int i = 0; i < p; i++) { ThreadStart func = delegate { for (int j = (i \* n) / p; j < ((i + 1) \* n) / p; j++) { localSums\[i\] += numbers\[j\]; } }; threads\[i\] = new Thread(func); threads\[i\].Start(); } int sum = 0; for (int i = 0; i < p; i++) { threads\[i\].Join(); sum += localSums\[i\]; } Debug.Assert(sum == (n\*(n+1))/2); } private static int\[\] GenerateInts(int p) { int\[\] nums = new int\[p\]; for (int i = 0; i < p; i++) { nums\[i\] = i + 1; } return nums; }
}
Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it. -Brian Kernighan
I wonder if the C# compiler could detect such scenarios and either generate a warning or just generate an appropriate local.
Tech, life, family, faith: Give me a visit. I'm currently blogging about: For Christians: The Significance of Yom Teruah The apostle Paul, modernly speaking: Epistles of Paul Judah Himango
-
I had a similar issue before, and the fix was the same, creating a new local var, and assigning to it.
**
xacc.ide-0.2.0.57 - now with C# 2.0 parser and seamless VS2005 solution support!
**