4

Need to make an multithreading application, which performs some activities and shows progress of all work in ProgressBar. Here is the code. Only thread named "thread - 0" makes all work. I need to distribute work between all threads.

public partial class Form1 : Form
    {
        const float maxWorkedValue = 10;
        const int threadsCount = 5;
        float totalProgress;
        int threadSleepTime = 1;

         object locker = new object();
        List<Thread> threads = new List<Thread>();

        public Form1()
        {
            InitializeComponent();

            prBar.Maximum =  (int)maxWorkedValue;
            prBar.Step = 1;

            for (int i = 0; i < threadsCount; i++)
            {
                threads.Add(new Thread(BeginCalculate) { Name = "Thread - " + i.ToString() });
                threads[i].Start();
            }
        }




        void BeginCalculate()
        {

            for (int i = 0; i < maxWorkedValue; i++)
            {
                lock (locker)
                {
                    float currentProgress = CalculateLongTask(i);
                    Update((currentProgress / maxWorkedValue) * 100, Thread.CurrentThread.Name);
                    Thread.Sleep(threadSleepTime);
                }
            }
        }


        float CalculateLongTask(int value)
        {
            for (int i=0; i<value;i++)
                Thread.Sleep(threadSleepTime);

            return totalProgress++;
        }

        void Update(float i, string threadName)
        {
            if (InvokeRequired)
            {
                BeginInvoke(new Action<float, string>(Update), new object[] { i, threadName });
                return;
            }
            label1.Text = threadName;
            prBar.Value = (int)i;
        }

    }

UPDATE2: I've updated the code above. But it not resolved the problem. How to synchronize the variable "i" between different threads? An exception (progressbar.Max) is thrown. Why?

 for (int i = 0; i < maxWorkedValue; i++)
        {
            lock (locker)
            {
                float currentProgress = CalculateLongTask(i);
                Update((currentProgress / maxWorkedValue) * 100, Thread.CurrentThread.Name);
                Thread.Sleep(15);
            }
        }
Alexandre
  • 13,030
  • 35
  • 114
  • 173
  • 2
    Not a solution, but why aren't you using a [ThreadPool](http://msdn.microsoft.com/en-us/library/system.threading.threadpool%28v=vs.80%29.aspx) out of curiosity? – Brad Christie Feb 05 '11 at 18:51
  • 1
    Correction: Thread 0 runs for a minute and then an exception (progressbar.Max) is thrown. Please post real code and/or include such details. – H H Feb 05 '11 at 19:12
  • Re the Update: OK, so if you get that then what is the real problem? Are there resources to be locked? etc. – H H Feb 05 '11 at 19:26
  • Re update2: `prBar.Maximum = maxWorkedValue * threadsCount;` – H H Feb 05 '11 at 19:42
  • And consider where the real work is done, don't just Sleep() all over the place. That is very critical for what/when to lock. – H H Feb 05 '11 at 19:43
  • Henk Holterman, it throws an error "Value of '60' is not valid for 'Value'. 'Value' should be between 'minimum' and 'maximum'. Parameter name: Value" – Alexandre Feb 05 '11 at 19:47
  • Just set that Maximum a lot higher. – H H Feb 05 '11 at 20:32

2 Answers2

3

EDIT:

If I understood you correctly you want to execute a loop (i.e.for (int i = 0; i < maxWorkedValue; i++) ) concurrently among many threads.

This is extremely easier to accomplish using the Parallel extensions 1.

Here's your modified code:

public partial class Form1 : Form
{
    const float maxWorkedValue = 100;
    const int threadCount = 5;
    float maxWorkedValue2 = 1;

    Thread thread;

    public Form1()
    {
        InitializeComponent();

        prBar.Maximum = (int)maxWorkedValue;
        prBar.Step = 1;

        thread = new Thread(BeginCalculate) { Name = "Father thread" };
        thread.Start();
    }

    void BeginCalculate()
    {
        Parallel.For(0, (int)maxWorkedValue,
            (i) =>
            {
                float currentProgress = CalculateLongTask(i);

                // here we are (probably) in a thread child 
                //of "Father thread" so let's use ManagedThreadId instead of the name...
                Update((currentProgress / maxWorkedValue) * 100, Thread.CurrentThread.ManagedThreadId.ToString());
                Thread.Sleep(2);
                return;
            });
    }


    float CalculateLongTask(int value)
    {
        for (int i = 0; i < value; i++)
            Thread.Sleep(15);

        return maxWorkedValue2++;
    }

    void Update(float i, string threadName)
    {
        if (InvokeRequired)
        {
            BeginInvoke(new Action<float, string>(Update), new object[] { i, threadName });
            return;
        }
        label1.Text = threadName;
        prBar.Value = Math.Min((int)i, 100);
    }

}

1
If you're using .NET 4.0 Parallel extensions comes with the framework.
If you're using .NET 3.5, look at this Q&A to know how to use it.

Community
  • 1
  • 1
digEmAll
  • 56,430
  • 9
  • 115
  • 140
  • I have a task to make what I've described above. – Alexandre Feb 05 '11 at 19:03
  • It's good, but I have to make a task strictly. I have to use the class Thread to "performs some activities and shows progress of all work in ProgressBar". How many threads you use in your edit? – Alexandre Feb 05 '11 at 20:02
  • @Alex: parallel library calculates the best number of thread to use according CPU cores number. So it depends on your CPU... – digEmAll Feb 05 '11 at 20:06
  • 1. It doesn't work. The progress bar's value doesn't increase. 2.And how can I do this using the class Thread and List threads? – Alexandre Feb 05 '11 at 20:13
  • @Alex: you should avoid using `Thread` directly in most cases. – H H Feb 05 '11 at 21:15
  • @Alex: it works, I tested id. You probably copied my first version of the code where I missed `thread.Start();` line ;). Anyway, if you strictly need to use `Thread` class (homework ?) you should specify it in the question... – digEmAll Feb 06 '11 at 10:02
  • @Alex: have a look at the example I've posted here --> http://0bj3ct1v3.pastebin.com/HAYcbhMG it should be helpful to you – digEmAll Feb 06 '11 at 14:32
2

Mostly, your locking scheme is broken:

void BeginCalculate()  // main thread method
{
    lock (locker)
    {
       ....  // 1 thread at a time
    }
}

All threads are run in sequence, you're not really multi-threading.

I can't comment on the rest except to say it doesn't look like something you should use for real.

You should probably outline what you really need to do, It won't be Sleep().

H H
  • 263,252
  • 30
  • 330
  • 514