Calling a method in a linq foreach - how much overhead is there?

Question

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.

Answer 1

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.

Answer 2

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.

Answer 3

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.