rbwhitaker / CSharpPlayersGuideEarlyAccess

A place to track issues with the C# Player's Guide for patches and future editions
18 stars 0 forks source link

5th Edition: "THE FOREACH LOOP"; Accessing the index of an array in a foreach loop #712

Closed TechProofreader closed 6 months ago

TechProofreader commented 6 months ago

First off, the book is amazing, so I wanted to say both bravo and thank you for it! Coming from a Python/R background, your book has been a phenomenal resource!

As for this "issue," it's more of something to note: in the FOREACH LOOP section on page 95 of the 5th Edition, it says that you lose access to an array's index if you use a foreach loop, and that it's best to stick to a for loop if you want to access an array's index. I was able to find a quick workaround to this, which I'll show below:

Simple FOR Loop:

int[] scores = new int[10] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };

for (int i = 0; i < scores.Length; i++)
{
    Console.WriteLine($"The value of Score #{i} is {scores[i]}.");
}

// This took 0.308 seconds to execute.

The workaround:

Simple FOREACH loop:

int[] scores = new int[10] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };

foreach (int i in scores)
{
    Console.WriteLine($"The value of Score #{Array.IndexOf(scores, i)} is {i}.");
}

// This took 0.334 seconds to execute.

Both print out the same result:

The value of Score #0 is 1.
The value of Score #1 is 2.
The value of Score #2 is 3.
The value of Score #3 is 4.
The value of Score #4 is 5.
The value of Score #5 is 6.
The value of Score #6 is 7.
The value of Score #7 is 8.
The value of Score #8 is 9.
The value of Score #9 is 10.

In online discussions, I see most people saying that even if one were to figure out a way to access the index of an array in a FOREACH loop, it would be resource heavy and more time consuming than simply using a FOR loop. However, Array.IndexOf(Array, Object) is a built-in Method of the System Namespace and is quite fast. Although I didn't use performance counters to do a deep dive, the console timer showed only a performance improvement of around ~8% in using the FOR loop instead of the FOREACH loop when accessing the array's index. For large datasets, that can definitely be a a concern, but for anything outside of Big Data or in memory limited applications, that doesn't seem too bad. I'm also interested in running tests to see if the percent difference between loop types reduces any further if CPU Affinity is taken into account.

I look forward to hearing what your take on the matter is!

rbwhitaker commented 6 months ago

I love the experimentation and analysis here. I'm going to respond with my own thoughts. This is going to be a slightly rambling, not-completely-organized response, because I don't have time for more.

First of all, I want to say that I did a thorough analysis of the performance of for vs. foreach in a blog post. That's probably the best place to start, because it covers all the basic, important points: http://csharpplayersguide.com/blog/2022/03/05/for-vs-foreach-performance/

Beyond that, there isn't much to add--certainly nothing earth-shattering--but I do have a few more thoughts.

There are two things that immediately popped into my mind with the samples provided are:

  1. I don't know how it was measured, and feel the need to put it into a tool that I trust to eliminate as much noise as possible and give me accurate timing information: Benchmark.NET.
  2. I think the Console.WriteLine is, honestly, the vast majority of the time you're reporting. Running instructions on a computer is fast. Doing things like writing to memory (that isn't in a register) is much slower. Like maybe 100x or 1000x slower. It isn't even close. But writing to disk or the console window might be 100x or 1000x slower than even that. If you really want to compare for vs. foreach, you probably need to use code that specifically doesn't do anything crazy with memory and definitely nothing with I/O (console or the file system).

Anyway, I started by copy/pasting this code into a Benchmark.NET test, with the code below:

using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;

BenchmarkRunner.Run<ForeachVsFor>();

public class ForeachVsFor
{
    private int[] scores = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10];

    [Benchmark]
    public void Foreach()
    {
        foreach (int i in scores)
        {
            Console.WriteLine($"The value of Score #{Array.IndexOf(scores, i)} is {i}.");
        }
    }

    [Benchmark]
    public void For()
    {
        for (int i = 0; i < scores.Length; i++)
        {
            Console.WriteLine($"The value of Score #{i} is {scores[i]}.");
        }
    }
}

