2
\$\begingroup\$

I'm trying to implement sum of 2 array using multi threading.

I expected that when I use more threads it will be faster than when I use one thread, but that isn't the case.

public static int[] a = new int[100];
        public static int[] b = new int[100];
        public static int[] c = new int[a.Length];
        public static int numberofthreads;
     
        static void Main(string[] args)
        {
            Random r = new Random();
            for (int i = 0; i < 100; i++)
            {
                a[i] = r.Next(5, 20);
                b[i] = r.Next(10, 30);
            }
            Console.WriteLine("Enter the Number of threads to use: ");
            numberofthreads = int.Parse(Console.ReadLine());

            var watch = new System.Diagnostics.Stopwatch();

            watch.Start();
            Thread[] threadsarray = new Thread[numberofthreads];
            List<Thread> threads = new List<Thread>();

            for (int i = 0; i < numberofthreads; i++)
            {
                threadsarray[i] = new Thread(new ParameterizedThreadStart(myThreadMethod));
                threadsarray[i].Start(i);
               // threadsarray[i].Join();
            }

            watch.Stop();
            Console.WriteLine("The First Array : ");
            Console.WriteLine(string.Join(",", a));
            Console.WriteLine("The Secound Array : ");
            Console.WriteLine(string.Join(",", b));
            Console.WriteLine("The Result Array : ");
            Console.WriteLine(string.Join(",", c));
            Console.WriteLine(" ");
            Console.WriteLine(watch.ElapsedMilliseconds.ToString());

        }
        static void myThreadMethod(object threadid)
        {
            lock (threadid)
            {
                int start = a.Length;
                int thid = (int)threadid;


                for (int i = thid * a.Length / numberofthreads;
                    i < (thid + 1) * a.Length / numberofthreads; i++)
                {
                    c[i] = a[i] + b[i];

                }
            }
            

        }
\$\endgroup\$
7
  • \$\begingroup\$ perhaps you're not aware of Task and async tasks on NET 4+ as they're the better version of using Thread on the old versions of NET. \$\endgroup\$
    – iSR5
    Nov 23, 2021 at 20:47
  • 5
    \$\begingroup\$ You really would not see improvement on such a small array. In fact you should see degraded performance since there is a small overhead to spin up each thread. You may also consider using a .NET Range Partitioner to reduce code, but it too would be slower on such a small array. \$\endgroup\$
    – Rick Davin
    Nov 23, 2021 at 22:09
  • 2
    \$\begingroup\$ I disagree with the close vote - performance is on-topic. \$\endgroup\$
    – Reinderien
    Nov 24, 2021 at 0:36
  • \$\begingroup\$ I'm trying to learn c# multi-threading can you help me with a better implementation , I don't know how to start learning at this topic. I googling for more answer but i didn't find an interesting topic. Thank you so much \$\endgroup\$ Nov 24, 2021 at 7:14
  • \$\begingroup\$ Are you trying to learn multi-threading with C# specifically or in general? The subject is vast : overhead, stalls, diminishing returns, synchronization ... I would suggest to find a "pet project" that uses a lot of parallel resources then try to apply threads concepts on it. e.g. apply an expensive block transform to a huge image and have each block processed by a different thread. \$\endgroup\$
    – Hurricane
    Nov 24, 2021 at 7:31

2 Answers 2

2
\$\begingroup\$

NOTE: my code examples below use .NET 6.

As I said in the comments, your test is really so small that you will actual degrade performance with multiple threads. As this is CodeReview, all aspects of your code are subject to review, so let's cover some of the basics:

You really should not have almost everything in Program.Main. Your specific logic should be placed in its own class, and leave Program at a minimum to call that logic.

In your own class, the arrays should not be static. They should be class properties.

Naming convention is to spell things out. So static field a becomes property ArrayA. Admittedly my own names could be better. I copied your code into a class that I named Original, so my modified class was named Alternative. Mea culpa.

You should have your code be flexible to allow for larger arrays.

You should employ some basic parameter checks for bad inputs.

You should not rely upon so many magic numbers.

On to the heart of the app ...

You should stop thinking in terms of threads and number of threads. You may send 1000 threads to the scheduler and the scheduler will likely run far less than that concurrently. Instead, think in terms of MaxDegreeOfParallelism, i.e. the maximum number of threads that will be running concurrently. You should not waste time spinning up more threads than that or else you will degrade performance.

Rather than a custom "roll your own" method, I am going to use a Partitioner from the System.Collections.Concurrent namespace.

I will now run a Parallel.ForEach passing in the ranged partitions as well as using ParallelOptions to declare MaxDegreeOfParallelism. Note as well that I have 2 methods: one runs single threaded and the other runs multi-threaded for easy comparison.

