Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Type providers usage of FSharpCoreAssemblyVersion is resulting in large LOH allocations and UI delays inside of VS #5930

Closed
davkean opened this issue Nov 20, 2018 · 8 comments

Comments

@davkean
Copy link
Member

davkean commented Nov 20, 2018

Trace: https://developercommunity.visualstudio.com/content/problem/379653/vs-slow-typing-and-hang-1.html.

GC Rollup By Generation

Gen Count MaxPause MaxPeak MB Max AllocMB/sec TotalPause TotalAlloc MB Alloc MB/MSec GC Survived MB/MSec GC MeanPause Induced
ALL 305 13,100.2 2,515.3 9,389.575 23,078.5 2,845.1 0.1 13.147 75.7 1
0 187 20.6 2,481.5 4,736.269 1,031.3 1,109.3 0.4 0.050 5.5 0
1 108 13,100.2 2,509.7 9,389.575 14,950.0 1,486.1 5.3 0.199 138.4 0
2 10 728.8 2,515.3 14.396 7,097.2 249.7 2.5 20,674.062 709.7 1

Type Providers usage of FSharpCoreAssemblyVersion in the above trace is causing 250 MB of LOH allocations:

image

@davkean
Copy link
Member Author

davkean commented Nov 20, 2018

This is very similar to #5931 I think, in that this is probably reading FSharp.Core.dll?

@baronfel
Copy link
Member

did some manual counts by debugging some tests in the TPSDK, and the big hit seems to be the ILModuleReader constructor. When we load mscorlib and FSharp.Core in order to read the FSharp.Core assembly version, the sizes/counts of the byte[] and strings in memory balloon wildly.

Some numbers:

Stage		Byte[]	Size		String[]	Size		String	Size
Pre-mscorlib	431	130872	        411		24360	        6180	953048
Post-mscorlib	438	959528	        414		24728	        6284	958320

// some bad stuff happens because my debugger broke, so I restarted 
// the magnitude still shows the effect though
Pre-f# core	1590	9508648         340	        18760	        16453	1559816
Post-f# core	1609	11844312	345		19208	        16534	1563992

Sizes are in bytes, but the effect seems to be apparent.

This should be a one-time hit because these module readers are held in a cache, but /shrug to that.

@cartermp
Copy link
Contributor

cartermp commented Jan 8, 2019

I think this is still fallout from #5929, since each Type Provider instantiation is having to do this large allocation whenever it's created. So if that issue can be addressed, this is less severe. Actually, I can't math - this is just big LOH allocations

There's ... a lot going on in the Type Provider SDK that probably ought not to be. The offending function(s) involved all allocate huge byte[] arrays into memory (e.g., FSharp.Core is 2.7 MB) which are all going on to the LOH (link). But that work seems unnecessary, since we can know where things like the FSharp.Core and System.Object being referenced live. The compiler has to know this information otherwise it couldn't even compile things. Unfortunately, we don't own this code path since it's entirely in the TPSDK.

@cartermp
Copy link
Contributor

cartermp commented Jan 9, 2019

Note that this also takes up notable CPU time:

image

image

image

@dsyme
Copy link
Contributor

dsyme commented Jan 9, 2019

That piece of code in the TPSDK doing byte array comparison has already been fixed https://github.com/fsprojects/FSharp.TypeProviders.SDK/pull/284/files#diff-7c540ae6a67f14c48d0403e7b021fc86R6646, we just need to update the various type providers to use this.

There are still too many ILModuleReader being created (we can save them in a time cache) but the aove should fix most things.

@cartermp
Copy link
Contributor

cartermp commented Jan 9, 2019

Hmmm, but it's still cracking open FSharp.Core and mscorlib/System.Runtime/netstandard and reading them into big bytes arrays just to find information that the compiler should already have. I suppose that's a bigger change though.

@dsyme
Copy link
Contributor

dsyme commented Jan 11, 2019

Hmmm, but it's still cracking open FSharp.Core and mscorlib/System.Runtime/netstandard and reading them into big bytes arrays just to find information that the compiler should already have. I suppose that's a bigger change though.

Correct.

@cartermp
Copy link
Contributor

Okay. I'll create an issue on the TPSDK since this ultimately a concern over there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants