Inline List.iter/List.iteri for performance

[TIHan]

This inlines List.iter/List.iteri for performance reasons. Normally I do not like inlining core functions in FSharp.Core because of the potential to expose internals. But, this does not show any of those downsides.

This is in reaction to this PR: #8175

It doesn't solve the main problem that the PR describes though. There is optimization behavior that is still causing a function allocation.

[auduchinok]

@TIHan Could it also be applied to arrays and sequences iteration?

[TIHan]

@auduchinok
Some of the common array APIs are already inlined and might not cause a function allocation, but not in all cases.

There is interesting behavior when it comes to the inlining of functions. The compiler can decide to inline a function even when it is not marked inline; there is a heuristic called lambdaInlineThreshold which is based on function size. There are caveats with that approach, especially when you get into nested functions and how the compiler decides to inline them or not.

AFAIK, a local function's implementation will be optimized first, which in itself could contain functions that could inline themselves. This affects the size of the overall function and may cause the function not to inline when called and have to create an allocation. This is the behavior of an inlined List.iter, which is why it was still allocating a function. Trying to understand how this mechanism works in the optimizer is difficult and will take time to fully grasp it. So far, my thought was to stop optimizing local functions if they are simple wrappers around other functions. They still get optimized when called because they might be inlined. However, it seems that CI is failing because it's creating IL that we don't expect, but it does not happen on my machine. This is a bit harder than I anticipated :)

[TIHan]

This is ready, just need a review.

[forki]

What happens if you revert your recent changes where you manually converted to for loop?

[TIHan]

So, the inline List.iter/List.iteri is fine, but I am going to separate out the specific optimizer change in a different PR.

[TIHan]

@forki I just built it and yea, it did not optimize it =/ - means I do not have a good test.

However, the test that is there is passing as a result of the change and failing if the change isn't in place.

Also, if I do not use partial application, it will be optimized with no allocation, even without the optimizer change.

and CheckTypesDeep cenv f g env tys = 
    tys |> List.iter (CheckTypeDeep cenv f g env true)

to this:

and CheckTypesDeep cenv f g env tys = 
    tys |> List.iter (fun ty -> CheckTypeDeep cenv f g env true ty)

[cartermp]

@TIHan want to remove the WIP?

[cartermp]

Nice. A bit surprised that these older, unoptimized functions for lists are still in use.

[forki]

Ok @TIHan so something is wrong with optimization of partial application? That should be investigated further

[TIHan]

@forki Definitely should be investigated. I'm still going to look at it.

[forki]

XOXO