7

I'm thinking of replacing a lot of inline foreaches with Linq and in so doing will make new methods, e.g. Current:

foreach(Item in List)
{
    Statement1
    Statement2
    Statement3
}

Idea:

List.Foreach(Item => Method(Item))

Obviously Method() contains Statement1..3

Is this good practise or is calling a method thousands of times going to degrade performance? My Lists have 10,000-100,000 elements.

1
  • 4
    No one knows what your tolerance is for performance degredation except you. Write the code both ways, measure the performance both ways, and then you'll know. Sep 21, 2010 at 14:13

4 Answers 4

12

Well, for one thing you can probably make the ForEach statement more efficient using a method group conversion

List.ForEach(Method);

That's removed one level of indirection.

Personally though, I don't think it's a good idea. The first approach is more readable, and likely to perform about as well. What's the advantage of using List<T>.ForEach here?

Eric Lippert talks about this more in an excellent blog post. I would use List<T>.ForEach if you already had a delegate you wanted to execute against each element, but I wouldn't introduce a delegate and an extra method just for the sake of it.

In terms of efficiency, I wouldn't expect to see much difference. The first form may perform a little better as it doesn't have the indirection of the delegate call - but the second form may be more efficient if the iteration loop within ForEach makes use of the fact that it has access to the internal data structures of the List<T>. I very much doubt you'll notice it either way. You could try to measure it if you're really bothered, of course.

6
  • 6
    Perhaps you are unaware that someone once did some pretty extensive performance comparisons on the various methods to enumerate a List<T>. Hope that helps. :) msmvps.com/blogs/jon_skeet/archive/2006/01/20/foreachperf.aspx
    – Ani
    Sep 21, 2010 at 12:42
  • 2
    @Ani: Gosh, I'd completely forgotten about that. Have just rerun the code with .NET 4, and the "language foreach" version seems to have improved somewhat.
    – Jon Skeet
    Sep 21, 2010 at 12:45
  • 1
    What is a "method group conversion" and how does it help? Sep 21, 2010 at 12:49
  • 1
    @Ian: It's the conversion from the name of a method to a delegate. The lambda expression will end up creating an extra method just to call Method. Method group conversions are what allow things like button.Click += ClickHandler;
    – Jon Skeet
    Sep 21, 2010 at 12:57
  • re "method group conversion", Thanks, I would had hoped the C# compiler would optimize this it’s self! Sep 21, 2010 at 14:10
2

If your motivation for considering the change is that the three statements in the body are too complicated, then I'd probably use ordinary foreach, but refactor the body to a method:

foreach(var item in List) 
  Method(item);

If the code in the body isn't complicated, then I'd agree with Jon that there is no good reason for using ForEach (it doesn't make the code more readable/declarative in any way).

I generally don't like using "LINQ-like" constructs to do imperative processing at the end of a LINQ query. I think that using foreach more clearly states that you're finished with querying data and you're doing some processing now.

1

I'm totally agree with Jon Skeet's answer. But since we are talking about ForEach performance, I have something addtional to your question. Be aware of that if your Statement 1~3 is not relative with each other, that is:

foreach(Item in List) 
{ 
   DoSomething();
   DoAnotherThing(); 
   DoTheLastThing();
} 

The code above probably has a worse performance than the following:

foreach(Item in List) 
{ 
   DoSomething();
} 
foreach(Item in List) 
{ 
   DoAnotherThing(); 
} 
foreach(Item in List) 
{ 
   DoTheLastThing();
} 

The reason that the latter code which needs 2 more go-over-loops has a better performance, is because when it keeps calling DoSomething() thousands of times, some necessary variables are always warm in CPU registers. Very low costs are used to access those variables. On the other hand, if it calls DoAnotherThing() immediately after calling DoSomthing(), those variables of DoSomething() which already in CPU registers will cool down. Much more costs are needed to access these variables in the next loop.

1
  • I wonder how many items in that list there would need to be in order to get the benefits from CPU registers warm variables
    – Coops
    Jun 27, 2013 at 9:13
0

I've always thought that you should write your code for readability first because the compiler and CLR do an exceptional job at optimisation. If you find that through benchmarking, that this code could be executed more quickly, then have a look at other options by all means.

E.g. for loops are quicker than foreach(), as they use array offsets which are internally optimised in the CLR.

But doesn't a List.ForEach () surely does a foreach ( ) anyway, so you are just giving the work to another method, rather than doing it yourself.

Strictly speaking, introducing more method calls will actually slow your code down on the first pass, because the CLR will JIT-compile methods as they are called, although subsequent calls to the method will not.

So my advice would be stick to writing readable code, then go from there if you can prove that this is a bottleneck of the system.

2
  • 1
    I'm not sure where you get the idea that the CLR does such an exceptional job at optimisation. Optimization isn't easy, particularly with things like multi-threading and side-effects (which C# includes), and my experience indicates that the CLR does reasonable well at eliminating overhead, but that you should not expect it to actually be able to rearrange code to even trivially "equivalent" variants that are faster. Write for readability because performance often doesn't matter, not because you expect miracles from the optimizer. Sep 21, 2010 at 13:06
  • I assume the goal is 2 fold: Less code which results in performance benefits. I would still go for readability first whether that be refactoring, or writing more lines of code to help maintainability. But prove it is a bottleneck first before trying to optimise. Sep 21, 2010 at 13:39

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.