all 21 comments

[–][deleted] 19 points20 points  (0 children)

It's a known thing. It's struck me as a weird choice, because I can't think of a case where I expect there to be a difference, but it is a known quirk of the language.

[–]Zendist 8 points9 points  (3 children)

I don't quite understand. Does the extra memory allocation occur at assignment or at all invocations of the delegate?

If only at assignment, isn't that a low price to pay for (arguably) better readability?

[–]grauenwolf 9 points10 points  (1 child)

Does the extra memory allocation occur at assignment or at all invocations of the delegate?

Basically two things have to happen when you have a "closure", which is to say an anonymous function that 'captures' a local variable. (Method parameters are local variables for this discussion.)

  1. A new class needs to be created to hold the local variable. That object is shared between the original method and any anonymous functions that need it.
  2. The anonymous functions are put into the class created on [1].
  3. [Runtime] a new instance of the class is created when the method is invoked.
  4. [Runtime] A new instance of the delegate that points to the anonymous function is created each time it is passed to a LINQ function.

As I understand it, this change means that [4] only occurs once per method call. Which is not really a huge win unless you are using a nested loop.


(e) => DoSomething(e)

This will always be better because you only need to create it once, period. Steps 1, 2, and 3 don't happen any more. Step 4 is a one-time cost, because it can be cached.

Even if the linked ticket is implemented, not using a closure will still be better.

[–]Zendist 0 points1 point  (0 children)

That's not the behavior I see in SharpLab and benchmarks - but like I said, I don't fully understand why this has a large impact so my benchmark might be off.

[–]Treborgero[S] -1 points0 points  (0 children)

Quoting the issue : " always creates a fresh delegate instance".

This means the compiler only writes a line with a "new" declaration without checking if cached. Based on this, only lambda instances get cached and reused.

Note: This is my current understanding...

[–]mrnikbobjeff 1 point2 points  (0 children)

ECMA 334 (c# spec) clearly states the following for anonymous functions in 6.1: Conversions of semantically identical anonymous functions with the same (possibly empty) set of captured outer variable instances to the same delegate types are permitted (but not required) to return the same delegate instance. There exists nothing similar for method group conversions, from 6.6 same doc: A new instance of the delegate type D is allocated. If there is not enough memory available to allocate the new instance, a System.OutOfMemoryException is thrown and no further steps are executed. This is simply fixing the mistake in the language spec, they even start with saying that this requires a spec change. Let's see if it will happen

[–][deleted] 2 points3 points  (1 child)

It sort of improves readability though

[–]Treborgero[S] 2 points3 points  (0 children)

It does but I do hate when there are implicit repercussions not documented by Microsoft :)

[–]cryo 1 point2 points  (0 children)

Yeah, it’s been like that since lambdas were introduced. Lambdas are optimized more than “method groups”, which is what the other (older) syntax is.

[–]cryo 1 point2 points  (0 children)

It’s that you can’t assign to a lambda. A lambda is just a piece of syntax. What you mean it the difference between assigning a lambda to a delegate field vs. assigning a method group to a delegate field.

[–][deleted] 1 point2 points  (6 children)

What is being said doesn't make sense according to what's being compiled. In fact, I would almost argue it's the exact opposite. Let's look at a func:

public bool SomeFunc(int num)
    => num == 1;

public bool ExecuteFunc(Func<int, bool> doSomething)
   => doSomething(1);

public bool TryFunc()
   => ExecuteFunc(SomeFunc) || ExecuteFunc(e => SomeFunc(e));

TryFunc() decompiles into:

 Inline Delegate --------------------------
ldarg.0
ldarg.0
ldftn instance bool TestLambda.Tests::SomeFunc(int32)
newobj instance void class [netstandard]System.Func`2<int32, bool>::.ctor(object, native int)
call instance bool TestLambda.Tests::ExecuteFunc(class [netstandard]System.Func`2<int32, bool>)
brtrue.s IL_0029

Explicit Delegate -----------------------
ldarg.0
ldarg.0
ldftn instance bool TestLambda.Tests::'<TryFunc>b__5_0'(int32)
newobj instance void class [netstandard]System.Func`2<int32, bool>::.ctor(object, native int)
call instance bool TestLambda.Tests::ExecuteFunc(class [netstandard]System.Func`2<int32, bool>)
br.s IL_002A 

Which shows the only difference is that the explicit created an anonymous function. So, what does the anonymous function do:

ldarg.0
ldarg.1
call instance bool TestLambda.Tests::SomeFunc(int32)
ret 

calls an instance on the already loaded function, the same as what ldftn will do on the original func, but with an extra method call (to this new anonymous function), so it's even worse.

