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

RawFSharpAssemblyDataBackedByLanguageService produces large amount of data both in Gen1 and LOH causing UI delays in VS #5933

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

Comments

@davkean
Copy link
Member

davkean commented Nov 20, 2018

Trace: https://developercommunity.visualstudio.com/content/problem/245320/coloring-typing-tooltips-and-intellisense-slow-in.html
Trace: https://developercommunity.visualstudio.com/content/problem/245786/slow-f-editing-experience-up-to-a-minute-until-typ.html

Gen Count MaxPause MaxPeak MB Max AllocMB/sec TotalPause TotalAlloc MB Alloc MB/MSec GC Survived MB/MSec GC MeanPause Induced
ALL 1844 123.2 686.4 3,358.514 17,263.7 11,083.7 0.6 0.475 9.4 0
0 336 20.4 680.4 3,358.514 1,509.5 2,459.5 0.1 1.033 4.5 0
1 1501 33.4 686.4 1,110.648 15,009.3 8,597.9 1.4 0.227 10.0 0
2 7 123.2 686.4 50.786 744.9 26.2 0.1 313.871 106.4 0

This class is producing huge amounts 4 GB of data in the above trace:
image

450 MB of this ends up on the large object heap:

image

@davkean
Copy link
Member Author

davkean commented Nov 20, 2018

@TIHan
Copy link
Contributor

TIHan commented Nov 20, 2018

Driver.EncodeInterfaceData seems to be the culprit of this.

@cartermp
Copy link
Contributor

cartermp commented Jan 7, 2019

Oh holy schmackos:

https://github.com/Microsoft/visualfsharp/blob/66b20f9d79933c5f284a66d09069fa91d976e6b9/src/fsharp/TastPickle.fs#L746-L805

This ends up allocating a 100KB array in memory twice:

https://github.com/Microsoft/visualfsharp/blob/fead0aac540485683f694524eadad79983ec28d9/src/absil/bytes.fs#L65-L67

@davkean
Copy link
Member Author

davkean commented Jan 7, 2019

Yeah that would do it.

@cartermp
Copy link
Contributor

cartermp commented Jan 7, 2019

Related, from that same trace:

image

ByteBuffer.Ensure also allocates lots of data into the LOH:

https://github.com/Microsoft/visualfsharp/blob/9b55eccd1bc83ed123c5ae08c577be5646861830/src/absil/bytes.fs#L69-L74

@TIHan
Copy link
Contributor

TIHan commented Jan 8, 2019

After digging into this for a day and half, this is a bit complicated to fix.

First, getting the ByteBuffer to be a chunked array isn't trivial because ilwrite.fs depends upon a byte []. We would have to make some interesting changes to ilwrite.fs to understand how to deal with chunked arrays rather than a single array. I think this is fine, but it would mean exposing ByteBuffer as a public type for ILResourceLocation. We would need to change what ILResourceLocation is. Currently it's a DU which I believe it should not be; should be a class.

Another approach would be to add a ChunkedByteBuffer and only use it for F# metadata. However, it still requires changes to ILResourceLocation and parts of ilwrite.fs for it to understand how to get the data.

Before we fix ByteBuffer and F# metadata writing, we should probably redesign ILResourceLocation and/or ILResource to be a better abstraction and get that functioning properly. After that, then we can come back to handling F# metadata differently.

@dsyme
Copy link
Contributor

dsyme commented Jan 9, 2019

In addition to looking at the shape/format of the allocated data, I think we need to dig into why RawFSharpAssemblyDataBackedByLanguageService is being created so often.

My intuition is that one memoized instance of this should be forced each time ParseAndCheckProjectImpl is run to completion, causing a GetCheckResultsAndImplementationsForProject, causing a IncrementalBuild.Eval on ``finalizedTypeCheckNode. This result should be saved into the partial build for the project (save` = `SavePartialBuild`) and should not be evaluated again unless something changes in the project.

Now ParseAndCheckProjectImpl is called by both the FCS entry point ParseAndCheckProject and the contents of transitive project references are required via IProjectReference.EvaluateRawContents. It is particularly important to discover if it is referenced projects that are being checked again (IProjectReference.EvaluateRawContents), or the current project (ParseAndCheckProject).

If it is rechecking the current project that is causing this then we actually don't need to create RawFSharpAssemblyDataBackedByLanguageService for this case. We could add an extra node to the incremental build graph to ensure this is only created for transitive references.

If it is rechecking referenced projects then something is fishy. I feel it's likely that something is going wrong with memoizing these results in the incremental build graph: I suspect something is forcing referenced projects to be checked over and over again. Perhaps we have even fixed a bug in that area?

I think the right approach is to get Trace logging which prints out the causal trace for the RawFSharpAssemblyDataBackedByLanguageService creation each time it is created. The best causal trace printing we have is via the userOpName which we pass around. Passing this down to RawFSharpAssemblyDataBackedByLanguageService and emitting a Trace.WriteLine call may just do the trick to get a trace that tells us why re-evaluations are happening.

@davkean
Copy link
Member Author

davkean commented Jan 9, 2019

If you just need a stack, PerfView can do that for you. I'm of the same feeling, as I called out in #5938 it feels like we're doing a bunch of work over and over again and missing the cache.

@TIHan
Copy link
Contributor

TIHan commented Jan 9, 2019

If it is rechecking the current project that is causing this then we actually don't need to create RawFSharpAssemblyDataBackedByLanguageService for this case. We could add an extra node to the incremental build graph to ensure this is only created for transitive references.

I assumed this was already happening, but we should get a trace to see.

@cartermp
Copy link
Contributor

We should have the trace to see if that's the case

@dsyme
Copy link
Contributor

dsyme commented Apr 1, 2022

This is mitigated already by @TIHan's work to use array pooling for the buffers. I did further mitigation in #12922, and will close this until we get remeasurement of an LOH problem for this. It might still happen when working a lot with large assemblies but the core common issue reported here should be gone I believe.

@dsyme dsyme closed this as completed Apr 1, 2022
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

5 participants