Is there any performance benefit in checking the item count prior to executing a foreach loop?

Question

I saw this in code and was wondering if there is any performance benefit to checking the item count prior to looping:

if (SqlParams.Count > 0)
    foreach (var prm in SqlParams)
        cmd.Parameters.Add(prm);

I always prefer to do a null check instead and let the foreach loop just hop out if there are 0 items.

if (SqlParams != null)
    foreach (var prm in SqlParams)
        cmd.Parameters.Add(prm);

Isn't that the better way?

Answer 1

Ultimately, the best answer is to actually test it. Make a method which loops over an empty array with and without checking the length first, call each 100,000 times and see which has a faster runtime.

public void withCheck(Integer[] array) {
    for (int i = 0; i < 100000; i++) {
        if (array.length > 0) {
            for (Integer i : array) {
                // nothing to do.
            }
        }
    }
}

public void withoutCheck(Integer[] array) {
    for (int i = 0; i < 100000; i++) {
        for (Integer i : array) {
            // nothing to do.
        }
    }
}

public void test() {
    long startTime = System.currentTimeMillis();
    withCheck();
    System.out.println("Time with check: " + (System.currentTimeMillis() - startTime) + "ms");
    startTime = System.currentTimeMillis();
    withoutCheck();
    System.out.println("Time without check: " + (System.currentTimeMillis() - startTime) + "ms");
}

When I write code like this, I generally ponder this as a trade-off between code readability and perceived or potential performance. In other words, adding a if (count > 0) check negatively impacts the readability, even though by a very small amount. So what is the gain? I presume there is very little or no advantage, but like you, I have not actually tested it. What I can tell you is that in all my years profiling, I have never found looping over an empty array to be a bottleneck.

Technically, it depends on how the iterator works. For a foreach loop like you have, you are creating an iterator object behind the scenes. My expectation would be, and again, this is unverified, is that the iterator object FOR AN ARRAY would simply be implemented like this (this is Java, but C# is probably comparable):

public class ArrayIterator<T> implements Iterator<T> {
    private int next = 0;
    public T next() {
        return backingArray[next++];
    }

    public boolean hasNext() {
        return next < backingArray.length;
    }
}

So iterating over such an iterator should exit out very quickly because hasNext() will return false. Notice it's the same check that your if statement would do.

Now if you are iterating over a linked list, the implementation should be different. If you explicitly check the size, you may be forcing it to scan the linked list. In Java, LinkedList.size() checks an explicit size variable so it doesn't walk the list, but I don't know how .NET implements it.

Another way to look at this is how likely is the collection to be empty? If that is a rare case, then if the theory that iterating over an empty array takes time is true, then adding an explicit check for empty first would only be faster if the collection is usually empty. If not, you are theoretically slowing down your average case just to slightly speed up your worst case.

Again, these are all theories and the timings are on the micro-level, so take it with a grain of salt.

TL;DR

It depends on the type of collection, the runtime platform, and the typical collection size, but intuitively, it seems unlikely to speed anything up in most situations. But the only way to know for sure is to test it.

Answer 2

Not really, because if there are 0 items, the setup of the foreach loop will find that out anyway and simply not execute the body of the loop.

It's theoretically possible, in a very tight loop in which it's common for your collection to be empty, and in which finding the count is a very inexpensive operation, for there to be a noticeable performance benefit here, but if you're doing database queries, that's almost certainly not the case. So don't worry about it unless profiling shows that this is actually taking a significant amount of execution time.

Answer 3

Looking at the source code of System.Collections.Generic.List<T> we see that MoveNext() is implemented like this:

public bool MoveNext() {
    List<T> localList = list;

    if (version == localList._version && ((uint)index < (uint)localList._size)) 
    {                                                     
        current = localList._items[index];                    
        index++;
        return true;
    }
    return MoveNextRare();
}

Meaning that the total cost of one check is

• Assigning an existing collection to another variable
• Comparing versions, whatever that is
• Casting two int values to uint and comparing them

When we look at the source code of .Count:

public int Count {
    get {
        lock (_root) { 
            return _list.Count; 
        }
    }
}

We see that the cost is acquiring a lock.

Conclusion

You can go measure and see which one performs best but I think it's safe to conclude that none of this will ever matter. You might have some uncommon collections to iterate over that don't do an early size check, but that's a very specific edge case that will point itself out then.

Bottom line: don't worry about it and you shouldn't bother adding a .Count > 0 or _collection != null check in the first place.

Answer 4

The only scenario I can think of where checking Count first would help performance is with a collection having a resource-hungry IEnumerable.MoveNext() implementation and yet an highly efficient ICollection.Count implementation. While it's technically possible, it's very unlikely that it would ever happen.

In fact, on some scenarios, checking Count() first would actually be less performant. Think for example of ORM frameworks like Entity Framework which takes advantage of linq's IQueryable interface. Behind the scenes, these objects reaches out to a datastore to provide collections of objects and computing operation results like sums, averages and of course count. Calling IQueryable.Count() here would actually end up making a SELECT COUNT(*) ... query on the database. If you don't need that information then it would just be needless overhead to get it.

Bottom line: I really wouldn't change anything to your actual coding style. It's perfectly acceptable to loop through a collection right away, even if it turns out to be empty. Making sure it isn't null first is okay if you cannot be sure of its provenance.

Answer 5

The benefit comes in the form of flexibility, but it has no performance benefit.

When a foreach runs over an ICollection with count of 0, the enumerator will register as having already traversed the whole set. Functionally, it's similar to the following for loop:

for(int i = 0; i > -1; i++)
    foo();

Now, if there is some other business rule that needs to fire prior to the foreach, that's a completely different story, and having the if condition accomodates that future capability nicely. But, it's also not strictly necessary, either.

EDIT: In response to your edit, the null check is a vastly better way to do it, since a collection can be null.

I wound up writing a IsNullOrEmpty extension method for IEnumerables, because I kept on seeing a lot of repeated conditions like:

if(SomeCollection != null
   && SomeCollection.Count > 0
   && (SomeOtherCondition))
{
    // Stuff that relies on SomeCollection having at least one value.
}

Answer 6

Short answer is: NO.

Details: SqlParameterCollection implements IEnumerable interface that allow you to use the foreach loop, the foreach itself insure that you don't get exception but in one condition that is System.NullReferenceException taking into your consideration that Null exception has nothing to do with (Count) property, you can have 0 params in the collection and the variable is initialized (not null).

So what do you need to do, simply check for null before the foreach syntax.

Answer 7

Consider system performance vs. a single execution. If the list is non-empty 99% of the time, then you are paying the cost of an extra check almost all the time.

On the other hand, if the list is empty 99% of the time, then it might work out differently (although I wonder if the extra invocation offsets any time you save -- remember we're talking about micro- or nano-seconds in this example).

So you can't just consider a single pass running in one thread, you have to consider the system's performance all together.