0

I would like to iterate through a List of a custom class with some typical properties. The list will be huge and growing(starts with 3000items and may reach +10k)

I am reading about best ways to iterate in a very fast way and checking properties, because I shall fill some counters.

In .NET, which loop runs faster, 'for' or 'foreach'?

http://www.codearsenal.net/2013/12/csharp-multithreading-loop-parallel-for.html#.U09-qPl_vA0

But if I need to follow a logic checking these properties... what would be faster?

At the moment my code looks like this:

  public void CalculateTotalCounters()
  {
     #region ParallelCounting

     countTotal = 0;
     countMapped = 0;
     countNotMapped = 0;
     countError = 0;

     errorFound = false;

     MyList.AsParallel().ForAll(acc =>
     {
        lock (this)
           ++countTotal;

        if (!string.IsNullOrEmpty(acc.isMapped))
        {
           lock (this)
              ++countMapped;
        }

        if (string.IsNullOrEmpty(acc.isMapped))
        {
           lock (this)
              ++countNotMapped;
        }

        if (acc.HasError || acc.NotUnique || acc.DefalutName || acc.DefaultValue)
        {
           lock (this)
           {
              ++countError;
              errorFound = true;
           }
        }
     });

     #endregion
  }

I know two of the conditions for isMapped could be in just one if-else(Might this affect the lock?)

Thank you.

Community
  • 1
  • 1
blfuentes
  • 2,465
  • 3
  • 36
  • 60
  • Are you sure the locking on the counters is necessary? – Codor Apr 17 '14 at 07:25
  • I got some strange results when I didn't use it. I couldn't find an answer close to my problem about the locking – blfuentes Apr 17 '14 at 07:27
  • Don't `lock(this)`, you don't know what else might be locking on the instance. – Rawling Apr 17 '14 at 07:56
  • This isn't answer, but I cannot comment due to low reputation. If you need set counters thread-safe, then use http://msdn.microsoft.com/en-us/library/dd78zt0c(v=vs.110).aspx instead of locking. – Honza Kovář Apr 17 '14 at 07:36

1 Answers1

2

The locks will kill the performance gained from parallel.

You can use interlocked increment http://msdn.microsoft.com/en-us/library/dd78zt0c(v=vs.110).aspx

Interlocked.Increment(countMapped)

Also you don't need to count so many things. After the loop you can calculate some:

countTotal = MyList.Count();
countNotMapped = countTotal - countMapped;
errorFound = errorCount > 0;

But whatever you do you should profile this, i.e. Time it. You can use Stopwatch class for this. Then you can try different things to see which is faster.

You should then try these:

countMapped = MyList.Count(acc => istring.IsNullOrEmpty(acc.isMapped);

countMapped = MyList.AsParallel().Count(acc => istring.IsNullOrEmpty(acc.isMapped);
weston
  • 51,132
  • 20
  • 132
  • 192
  • Thanks. I tested three approaches with 4500 items:parallel ForAll(got ~860ms) "Normal" count ~430ms and "parallel" count ~800ms. The best then will be just using two counts and calculate the other properties when the loop finishes. – blfuentes Apr 17 '14 at 08:01