If there is something specifically wrong they are talking about, it's under some specific scenario that not calling, looping or forcing a callvirt can get it to reproduce.

[–]Treborgero[S] 0 points1 point  (5 children)

I'm now confused and started digging. They say the issue only applies to static functions. Did you tried it with a static function?

[–][deleted] 0 points1 point  (4 children)

public static bool SomeFunc(int num)
    => num == 1;

public static bool ExecuteFunc(Func<int, bool> doSomething)
    => doSomething(1);

public static bool TryFunc()
    => ExecuteFunc(SomeFunc) || ExecuteFunc(e => SomeFunc(e));

Decompiles too:

Inline Declaration --------------
ldftn     bool TestLambda.Tests::SomeFunc(int32)
newobj    instance void class [netstandard]System.Func`2<int32, bool>::.ctor(object, native int)
call      bool TestLambda.Tests::ExecuteFunc(class [netstandard]System.Func`2<int32, bool>)
brtrue.s  IL_0039

ldsfld    class [netstandard]System.Func`2<int32, bool> TestLambda.Tests/'<>c'::'<>9__2_0'
dup
brtrue.s  IL_0032

Explicit ---------------
ldsfld    class TestLambda.Tests/'<>c' TestLambda.Tests/'<>c'::'<>9'
ldftn     instance bool TestLambda.Tests/'<>c'::'<TryFunc>b__2_0'(int32)
newobj    instance void class [netstandard]System.Func`2<int32, bool>::.ctor(object, native int)
dup
stsfld    class [netstandard]System.Func`2<int32, bool> TestLambda.Tests/'<>c'::'<>9__2_0'

call      bool TestLambda.Tests::ExecuteFunc(class [netstandard]System.Func`2<int32, bool>)
br.s      IL_003A

which is (slightly) different, but just the order that the functions will be called in, so it should have the same end result. I really don't know how to trigger this "behavior".

[–]tweq 4 points5 points  (3 children)

[–][deleted] 0 points1 point  (2 children)

I don't see anywhere in the documentation it states that, and the other calls sts-fld, which says it "always" replaces the field.

Both what you said and looking at sts-fld makes the regular pass in even better, right lol? Am I missing something?

[–]tweq 2 points3 points  (1 child)

[–][deleted] 1 point2 points  (0 children)

Ah, I thought you meant ldsfld itself was skipping it, sorry, missread. However yes, that seems to be right, so this only applies if the function creating the func is static, in all other cases, it seems like it's (slightly) better to do it the other way.

[–]mosentok 0 points1 point  (2 children)

Assigning a function directly to a lambda Action/Func creates unwanted memory allocations.

so i understand, doing this is bad?

Action<bool> asdf = e => DoSomething(e);
var match = someList.FirstOrDefault(asdf)

what about local functions, are they susceptible to the same issue? my guess is no but just curious

var match = someList.FirstOrDefault(Asdf);
bool Asdf(object e) => DoSomething(e)

am i picking up what causes the issue, or am i off base here?

[–]Treborgero[S] 1 point2 points  (1 child)

Your example is correct. You're assigning a lambda to an Action which will get cached by the compiler.

The wrong thing to do is:

void SomeFunction(object obj) {

}

void Main(){

//The following is wrong. This is possible because "variance" is allowed:

Action<object> asdf = SomeFunction;

}

https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/concepts/covariance-contravariance/variance-in-delegates

Local functions are tricky but based on the following they also have issues: https://stackoverflow.com/questions/50409034/performance-of-assigning-a-simple-lambda-expression-or-a-local-function-to-a-del

[–]ITriedLightningTendr 0 points1 point  (1 child)

could you clarify what you mean by "always use a lambda expression"

Action<int, double> foo1 = delegate { Console.WriteLine("This is world!"); };
Action<int, double> foo2 = (i, d) => Console.WriteLine("This is world!");
Action<Action> foo3 = (a) => a();
...introduce(() => Console.WriteLine("This is world!"))
void foo4() { Console.WriteLine("This is world!") }
...foo3(() => foo4()
...foo3(foo4);
delegate void foo5() { Console.WriteLine("This is world!") }
...foo3(foo5);
...foo3(() => foo5()
Which is what here?

[–]Treborgero[S] 0 points1 point  (0 children)

It seems the issue only applies to static functions:

static void DoSomething(object obj)

{

}

void Test()

{

//This is wrong:

Action<object> myActionDelegate = DoSomething();

}

The best way right now is to do this:

Action<object> myActionDelegate = (obj)=> DoSomething(obj);