dotnet / dotnet/fsharp

Poor codegen for 'use' binding on struct enumerator

Open
#5,291 0 comments 0 reactions 0 assignees View on GitHub
Area-Compiler-Optimization Feature Improvement
Dominant language
F#
Stars
4.3k
Forks
876
Avg merge
4d 11h
Merged PRs (30d)
131

Description

The F# compiler in VS 15.7.1 generates poor-quality IL for a ``use`` binding when the variable type is a struct known to implement ``IEnumerator<'T>``. (It may do this for all struct types implementing ``IDisposable`` directly or indirectly, I've only checked this one case.)

#### Repro steps

I had a bit of code where I was working with a struct enumerator type exposed by a C# library. I thought it might be interesting to try using F# statically-resolved type parameters to call ``GetEnumerator`` on the collection type to get the strongly-typed, struct-based enumerator for the class, then use it to implement some usual functions like ``fold`` with that enumerator in a way that'd avoid boxing it.

```fsharp
(*

Copyright 2018 Jack Pappas

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.

*)

/// Functional operations on sequences utilizing
/// custom enumerator types to avoid boxing.
[]
module ExtCore.Collections.StrongSeq

open System.Collections.Generic
open OptimizedClosures

[]
let foldStrong (folder : 'State -> 'T -> 'State) state (enumerator : ('TEnumerator :> IEnumerator<'T>)) : 'State =
let folder = FSharpFunc<_,_,_>.Adapt folder

let mutable enumerator' = enumerator
let mutable state = state
while enumerator'.MoveNext () do
state <- folder.Invoke (state, enumerator.Current)
state

[]
let inline fold< ^State, ^T, ^TSeq, ^TEnumerator
when ^TSeq: (member GetEnumerator: unit -> ^TEnumerator)
and ^TEnumerator :> IEnumerator< ^T > >
(folder : ^State -> ^T -> ^State) state (source : ^TSeq) : 'State =
// This 'use' binding causes unnecessary boxing of 'enumerator'.
use enumerator = (^TSeq : (member GetEnumerator: unit -> ^TEnumerator) (source))
foldStrong folder state enumerator

module Example =
let testFoldOnListT () =
let items = ResizeArray ([| 1; 1; 2; 3; 5; 8 |])

("", items)
||> fold (fun state x ->
state + string x)
```

1. Compile repro code in Release mode, targeting .NET Framework 4.5.

2. Examine IL generated for the ``testFoldOnListT`` function:

```
.method public static
string testFoldOnListT () cil managed
{
// Method begins at RVA 0x4aec8
// Code size 83 (0x53)
.maxstack 5
.locals init (
[0] class [mscorlib]System.Collections.Generic.List`1,
[1] class [FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2>,
[2] valuetype [mscorlib]System.Collections.Generic.List`1/Enumerator,
[3] string,
[4] class [mscorlib]System.IDisposable
)

IL_0000: ldc.i4.6
IL_0001: newarr [mscorlib]System.Int32
IL_0006: dup
IL_0007: ldtoken field valuetype ''/T155121_24Bytes@ ExtCore.Collections.StrongSeqModule/Example::field155122@
IL_000c: call void [mscorlib]System.Runtime.CompilerServices.RuntimeHelpers::InitializeArray(class [mscorlib]System.Array, valuetype [mscorlib]System.RuntimeFieldHandle)
IL_0011: newobj instance void class [mscorlib]System.Collections.Generic.List`1::.ctor(class [mscorlib]System.Collections.Generic.IEnumerable`1)
IL_0016: stloc.0
IL_0017: newobj instance void ExtCore.Collections.StrongSeqModule/Example/testFoldOnListT@65::.ctor()
IL_001c: stloc.1
IL_001d: ldloc.0
IL_001e: callvirt instance valuetype [mscorlib]System.Collections.Generic.List`1/Enumerator class [mscorlib]System.Collections.Generic.List`1::GetEnumerator()
IL_0023: stloc.2
.try
{
IL_0024: ldloc.1
IL_0025: ldstr ""
IL_002a: ldloc.2
IL_002b: call !!0 ExtCore.Collections.StrongSeqModule::FoldStrong>(class [FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2>, !!0, !!2)
IL_0030: stloc.3
IL_0031: leave.s IL_0051
} // end .try
finally
{
IL_0033: ldloc.2
IL_0034: box valuetype [mscorlib]System.Collections.Generic.List`1/Enumerator
IL_0039: isinst [mscorlib]System.IDisposable
IL_003e: stloc.s 4
IL_0040: ldloc.s 4
IL_0042: brfalse.s IL_004e

IL_0044: ldloc.s 4
IL_0046: callvirt instance void [mscorlib]System.IDisposable::Dispose()
IL_004b: ldnull
IL_004c: pop
IL_004d: endfinally

IL_004e: ldnull
IL_004f: pop
IL_0050: endfinally
} // end handler

IL_0051: ldloc.3
IL_0052: ret
} // end of method Example::testFoldOnListT
```

#### Expected behavior

The IL generated for the ``finally`` clause (from the ``use`` binding in the ``fold`` function above) should be more like this:
```
finally
{
IL_0033: ldloc.2
IL_0034: constrained. valuetype [mscorlib]System.Collections.Generic.List`1/Enumerator
IL_0039: callvirt instance void [mscorlib]System.IDisposable::Dispose()
IL_003e: endfinally
}
```

#### Actual behavior

The IL generated for the ``finally`` clause unnecessarily boxes the enumerator (a struct implementing ``IEnumerator<'T>``, which means it also implements ``IDisposable``).

The compiler is also performing a type test (``box`` followed by ``isinst`` to check whether it implements ``IDisposable``) on the local variable of type ``System.Collections.Generic.List`1/Enumerator``, even though that type is known at compile-time to implement ``IDisposable``. (Anything we create a ``use`` binding for must implement ``IDisposable`` by definition, so the ``finally`` generated for a ``use`` binding should never contain this type test. The null check only needs to be there when the type of the variable being bound with ``use`` is not known to be a struct type; even if the struct would need to be boxed for some reason, it'd never be null.)

Both of these issues seem to be caused by the way the compiler is handling the inlining + static type parameter resolution for the ``fold`` function (above). I created a small example where I've manually implemented something similar to what I'm doing with ``testFoldOnListT`` and ``fold``, and the generated IL is reasonably close to what I listed in the "expected behavior" section above.

```fsharp
let enumeratorWithoutBoxing () =
let items = ResizeArray ([| 1; 1; 2; 3; 5; 8 |])
let mutable output = ""
use mutable enumerator = items.GetEnumerator ()
while enumerator.MoveNext () do
output <- output + string enumerator.Current
output
```

``finally`` block for the ``use`` binding in the ``enumeratorWithoutBoxing`` function:

```
finally
{
IL_0086: ldloca.s 2
IL_0088: constrained. valuetype [mscorlib]System.Collections.Generic.List`1/Enumerator
IL_008e: callvirt instance void [mscorlib]System.IDisposable::Dispose()
IL_0093: ldnull
IL_0094: pop
IL_0095: endfinally
}
```

#### Known workarounds

I haven't come up with any workarounds yet.

#### Related information

* Visual Studio 15.7.1

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.