Read/store ILCustomAttrs, ILSecurityDecls, ILTypeDef sensibly

[dsyme]

The memory trace here shows a Gomorrah of objects associated with ILCustomAttrs, ILSecurityDecls, ILTypeDef, accounting for 10-20% (perhaps more) of long-term managed memory use when editing in VisualFSharp.sln. The same will be true in other large solutions.

This has long been a problem and instead of fiddling (e.g. reducing the size of these objects) it's better to solve it more properly.

The underlying problem is that we rely too much on per-TypeDef, per-MethodDef, per-PropertyDef etc. closures to capture the continuations needed to read the rest of these data structures. In each case, all that's really needed is a single shared closure (which captures the reading context) plus a metadata token. This PR implements this.

This should eliminate most of these items from the linked comment:

Microsoft.FSharp.Compiler.AbstractIL.IL+ILAttributes	600,800	14,727,824	17,796,800
Microsoft.FSharp.Compiler.AbstractIL.ILBinaryReader+seekReadCustomAttrs@2515	572,532	11,450,640	8,447,272
Func<Microsoft.FSharp.Collections.FSharpList<Microsoft.FSharp.Compiler.AbstractIL.IL+ILPermission>>	251,455	8,046,560	3,178,432
Lazy<Microsoft.FSharp.Collections.FSharpList<Microsoft.FSharp.Compiler.AbstractIL.IL+ILPermission>>	251,455	5,625,008	4,767,600
Microsoft.FSharp.Compiler.AbstractIL.ILBinaryReader+seekReadSecurityDecls@2537	251,455	5,029,100	993,260

TODO:

• validate that the object above are no longer present in a memory dump
• get it green

[dsyme]

@dotnet-bot Test Ubuntu16.04 Release_fcs Build please

[dsyme]

I can confirm that after applying this PR the objects are no longer in the dumps. Here's an example:

[dsyme]

Here's an analysis of the cumulative effect of changes #4586, #4590 , #4597.

Scenario:

• VisualFSharp.sln
• In-memory cross-project references off
• Opening two files - one fsc.fs and one in FSharp.Editor.dll
• Wait for color checking end symbols in both, then do Find All References on RoslynHelpers at end of second file

dev15.6: 1392MB private memory at steady state, 194MB FSharpChecker
Master + this PR: 854MB private memory at steady state, 90MB FSharpChecker

That's over 50% reduction in managed heap, and 38% reduction in overall memory usage.

dev15.6

Master + this PR:

[isaacabraham]

That's a ridiculously good result :-) What implications does this have in terms of performance - is there any reduction in perf?

[dsyme]

That's a ridiculously good result :-) What implications does this have in terms of performance - is there any reduction in perf?

Not as far as I'm aware. The only thing I'm aware of is that the F# compiler (fsc.exe, not Visual Studio) uses memory mapped files less often, preferring byte[] in some cases to avoid locking files.

[dsyme]

@dotnet-bot Test Ubuntu16.04 Release_default Build please

[forki]

'nuff said

[dsyme]

@isaacabraham Note this includes @AviAvni's memory work. It's great to see that, once ByteFile is removed, we've been focusing on the right data structures. Typar is indeed next on the list.

[AviAvni]

@dsyme Thanks for letting me know I can work on Typar just waited until you finish your great work

[TIHan]

This is wonderful.

[TIHan]

What are these doing? What is map2of2?

[AviAvni]

@TIHan in visualfsharp\src\fsharp\lib.fs
let map2Of2 f (a1,a2) = (a1,f a2)

[dsyme]

Yeah, I removed the groupByFirst helper (which was a reimplementation of groupBy) but then had to add these since the returned results were slightly different - I should have added a new groupByFirst helper implemented in terms of groupBy

[dsyme]

@Krzysztof-Cieslak @nosami @auduchinok FCS updated and published as FSharp.Compiler.Service 22.0.1

[eiriktsarpalis]

Great work. Is this change incorporated in the VS 15.7 previews?

[cartermp]

@eiriktsarpalis not yet. We'll need to make another insertion into a preview release branch before it goes out on that channel.