1

I have these codes in my windows form C# application:

private void button7_Click(object sender, EventArgs e)
{
    ThreadStart starter = delegate { thread_func(2, 1000000); };
    thread1_thread = new Thread(starter);
    starter = delegate { thread_func(1000000, 2000000); };
    thread2_thread = new Thread(starter);
    starter = delegate { thread_func(2000000, 3000000); };
    thread3_thread = new Thread(starter);
    starter = delegate { thread_func(3000000, 4000000); };
    thread4_thread = new Thread(starter);

    thread1_thread.Start();
    thread2_thread.Start();
    thread3_thread.Start();
    thread4_thread.Start();

}

void thread_func(decimal input1,decimal input2)
{
    for (; input1 < input2; input1++)
    {
        threadNumbers_list.Add(input1);
        if (input1 % 2 != 0)
        {
            if (isPrime_func(input1))
            {
                PrimeNumbers_decimal_list.Add(input1);
            }
        }
    }
}
public static Boolean isPrime_func(decimal number)
    {
        decimal boundary = (decimal)Math.Floor(Math.Sqrt((double)number));

        if (number == 1) return false;
        if (number == 2) return true;

        for (decimal i = 2; i <= boundary; ++i)
        {
            if (number % i == 0) return false;
        }

        return true;
    }

Every time I run click that button I get different results. I have tried many things but could not figure out why this happens. Even for lower ranges it happens. Just in range of 100 numbers for example it gives the same result always. Some time my list count reaches 283138 and sometimes 283131 and other near numbers.

Another weird this is that when I comment checking even numbers, operation takes shorter time than this mode. What's wrong?

ACE
  • 23
  • 6
  • 5
    `threadNumbers_list` and `PrimeNumbers_decimal_list` are probably not thread safe. – Yacoub Massad Feb 17 '16 at 18:59
  • i created that list just for checking if all iteration happens or not! – ACE Feb 17 '16 at 19:00
  • 1
    What about `PrimeNumbers_decimal_list`? Also, how do you wait for the threads to finish? Do you use `Thread.Join`? – Yacoub Massad Feb 17 '16 at 19:00
  • I check the VS events and wait for all threads to exit and also check app cpu ussage then click another button that show primenumberlist count – ACE Feb 17 '16 at 19:03
  • I created multiple thread multiple primefunction but could not solve that! – ACE Feb 17 '16 at 19:05

1 Answers1

2

When multiple threads access a list, that list have to be thread safe or otherwise you are going to have a lot of problems.

.NET provides some thread-safe collections like the ConcurrentQueue<T> class.

Side note: Please consider using Tasks instead of threads. Also, the .NET framework supports data parallelism via the Parallel class. Consider using such class instead.

Regarding the performance when you don't check if the number is even, I tested this locally and I got the following numbers:

  • It takes ~76 seconds when I don't check if the number is even.
  • It takes ~66 seconds when I do check if the number is even.

So this does not match your measurements. It might be caused by the way you measure. I measure with a Stopwatch like this:

//...

Stopwatch sw = Stopwatch.StartNew();

thread1_thread.Start();
thread2_thread.Start();
thread3_thread.Start();
thread4_thread.Start();

thread1_thread.Join();
thread2_thread.Join();
thread3_thread.Join();
thread4_thread.Join();

long result = sw.ElapsedMilliseconds;

//...

By the way, here is something that you can do that might save some execution time for you:

Create a normal List<T> instance for each thread inside the thread_func method so that you don't have multi-threading issues. Then after the loop finishes, you can update the master list from the local list. Only updating the master list has to be thread safe. In this case I would prefer that the master list is a normal List<T> and that you use the lock keyword to synchronize access to it because you only need to update it 4 times (the number of threads).

Yacoub Massad
  • 27,509
  • 2
  • 36
  • 62
  • What is the definition of `PrimeNumbers_decimal_list`? Is it a `List`? You can change it to `ConcurrentQueue` and use the [`Enqueue`](https://msdn.microsoft.com/en-us/library/dd287262(v=vs.110).aspx) method instead of the `Add` method. – Yacoub Massad Feb 17 '16 at 19:10
  • ok thanks, i'm gonna check it. List PrimeNumbers_decimal_list = new List(); – ACE Feb 17 '16 at 19:12
  • Yeah! thank you so much @Yacoub the result problem has been solved :) – ACE Feb 17 '16 at 19:25
  • how about that weird thing why checking for even numbers does not speed operation, also slower it? – ACE Feb 17 '16 at 19:26
  • You are welcome. Does the even number slow issue happen even after you used the `ConcurrentQueue` class? – Yacoub Massad Feb 17 '16 at 19:29
  • Can you show the `isPrime_func` method? How do you measure the time? – Yacoub Massad Feb 17 '16 at 19:32
  • I didn't check it yet! i'm gonna check it – ACE Feb 17 '16 at 19:32
  • I added function to my post i measure time with VS 2015 Diagnostics tools – ACE Feb 17 '16 at 19:38
  • checked it with 'ConcurrentQueue' but no change at the speed result! – ACE Feb 17 '16 at 19:48
  • I'm just amateur in programming, so i don't know much about these tricks. does this affects on time? and i don't know why VS tools show that kind of time! any way i changed the thread code and it takes about 20s shorter than previous mode i just check odd numbers in other way – ACE Feb 17 '16 at 20:21
  • `void thread_func(decimal input1,decimal input2,int lastdigit) { for (; input1 < input2; input1++) { //threadNumbers_list.Add(input1); if (input1 % 10 == lastdigit) { if (isPrime_func(input1)) { PrimeNumbers_decimal_ConcurrentQueue.Enqueue(input1); } } } }` – ACE Feb 17 '16 at 20:23
  • ` ThreadStart starter = delegate { thread_func(2, 2000000, 1); }; thread1_thread = new Thread(starter); starter = delegate { thread_func(2, 2000000, 3); }; thread2_thread = new Thread(starter); starter = delegate { thread_func(2, 2000000, 5); }; thread3_thread = new Thread(starter); starter = delegate { thread_func(2, 2000000, 7); }; thread4_thread = new Thread(starter); starter = delegate { thread_func(2, 2000000, 9); }; thread5_thread = new Thread(starter); ` – ACE Feb 17 '16 at 20:23
  • @ACE, I am not sure I understand your question. – Yacoub Massad Feb 17 '16 at 20:29
  • i could make it faster by checking last digit. and my question is what you suggested, does it make it faster? – ACE Feb 17 '16 at 20:37
  • You mean using a local list for each thread? I guess yes. But you always have to measure to be sure. – Yacoub Massad Feb 17 '16 at 20:39
  • yeah i meant that. OK thank for your suggested. really helped me very much with that info about list – ACE Feb 17 '16 at 20:46