Improve the performance by multi-threading

I'm now trying to improve a winform application's performance by make it multi-threaded. Currently the class looks like:

public class MainClass
{
   List<DataItem> data; //thousands of DataItem, but each is independent

   //and a lot of non-thread-safe variables here,variable1 variable2 ...

   public void Go()
   {
      data.ForEach(item => DealWithDataItem(item));
   }

   public void DealWithDataItem(DataItem item)
   {
      //costs really long time here
      Step1(item);
      Step2(item); //and a lot of StepN(item)
   }

   public void StepN(DataItem item)
   {
      //variable1 = blabla
      //variable2 = blabla ..etc
   }
}

I want to use ThreadPool开发者_运维知识库 for each DataItem.

data.ForEach(item => ThreadPool.QueueUserWorkItem( s => DealWithDataItem(item) ));

But so many non-thread-safe variables! I can't declare them in some method, because it's shared between StepN methods. And it's quite hard to make them all thread-safe! Am I doing anything wrong? Any good solutions? Thanks!

---

Try using ParallelEnumerable.AsParallel.

data.AsParallel.ForEach(DoWork);

It will automatically create threads depending on amount of processors / cores. The only problem, that it's included in Framework 4.0. More info about PLINQ. (And as andras commented: for framwork 3.5 it is available as stand-alone Reactive Extensions (Rx))

UPD: as 0xA3 said, refactoring code, making each item have it's own calc variables is strongly recommended. I suggest you to extract calculation logics to DataItem

Or create special class like "Calculator", which would be do all the work, so DataItem would only store data, and logics of calculations would be contained in Calculator class.

data.AsParallel.ForEach(x=> new Calculator().DoWork(x));

where Calculator class is something like this

class Calculator
{
   // variables here

  void DoWork(DataItem item)
  {
     Step1(item);
     Step2(item);
     // ...
     // StepN(item);
  }
}

---

Probably the best way would be to refactor your code, so that you get rid of all that fields shared between the different data items.

Change (or subclass) the DataItem class to contain all the relevant data and methods for manipulating a dataItem, so that your code changes to something like this:

public void DealWithDataItem(DataItem item)
{
    item.Step1(); // does not change the state of `this` 
                  // and only changes variables that are private to `item`
    item.Step2(); // and a lot of StepN(item)
}

---

Since each DataItem is independent, move the work into a new DataItem worker method, and let each instance deal with itself:

public class MainClass
{
    List<DataItem> data; //thousands of DataItem, but each is independent

    public void Go()
    {
        data.ForEach(item => ThreadPool.QueueUserWorkItem(s => s.DealWithSelf()));
    }
}

public class DataItem
{
    //and a lot of non-thread-safe variables here,variable1 variable2 ...

    void DealWithSelf()
    {
        //costs really long time here
        Step1(item);
        Step2(item); //and a lot of StepN(item)
    }

    public void StepN(DataItem item)
    {
        //variable1 = blabla
        //variable2 = blabla ..etc
    }
}

---

Is MainClass in your GUI thread? You should not be doing any data processing in your GUI thread; run MainClass in a separate thread.

How to do this? That depends entirely on the blabla stuff you have not shown us. Does MainClass need to return a result? Use BeginInvoke/EndInvoke. Do you need to update the GUI? Use BackgroundWorker. If you want a better answer you will have to give us more information.