I ran this in release mode without a debugger attached and got the following results:

Method Mean Error StdDev
Foreach 2.969 ms 0.0723 ms 0.1992 ms
For 2.822 ms 0.0564 ms 0.1426 ms

This jives with the general data presented initially, though it is notable that these are milliseconds, not seconds. I'm not sure why that is. I actually wonder if Benchmark.NET is doing something with the console input to make it 100x faster. It might be the computer I'm on. There are still a lot of variables here. But the point is that Benchmark.NET sorts out a bunch of ways that profiling and benchmarking can go wrong. So I trust its numbers better than anything I'll collect manually, and starting here, with mostly the original code, makes me feel good about things.

It is nice and convenient that the two versions tell the same basic story though: for seems faster than foreach, but only marginally so. I can live with that for now.

But now let's eliminate the Console.WriteLine part and strip it down to simple operations. I don't want it to be so simple that the code gets optimized into nothing. I've chosen to add up all the items in the array, but multiply them by their index, so that both the value and the index matter.

Here's the updated benchmark code:

using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;

BenchmarkRunner.Run<ForeachVsFor>();

public class ForeachVsFor
{
    private int[] scores = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10];

    [Benchmark]
    public int Foreach()
    {
        int sum = 0;
        foreach (int i in scores)
        {
            sum += i * Array.IndexOf(scores, i);
        }
        return sum;
    }

    [Benchmark]
    public int For()
    {
        int sum = 0;
        for (int i = 0; i < scores.Length; i++)
        {
            sum += scores[i] * i;
        }
        return sum;
    }
}
And the measured results: Method Mean Error StdDev
Foreach 31.548 ns 0.1310 ns 0.1161 ns
For 6.413 ns 0.0337 ns 0.0315 ns

Note that without the Console.WriteLines in there, what's left is tiny. We're not talking milliseconds. We're not even talking microseconds. We're talking _nano_seconds. But... foreach is now over 5x slower than for. Does it matter? Eh. Probably not that much. I have yet to encounter a situation where something was slow enough for me to worry, but changing things by a few nanoseconds made enough of a difference to matter.

So I'll reiterate what I wrote in that blog post: for is definitely the faster option, but foreach is often more readable. Most of the time, readability is a much more important quality than saving a few nanoseconds. I'll go with a foreach over a for (assuming it makes sense in the code) any day.

But do keep in mind some of the other things covered in that blog post. Arrays are extremely optimized. Other types don't get the same amount of optimization. The mechanical overhead of an arbitrary IEnumerator<T> will likely show even worse performance.

One last thing is that the foreach code will perform worse at a faster rate than the for code as the collection gets bigger. You mentioned big data style problems. Certainly, if you're talking about billions of elements, this type of thing will matter more. But how bad does it need to get? It depends on your tolerance. But let's throw in something bigger than 10 items, which is the realm of a toy problem, and try something that is a bit more organic without being embarrassingly large. I'll run it again with 100, 1000, and 10000 items.

100 Items: Method Mean Error StdDev
Foreach 725.34 ns 4.504 ns 3.992 ns
For 63.57 ns 0.392 ns 0.366 ns

That's a 10x increase, though we're still talking about nanoseconds--just now tens and hundreds of nanoseconds.

1000 Items: Method Mean Error StdDev
Foreach 54,646.1 ns 338.15 ns 316.31 ns
For 573.3 ns 4.16 ns 3.89 ns

The foreach code is now roughly 100x worse than the for version, and it is now measured in a tens of microseconds, rather than nanoseconds.

10,000 Items: Method Mean Error StdDev
Foreach 5,040.748 us 19.0581 us 17.8270 us
For 5.874 us 0.0219 us 0.0194 us

