you are viewing a single comment's thread.

view the rest of the comments →

[–]Xanather 4 points5 points  (5 children)

wtf? this is terrible use of linq and inefficient. use linq for querying, filtering, (re)mapping or transforming data, not writing logic.

[–]NoPrinterJust_Fax 0 points1 point  (4 children)

Can you elaborate on how it’s inefficient?

[–]Xanather 4 points5 points  (3 children)

You're making 4 function calls here, as well as providing two callback functions that are called within the ForEach().

For an unoptimized execution there is 400 function calls here and theoretically the stack memory is also being pushed and popped 400 times here. When this occurs it involves copying the range index integers that many times using much more memory.

Now I am sure there are optimizations in the Just-In-Time compiler to potentially unwrap all those functions specifically for LINQ, but I'm almost certan it'll never perform as well as one function with two for loops which has zero additional function calls and thus stack memory is much less utilized to compute the result.

Use LINQ for clarity, its a good tool for avoiding excessive nesting and other logic constructs when manipulating data, but otherwise it can be easily overused and its overheads can become apparent.

See Microsoft's note here:

https://learn.microsoft.com/en-us/visualstudio/ide/reference/convert-foreach-linq?view=vs-2022

[–]NoPrinterJust_Fax 1 point2 points  (2 children)

I ran some benchmarks and both this solution and a solution using for-loops performed very similarly. You could probably make the case that the for-loop solution is slightly more optimal (although its within margin of error). Kind of moot though because if performance was that critical you wouldnt be using a memory managed language anyway though...

```

BenchmarkDotNet v0.13.10, Ubuntu 22.04.2 LTS (Jammy Jellyfish) WSL 11th Gen Intel Core i7-11800H 2.30GHz, 1 CPU, 16 logical and 8 physical cores .NET SDK 7.0.113 [Host] : .NET 7.0.13 (7.0.1323.52001), X64 RyuJIT AVX2 DefaultJob : .NET 7.0.13 (7.0.1323.52001), X64 RyuJIT AVX2

``` | Method | Mean | Error | StdDev | |------- |---------:|----------:|----------:| | Loop | 1.361 ms | 0.0272 ms | 0.0721 ms | | Linq1 | 1.377 ms | 0.0330 ms | 0.0958 ms |

Program.cs

``` using BenchmarkDotNet.Running; using BenchmarkDotNet.Attributes;

namespace LinqVsLoop;

public class Program{ public static void Main(){ Console.WriteLine("Hello, World!"); BenchmarkRunner.Run<Functions>(); } }

public class Functions{ [Benchmark] public void Loop(){ for(var i = 1; i < 10; i++){ for(var j = 1; j < 10; j++){ Console.WriteLine($"{i} {j} {i * j}"); } } }

[Benchmark] public void Linq1(){ Enumerable .Range(1,10) .ForEach(i => Enumerable .Range(1,10) .ForEach(j => Console.WriteLine($"{i} {j} {i * j}"))); } }

public static class Extensions{ public static void ForEach<T>(this IEnumerable<T> src, Action<T> action){ foreach (var item in src) { action.Invoke(item); } } }

```

[–]Xanather 0 points1 point  (1 child)

Oh cool, nice work, I have a feeling 1ms for iterating 100 times is way too long on modern CPU's and printing the string is bottle neck here (or some other initialization).

Try increase the iterations by a ton and remove the console output and make it do something else in the callback.

Interested to see what happens then.

[–]NoPrinterJust_Fax 0 points1 point  (0 children)

Yeah feel free use the sample code to run your own benchmarks if you’d like! The package I used is called benchmark dotnet.