Increasing performance and accuracy in multithreaded loop

I was wondering if the following set up will return consistent result, i.e. no thread function call will be skipped. Also, is there any way to improve the threaded loop?

Note the first loop is the multithread version and the results are out of order and this is okay but I was hoping to find a way to get them in order. The second loop is the regular version non threaded.

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

public class Example
 {

public static int[] LoopPoints = {1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20};
public static Stack StackLoopPoints;
public static void Main()
{
    StackLoopPoints = new Stack();
    StackLoopPoints.Push(1);
    StackLoopPoints.Push(2);
    StackLoopPoints.Push(3);
    StackLoopPoints.Push(4);
    StackLoopPoints.Push(5);
    StackLoopPoints.Push(6);
    StackLoopPoints.Push(7);
    StackLoopPoints.Push(8);
    StackLoopPoints.Push(9);
    StackLoopPoints.Push(10);
    StackLoopPoints.Push(11);
    StackLoopPoints.Push(12);
    StackLoopPoints.Push(13);
    StackLoopPoints.Push(14);
    StackLoopPoints.Push(15);
    StackLoopPoints.Push(16);
    StackLoopPoints.Push(17);
    StackLoopPoints.Push(18);
    StackLoopPoints.Push(19);
    StackLoopPoints.Push(20);

    //Array
    List tasks = new List();
    for (int ctr = 1; ctr  { Console.Write(DoStuffStack(ctr)); }));
    }
    Task.WaitAll(tasks.ToArray());

    Console.WriteLine("");

  // Normal
  for (int ctr = 21; ctr <= 40; ctr++)
  {
      Console.Write(DoStuffNormal(ctr));
  }

  Console.Read();
}

 private static string DoStuffNormal(int ctr)
  {
   Thread.Sleep(TimeSpan.FromSeconds(1d));
   return " " + ctr.ToString() + " ";
  }

  private static string DoStuffStack(int ctr)
  {
   Thread.Sleep(TimeSpan.FromSeconds(1d));
   return " " + StackLoopPoints.Pop().ToString() + " ";
  }

 }

So your first question.


no thread function call will be skipped.

You seem to be lost on what the WaitAll(Task[]) function does. This function is essentially a while loop that only exits when all threads it is called upon have finished executing. In this case, your program will always execute all threads in your 'tasks' list. If you were to call something like WaitAll(Task[], Int32) instead, like: Task.WaitAll(tasks, 500); then your code would wait 500 milliseconds for your threads to join, and then continue on. It would give you an undesirable affect of not guaranteeing the execution of all threads.


Also is there any way to improve the threaded loop? ...I was hoping to find a way to get them in order.

You have to understand that threads do not guarantee execution in order of insertion, or when they were started. Imagine you have a 5 gallon bucket that is half filled with bouncy balls. Each ball has a number on it from [0, n], and was placed in the bucket in order from 0 -> n. Now, if we put a lid on the bucket, shake the bucket, pull the lid off, put a blind fold on and then try to pull out the balls in order of n - 0 we have a close idea of what happens when trying to get an ordered execution of threads.

Edit:

Due to the comments from @ErnieL I realized that there is another aspect of multithreading which needs to be addressed about scope. We are not talking about scope as in my object exists between braces, but rather the scope of who has control over my object.

The point was brought up about doing StackLoopPoints.Pop().ToString(). Prima facie this is an easy thing to do and results in the data that we want, but there is actually a tricky situation happening behind the scenes. We have to ask ourselves: What happens in the time span between when Pop() and ToString() being called?


The answer is we do not know. The reason is because Pop() and ToString() are two separate instructions, which in multithreading allows for a context switch (a.k.a. change in thread being executed) to occur. Since there is no way to force a context switch to not occur*** we have to code against the situation of a context switch occurring, this is what @ErnieL's comments are saying.

We can actually improve DoStuffStack(int ctr) by protecting it a little bit.

Method 1

private static string DoStuffStack(int ctr)
{
    thread.Sleep(TimeSpan.FromSeconds(1d));
    // Pick which method you want to use. I realized the variable ctr is not being used, 
    // and wanted to give it a chance to do something.
    const int value = StackLoopPoints[ctr];  // This way we always grab the intended item.
    const int value = StackLoopPoints.Pop(); // This way we grab an item in the list as long as the list has items.
    return " " + value.ToString();
}

Method 2

There are better ways of dealing with thread safety than this, but this is a basic technique for dealing with safely removing an item from StackLoopPoints.

static int currentLock = 0;
private static string DoStuffStack(int ctr)
{
    while(currentLock != ctr)
    {
        thread.Sleep(TimeSpan.FromSeconds(1d));
        // This may fail to work correctly.
        // So please, do not break out of the loop in the if block.
        if(currentLock != -1)
        {
            currentLock = ctr;
        }
    }

    int value = StackLoopPoints.Pop(); // We can't return without clearing our variable.
    currentLock = -1; // We have the value we need, and no longer require the lock.

    return " " + value.ToString();
}

*** There are cases where you can force a context switch to not occur, but that is a conversation for another time.

StackExchange Code Review Q#71763, answer score: 5