The foreach version--even though it is highly optimized because the compiler knows a ton about arrays--is now nearly 1000x slower. And while you can reasonably measure the for loop in microseconds (the us should be μs, but that doesn't work in a console window), the foreach version is now good and well into millisecond territory.

One more for fun. 100,000 items: Method Mean Error StdDev
Foreach 662,750.06 us 2,573.641 us 2,281.465 us
For 60.09 us 0.172 us 0.161 us

At this point, the for loop is measured in tens of microseconds while the foreach is 662 milliseconds--nearly two thirds of a full second, and 10,000x slower.

At this point, our dataset is definitely big. I don't know that I'd put it into the "Big Data" category yet, though we're getting there.

For what it is worth, this is because the foreach version is an O(n^2) algorithm (an "order n squared algorithm") while the for version is an O(n) (an "order n algorithm") meaning as the number of items in the collection gets bigger (n) the time it takes grows at a rate of n * n/n^2. It will get very slow much faster than the O(n) version. (That's all very technical. I think Big-O notation is useful for programmers, but if you don't already know it, it is a topic for another day.)

So I'd summarize these results by saying The foreach version begins slower than the for version, and gets much worse long before you reach Big Data territory. When you're still in "big but not crazy big" territory.

I might suggest this tweak to the foreach code to bring it back to O(n) Land. The expensive part of that code is really the IndexOf, which must look through the collection until it finds what it is looking for, and must do so in each pass of the foreach. This version sidesteps that.

int index = 0;
foreach (int score in scores)
{
    Console.WriteLine("Score at index {index} is {score}.");
    index++;
}

We have an index without searching the full dataset for it, and update it on the side as we go. It is what I've used in the rare cases where a foreach is the right choice but where I also need an index.

I'm not going to have time to profile it tonight, but I suspect it will be slightly worse than the for version, but should stay close, since they're both O(n) algorithms at that point. So it shouldn't ever end up 10,000x worse or anything.

Here's the code I had at the end, in case you want to run it:

using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;

BenchmarkRunner.Run<ForeachVsFor>();

public class ForeachVsFor
{
    private int[] scores;

    [GlobalSetup]
    public void Setup()
    {
        int count = 100_000;
        scores = new int[count];
        for (int i = 0; i < count; i++)
            scores[i] = i;
    }

    [Benchmark]
    public int Foreach()
    {
        int sum = 0;
        foreach (int i in scores)
        {
            sum += i * Array.IndexOf(scores, i);
        }
        return sum;
    }

    [Benchmark]
    public int For()
    {
        int sum = 0;
        for (int i = 0; i < scores.Length; i++)
        {
            sum += scores[i] * i;
        }
        return sum;
    }
}
TechProofreader commented 6 months ago

Thank you for the detailed and timely response!

I just wanted to tie up the discussion with some interesting tidbits for anyone else who comes across this post:

In looking further into what you explained, it appears that Microsoft has had performance related issues with iterating over arrays and lists using FOR and FOREACH loops. Regarding arrays, Microsoft apparently designed the JIT to translate FOR and FOREACH C# code into the same IL language when iterating over arrays. As such, the performance between either looping method was supposed to be negligible. With that in mind, it seems like the ballooning bottleneck here is the use of the Array.IndexOf(Array, Object) since, as you explained, it's an enumeration method, which certainly isn't going to be fun on large data sets.

Back in 2009, Jon Skeet actually ran some benchmark tests on FOR and FOREACH loops, iterating ten thousand times over one million integers via an array within each loop style, and he found the performance results to be nearly identical, with FOR loops being slightly faster. He also made a blog post on this matter here.

In 2022, Nick Chapsas discovered that, in .NET 6, C# treated FOR and FOREACH loops as mostly the same over arrays, but treated lists entirely differently when iterating over them between a FOR and FOREACH loop. Nick then discovered that, in trying to equalize the performance of iterating over lists with FOR and FOREACH loops, Microsoft wound up degrading the performance of FOR loops involving lists in .NET 7. Microsoft then had to rollback some framework updates. From what I remember, this is all supposed to be fixed in .NET 8, but I haven't gotten around to checking yet. Here's a link to Nick's video on this topic, and here's a link to his GitHub page showing the raw results..

Here's the link to the GitHub pull request discussing some of these issues.

After reading through some other discussions on this topic, everyone came to the same conclusion: .NET is weird and there's still strange behavior occurring when iterating over various collections using FOR vs FOREACH loops, and that measuring is the only reliable option for performance verification since everyone's hardware and code is different.

In a nutshell: .NET and C# is a quirky pair lol.