Alternative.cs (forgive the poor name)

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Collections.Concurrent;
using System.Diagnostics;

namespace Threading_Example
{
    internal class Alternative
    {

        public int[] ArrayA { get; private set; } = new int[0];
        public int[] ArrayB { get; private set; } = new int[0];
        public int[] ArrayC { get; private set; } = new int[0];

        public int MaxDegreeOfParallelism { get; private set; }
        public int ArraySize { get; private set; } = 100;

        private Alternative(int size, int maxDegreeOfParallelism)
        {
            // There are probably more checks that could be done but here are the minimum.
            if (size <= 0)
            {
                throw new ArgumentOutOfRangeException(nameof(size), "Array size must be greater than 0");
            }
            if (maxDegreeOfParallelism <= 0)
            {
                throw new ArgumentOutOfRangeException(nameof(maxDegreeOfParallelism), "MaxDegreeOfParallelism must be greater than 0");
            }

            // While I could call Initialize() here, my philosophy is that a constructor
            // should do the BARE MINIMUM, which is only to set some properties and
            // then return as quickly as possible.
            // To that end, I mark the constructor as private, and require accessing
            // it via the public Create().

            this.ArraySize = size;
            this.MaxDegreeOfParallelism = maxDegreeOfParallelism;
        }

        public static Alternative Create(int size, int maxDegreeOfParallelism)
        {
            var instance = new Alternative(size, maxDegreeOfParallelism);
            // Initialize is intentionally run after construction.
            instance.Initialize();
            return instance;
        }

        private void Initialize()
        {
            ArrayA = new int[ArraySize];
            ArrayB = new int[ArraySize];
            ArrayC = new int[ArraySize];

            // Magic number replacements.
            // https://en.wikipedia.org/wiki/Magic_number_(programming)
            // Consider increasing with bigger arrays.
            // Consider making parameters or properties rather than constants.
            const int minA = 5;
            const int maxA = 20;
            const int minB = 10;
            const int maxB = 30;

            Random random = new Random();
            for (int i = 0; i < ArraySize; i++)
            {
                // Consider increasing the magic num
                ArrayA[i] = random.Next(minA, maxA);
                ArrayB[i] = random.Next(minB, maxB);
            }
        }

        public TimeSpan RunSingleThreaded()
        {
            var watch = Stopwatch.StartNew();
            for (int i = 0; i < ArraySize; i++)
            {
                ArrayC[i] = ArrayA[i] + ArrayB[i];

            }
            watch.Stop();
            return watch.Elapsed;
        }

        public TimeSpan RunMultiThreaded()
        {
            var watch = Stopwatch.StartNew();

            var rangeSize = ArraySize / MaxDegreeOfParallelism;
            if (ArraySize % MaxDegreeOfParallelism != 0)
            {
                rangeSize++;
            }

            // https://docs.microsoft.com/en-us/dotnet/api/system.collections.concurrent.partitioner?view=net-6.0
            var partitions = Partitioner.Create(0, ArraySize, rangeSize);
            var options = new ParallelOptions() { MaxDegreeOfParallelism = MaxDegreeOfParallelism };

            Parallel.ForEach(partitions, options, x =>
            {
                for (var i = x.Item1; i < x.Item2; i++)
                {
                    ArrayC[i] = ArrayA[i] + ArrayB[i];
                }
            });

            watch.Stop();
            return watch.Elapsed;
        }

    }
}

Program.cs

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

// https://codereview.stackexchange.com/questions/270338/sum-of-2-arrays-using-multi-threading

namespace Threading_Example
{
    class Program
    {
        static void Main(string[] args)
        {
            RunTest(100, 4);
            RunTest(100, 8);
            RunTest(1_000_000, 4);
            RunTest(1_000_000, 8);
            RunTest(1_000_000_000, 4);
            RunTest(1_000_000_000, 8);

            Console.WriteLine();
            Console.WriteLine("Press ENTER key to quit.");
            Console.ReadLine();
        }

        private static void RunTest(int size, int maxDegreeOfParallism)
        {
            var example = Alternative.Create(size, maxDegreeOfParallism);

            Console.WriteLine();
            Console.WriteLine($"Running single threaded for array size = {size:N0}");
            var elapsed1 = example.RunSingleThreaded();
            Console.WriteLine($"   Elapsed = {elapsed1}");

            Console.WriteLine($"Running multi-threaded for array size = {size:N0}, maxDegreeOfParallism = {maxDegreeOfParallism}");
            var elapsed2 = example.RunMultiThreaded();
            Console.WriteLine($"   Elapsed = {elapsed2}");
        }
    }
}

Sample Console Output

