Use for..in..do instead of List.iter to prevent function allocations

[TIHan]

In these specific cases for post inference checks, using List.iter is causing a lot of function allocations. Can be resolved using for..in..do, which is optimized for list meaning that it does not use the enumerator.

[forki]

What about changing the compiler to rewrite this?

[TIHan]

I want to do that, but it may not be straight forward as we think. It would be quite an interesting optimization. For Array.iter, we inlined the function for that case. I do not want to inline List.iter.

[TIHan]

The easiest thing would be to inline List.iter though. I checked to see if we would need to expose any private internals in order for it to work, but it seems that we might be ok.

I'm generally not comfortable inlining functions in FSharp.Core in order to obtain performance due to potentially exposing private internals that should not be exposed.

[TIHan]

Well, we can't quite inline List.iter - since it requires recursion.

[forki]

Does the implementation of List.iter use a for loop? If this is not possible we could try to use similar techniques like in my seq.map fusion PR. Will Smith <notifications@github.com> schrieb am So., 12. Jan. 2020, 14:40:

…

Well, we can't quite inline List.iter - since it requires recursion. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#8175?email_source=notifications&email_token=AAAOANC2MX2YXJ67Z2HSMDLQ5MMVVA5CNFSM4KFXTPFKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIW2ICI#issuecomment-573416457>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAOANGCAVU5VRZVD7ZHMADQ5MMVVANCNFSM4KFXTPFA> .

[TIHan]

It doesn't use a for loop, but I think we could re-implement it using a for loop though.

[TIHan]

I tried inlining List.iter, while it inlined correctly, it didn't stop the function allocation:

internal static void CheckTypesDeep(cenv cenv, FSharpFunc<Tast.TType, Unit> f_0, FSharpOption<FSharpFunc<bool, FSharpFunc<Tast.EntityRef, Unit>>> f_1, FSharpOption<FSharpFunc<Tuple<Tast.EntityRef, FSharpList<Tast.TType>>, Unit>> f_2, FSharpOption<FSharpFunc<Tast.TraitConstraintSln, Unit>> f_3, FSharpOption<FSharpFunc<Tuple<env, Tast.Typar>, Unit>> f_4, TcGlobals.TcGlobals g, env env, FSharpList<Tast.TType> tys)
{
	FSharpFunc<Tast.TType, Unit> fSharpFunc = new CheckTypesDeep@382(cenv, f_0, f_1, f_2, f_3, f_4, g, env);
	FSharpList<Tast.TType> fSharpList = tys;
	for (FSharpList<Tast.TType> tailOrNull = fSharpList.TailOrNull; tailOrNull != null; tailOrNull = fSharpList.TailOrNull)
	{
		Tast.TType headOrDefault = fSharpList.HeadOrDefault;
		fSharpFunc.Invoke(headOrDefault);
		fSharpList = tailOrNull;
	}
}

Why is it still allocating the function? I have no idea. It's probably an optimization bug.

Below is just using this PR:

internal static void CheckTypesDeep(cenv cenv, FSharpFunc<Tast.TType, Unit> f_0, FSharpOption<FSharpFunc<bool, FSharpFunc<Tast.EntityRef, Unit>>> f_1, FSharpOption<FSharpFunc<Tuple<Tast.EntityRef, FSharpList<Tast.TType>>, Unit>> f_2, FSharpOption<FSharpFunc<Tast.TraitConstraintSln, Unit>> f_3, FSharpOption<FSharpFunc<Tuple<env, Tast.Typar>, Unit>> f_4, TcGlobals.TcGlobals g, env env, FSharpList<Tast.TType> tys)
{
	FSharpList<Tast.TType> fSharpList = tys;
	for (FSharpList<Tast.TType> tailOrNull = fSharpList.TailOrNull; tailOrNull != null; tailOrNull = fSharpList.TailOrNull)
	{
		Tast.TType ty = fSharpList.HeadOrDefault;
		CheckTypeDeep(cenv, f_0, f_1, f_2, f_3, f_4, g, env, isInner: true, ty);
		fSharpList = tailOrNull;
	}
}

--

Here are some examples to test out:

open System
open System.Runtime.CompilerServices

let inline iter action (list: 'T list) =
    for x in list do action x
    
//[<MethodImpl(MethodImplOptions.NoInlining)>]
let checkIt (a: int) (b: int) (c: int) (d: int) (e: int) (x: string) = if x = "test" then printf "test"

let check1 a b c d e (x: string list) =
    x |> iter (checkIt a b c d e)
    
let check2 a b c d e (x: string list) =
    for xx in x do checkIt a b c d e xx

check1 will still allocate a function while check2 does not:

    public static void check1(int a, int b, int c, int d, int e, FSharpList<string> x)
    {
        FSharpFunc<string, Unit> fSharpFunc = new check1@11();
        FSharpList<string> fSharpList = x;
        for (FSharpList<string> tailOrNull = fSharpList.TailOrNull; tailOrNull != null; tailOrNull = fSharpList.TailOrNull)
        {
            string headOrDefault = fSharpList.HeadOrDefault;
            fSharpFunc.Invoke(headOrDefault);
            fSharpList = tailOrNull;
        }
    }

    public static void check2(int a, int b, int c, int d, int e, FSharpList<string> x)
    {
        FSharpList<string> fSharpList = x;
        for (FSharpList<string> tailOrNull = fSharpList.TailOrNull; tailOrNull != null; tailOrNull = fSharpList.TailOrNull)
        {
            string headOrDefault = fSharpList.HeadOrDefault;
            if (string.Equals(headOrDefault, "test"))
            {
                PrintfFormat<Unit, TextWriter, Unit, Unit> format = new PrintfFormat<Unit, TextWriter, Unit, Unit, Unit>("test");
                PrintfModule.PrintFormatToTextWriter(Console.Out, format);
            }
            fSharpList = tailOrNull;
        }
    }

--
But, if you get rid of just one parameter, then they are both identical and don't allocate and notice that check2 didn't inline checkIt like before:

let checkIt (a: int) (b: int) (c: int) (d: int) (x: string) = if x = "test" then printf "test"

let check1 a b c d (x: string list) =
    x |> iter (checkIt a b c d)
    
let check2 a b c d (x: string list) =
    for xx in x do checkIt a b c d xx

    public static void check1(int a, int b, int c, int d, FSharpList<string> x)
    {
        FSharpList<string> fSharpList = x;
        for (FSharpList<string> tailOrNull = fSharpList.TailOrNull; tailOrNull != null; tailOrNull = fSharpList.TailOrNull)
        {
            string headOrDefault = fSharpList.HeadOrDefault;
            checkIt(a, b, c, d, headOrDefault);
            fSharpList = tailOrNull;
        }
    }

    public static void check2(int a, int b, int c, int d, FSharpList<string> x)
    {
        FSharpList<string> fSharpList = x;
        for (FSharpList<string> tailOrNull = fSharpList.TailOrNull; tailOrNull != null; tailOrNull = fSharpList.TailOrNull)
        {
            string headOrDefault = fSharpList.HeadOrDefault;
            checkIt(a, b, c, d, headOrDefault);
            fSharpList = tailOrNull;
        }
    }

[TIHan]

In regards to using the inlined version of List.iter and still allocating the function, it has to do with the lambdaInlineThreshold in the optimizer.

[cartermp]

2.5% allocations in a sample is a little too much. Great change!