This .NET 6 app was run on my PC with AMD Ryzen 9 3900X but it was being run inside a Hyper-V VM with 6 virtual processors.

Running single threaded for array size = 100
   Elapsed = 00:00:00.0000223
Running multi-threaded for array size = 100, maxDegreeOfParallism = 4
   Elapsed = 00:00:00.0264532

Running single threaded for array size = 100
   Elapsed = 00:00:00.0000009
Running multi-threaded for array size = 100, maxDegreeOfParallism = 8
   Elapsed = 00:00:00.0044602

Running single threaded for array size = 1,000,000
   Elapsed = 00:00:00.0074864
Running multi-threaded for array size = 1,000,000, maxDegreeOfParallism = 4
   Elapsed = 00:00:00.0007149

Running single threaded for array size = 1,000,000
   Elapsed = 00:00:00.0026112
Running multi-threaded for array size = 1,000,000, maxDegreeOfParallism = 8
   Elapsed = 00:00:00.0015015

Running single threaded for array size = 1,000,000,000
   Elapsed = 00:00:01.9090166
Running multi-threaded for array size = 1,000,000,000, maxDegreeOfParallism = 4
   Elapsed = 00:00:00.6679679

Running single threaded for array size = 1,000,000,000
   Elapsed = 00:00:02.2135724
Running multi-threaded for array size = 1,000,000,000, maxDegreeOfParallism = 8
   Elapsed = 00:00:00.7445674

Press ENTER key to quit.

Findings

What you should clearly see is that 100 elements is too small to gain performance from multi-threading. We see some improvement going to 1 million elements, and even more so going to 1 billion.

What one could also see if that sometimes a larger MaxDegreeOfParallelism does not always mean better performance. It depends on many factors, such as complexity of calculations as well as CPU speed to name a couple.

UPDATE

I created a custom partitioner that has been posted as its own CR question. You may want to check it out since it demonstrates some eye-opening behavior when processing in parallel. It uses a slightly modified version of my answer using your array examples.

\$\endgroup\$
1
\$\begingroup\$

Joining threads

After starting a thread, you have to call join to make sure it is done with its work. I see you have commented join out and I can guess why. The correct way to do this is to first start all the threads and then join them like so:

for (int i = 0; i < numberofthreads; i++)
{
    threadsarray[i] = new Thread(new ParameterizedThreadStart(myThreadMethod));
    threadsarray[i].Start(i);
}

for (int i = 0; i < numberofthreads; i++)
{
    threadsarray[i].Join();
}

Unnecessarily locking threads

In myThreadMethod you are creating a critical section which basically means that only one thread at a time will be able to execute the function and the rest will be idling away waiting for the lock to become open. There is no need here to lock.

Performance

There are a couple of issues here with most of them already mentioned in the comments.

Your calculation is too simple. Most of the time is simply spent reading and writing to memory and multi-threading will not really help with that.

Let's see if we can create a bit more complicated tasks for the threads to keep them busy:

c[i] = (int)Math.Sqrt(Math.Pow(a[i], 2.0) + Math.Pow(b[i], 2.0));

Putting it all together:

using System;
using System.Collections.Generic;
using System.Threading;

class Program
{
    public const int count = 100000000;
    public static int[] a = new int[count];
    public static int[] b = new int[count];
    public static int[] c = new int[count];
    public static int numberofthreads;

    static void Main(string[] args)
    {
        Random r = new Random();
        for (int i = 0; i < count; i++)
        {
            a[i] = r.Next(5, 20);
            b[i] = r.Next(10, 30);
        }

        Console.WriteLine("Enter the Number of threads to use: ");
        numberofthreads = int.Parse(Console.ReadLine());

        var watch = new System.Diagnostics.Stopwatch();

        watch.Start();
        Thread[] threadsarray = new Thread[numberofthreads];
        List<Thread> threads = new List<Thread>();

        for (int i = 0; i < numberofthreads; i++)
        {
            threadsarray[i] = new Thread(new ParameterizedThreadStart(myThreadMethod));
            threadsarray[i].Start(i);
        }

        for (int i = 0; i < numberofthreads; i++)
            threadsarray[i].Join();


        watch.Stop();
        Console.WriteLine($"time = {watch.ElapsedMilliseconds.ToString()} ms");

        int total = 0;
        foreach (int v in c)
            total += v;

        Console.Write("c = " + c);
    }
    static void myThreadMethod(object threadid)
    {
        int start = a.Length;
        int thid = (int)threadid;
        for (int i = thid * a.Length / numberofthreads; i < (thid + 1) * a.Length / numberofthreads; i++)
        {
            c[i] = (int)Math.Sqrt(Math.Pow(a[i], 2.0) + Math.Pow(b[i], 2.0));
        }
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.