fsharp / fsharp-compiler-docs

Doc build for FSharp.Compiler.Service
https://fsharp.github.io/fsharp-compiler-docs
MIT License
279 stars 123 forks source link

FCS uses too much memory #158

Closed vasily-kirichenko closed 10 years ago

vasily-kirichenko commented 10 years ago

See the VFPT issue https://github.com/fsprojects/VisualFSharpPowerTools/issues/474#issuecomment-45424007

@dsyme suggested to check these caches first: https://github.com/fsharp/FSharp.Compiler.Service/blob/master/src/fsharp/vs/service.fs#L2235

It's a very serious problem: it causes VS to quickly occupy RAM at the point when everything becomes unusable (~1.8...2.5GB) due to continuous garbage collecting. In practice it means I have to restart VS every 0.5..1 hour during the day as I working on a solution with 50+ F# projects. Even intensive working on VFPT more than an hour is impossible.

I created a console with which I can quickly (a minute) check that the bug is fixed or not.

Please, fix it ASAP.

vasily-kirichenko commented 10 years ago

I removed Add / Put calls on incrementalBuildersCache, braceMatchMru, parseFileInProjectMru, typeCheckLookup and typeCheckLookup2. It has not helped at all. The process memory increases linearly up to 6th GetAllUsesOfAllSymbolsInFile call, then stabilize, then decreases (from 260MB to 160MB), then increases again.

So I suspect there're more caches in FCS.

dsyme commented 10 years ago

I tried out the console app. However I don't see a leak as such - memory use rises but "Peak Working Set" stabilizes at around 400MB.

So is the issue "VFPT/FCS uses too much memory on big solutions" or "FCS has a leak"?

vasily-kirichenko commented 10 years ago

Yes, the memory use stabilizes after 5-6 runs. So, the issue is "FCS uses too much memory". I'm not sure about "big" solutions since it easily eats 2GB ram on the small VFPT one.

vasily-kirichenko commented 10 years ago

From VFPT point of view we wanted FCS to allocate fixed amount of memory for each project. It may discard caches for projects but it certainly should not keep some "historical" caches for projects. As a result, subsequent GetAllUsesOfAllSymbolsInFile calls (for example) with same ProjectOptions (with different LoadTime) should not increase memory usage at all. Simply speaking, FCS should keep any cached data by ProjectFullName key, not by ProjectOptions.

dsyme commented 10 years ago

OK, it would be good to identify a concrete memory bug or actionable saving.

FCS uses memory for sure. How much it uses will be largely dictated by a combination of

I know you set the projectCacheSize parameter to 200 in order to improve cross-solution operations when there are lots of small projects. But always caching all projects (even 5-10) is to some extent asking for trouble - you're basically configuring FCS to say "memory be damned, cache everything!" :)

Does the memory use go so high when you set the number of cached projects back to something small (or disable the cache)?https://github.com/fsprojects/VisualFSharpPowerTools/blob/1333806aa332bc63293fd45740b7c3c830dd5e35/src/FSharpVSPowerTools.Core/LanguageService.fs#L125

I don't think there are any other signficant hidden caches involved, but I may be wrong.

dsyme commented 10 years ago

@vasily-kirichenko I'm trying to repro a situtation where VFPT uses 2GB+ when opening its own solution.

I did the following using the latest gallery published VFPT VSIX:

At this point memory use was around 1GB. I then started randomly typing into a few F# files and it went up a bit more, to around 1.2GB.

I didn't do any debugging or the like.

Based on this it feels like this solution should always stay within a 1.5GB memory budget, and if it doesn't (e.g. after renaming files, or changing other project options, or doing other VS activities or ...) then that would be considered a bug (either in VFPT or FCS) or would would at least motivate better caching mechanisms/policy.

So it would be good to have a repro where this solution goes over 2GB (even with projectCacheSize=200=cache-everything)?

thx

7sharp9 commented 10 years ago

Do we need to consider any of this for Xamarin Studio?

On 10 Jun 2014, at 09:42, Don Syme notifications@github.com wrote:

@vasily-kirichenko I'm trying to repro a situtation where VFPT uses 2GB+ when opening its own solution.

I did the following using the latest gallery published VFPT VSIX:

opened "FSharpVSPowerTools.sln" (after building it) opened some random files and did "Shift-F12" on a number of symbols opened every F# file in the solution (actually both Xaml and F#) At this point memory use was around 1GB. I then started randomly typing into a few F# files and it went up a bit more, to around 1.2GB.

I didn't do any debugging or the like.

Based on this it feels like this solution should always stay within a 1.5GB memory budget, and if it doesn't (e.g. after renaming files, or changing other project options, or doing other VS activities or ...) then that would be considered a bug (either in VFPT or FCS) or would would at least motivate better caching mechanisms/policy.

So it would be good to have a repro where this solution goes over 2GB (even with projectCacheSize=200=cache-everything)?

thx

— Reply to this email directly or view it on GitHub.

dsyme commented 10 years ago

@7sharp9 Eventually yes, though currently very large F# solutions are very, very likely to be using VS, and XS has fewer cross-solution features, and doesn't set projectCacheSize=200 AFAIK.

dsyme commented 10 years ago

@vasily-kirichenko I just read this comment (https://github.com/fsharp/FSharp.Compiler.Service/issues/158#issuecomment-45612283). Agreed that FCS should not keep any out-of-date caches keyed on ProjectOptions (though I'm still unsure whether this is causing significant increased memory use)

quasilord commented 10 years ago

@vasily-kirichenko is right - there can be out of date entries in typeCheckLookup and typeCheckLookup2. I have a fix for this.

vasily-kirichenko commented 10 years ago
  1. Load VFPT solution and Find all ResizeArray references. -> 980MB
  2. Checkout some old commit which causes VS to ask you to "reload all". Do it. Find all refs again. -> 1.2GB
  3. Repeat #2 -> 1,45GB
  4. Repeat -> 1.65GB
  5. Repeat -> 1.9GB
  6. Repeat -> 2.0GB
  7. Repeat -> 2.1GB and VS freezes then redraws itself before shows Find refs results (I mean the plane main window border appears for a moment, that the usual one is drawn).
  8. Repeat -> 2.3GB
  9. Repeat -> While reloading the solution VS freezes again, but memory is shrunk to 2.1GB 10 -> 2.2GB

image

(where colons are "private set" and "peak private set").

quasilord commented 10 years ago

Fixed in 0.0.52, published, see https://github.com/fsharp/FSharp.Compiler.Service/commit/d6a0644737d7a9d402872c35f3743e347dc521c0.

@vasily-kirichenko Your test program now exhibits stable memory after one "w", but only if GC.Collect() is added after each "w"

    printfn "Got %d symbol uses in %O" uses.Length sw.Elapsed
    GC.Collect()
    GC.Collect()

There can still be "stale" entries in caches like typeCheck and typeCheck2 if projects are simply no longer in use (e.g. unloaded or renamed). Calling ClearLanguageServiceRootCachesAndCollectAndFinalizeAllTransients will flush these, along with everything else.

dsyme commented 10 years ago

@vasily-kirichenko Could you report results for your repro above, when using 0.0.52?

vasily-kirichenko commented 10 years ago

Yes, I'm doing the test right now.

dsyme commented 10 years ago

Great

vasily-kirichenko commented 10 years ago
  1. 0.9 GB
  2. 1.2
  3. 1.3
  4. 1.47
  5. 1.68
  6. 1.78
  7. 2.0
  8. 2.3
  9. 2.4
  10. 2.6

So, no improvements.

Additionally, a strange effect added: after reloading the solution Highlight refs does not work at all or work partially like this:

6677

Then I execute Find all refs - nothing for about 20 seconds (0% CPU load), then some CPU load and refs appear. At the same moment Highlight refs started working. I'm not sure if this is FCS or VFPT bug. Need investigating.

dsyme commented 10 years ago

What actions does a "reload" perform w.r.t. FCS? Which of InvalidateConfiguration/InvalidateAll/ClearLanguageServiceRootCachesAndCollectAndFinalizeAllTransients does it call?

dsyme commented 10 years ago

@vasily-kirichenko the "strange behaviour" feels like it was because there was a longish-running operation completing in the FCS queue such as a parse-and-check of the solution. A "Reload" won't cancel such an operation in the FCS design, nor do the Async<_> operations returned by FCS pay attention to cancellation tokens.

vasily-kirichenko commented 10 years ago

On solution reload we call ClearLanguageServiceRootCachesAndCollectAndFinalizeAllTransients (I've checked it right now in the debugger).

It'd be great if FCS exposes its queue length for debug purposes.

dsyme commented 10 years ago

FWIW, For step 2 in the repo above I repeatedly switched between these two:

git reset --hard c40df4425c460635a39ac4b6829efe77bc7d06bf
git reset --hard ace90a0b4b710a97f571614836405d61470c47d3

and interleaved a Ctrl-, quick symbol find for a symbol in the VFPT source code e.g. 'HighlightUsageTagger'.

This induces a project reloiad. With the current gallery VSPT installed there was definitely a leak.

I'm 99% sure this is not the same as the "stale entries" problem fixed by @quasilord above. That wouldn't continue to leak when switching between two commits. It may not even be intrinsically an FCS problem - are there caches or other objects in VFPT which may be keeping handles to big objects?

dungpa commented 10 years ago

I took two VS snapshots while opening VFPT solution using PerfView.

The first snapshot: load the projects into VS and wait for colorization to finish. The second one: switch git branch, reload VS projects and wait for colorization to finish.

Here the diff between the snapshots. To my surprise, XamlTypeProvider appears in the list of total metric:

image

Here is a stack trace which has quite a lot of samples. It seems FCS stuck inside XamlTypeProvider and couldn't release memories:

image

I tried to remove XamlTypeProvider from VFPT solution, memory usage seems to stabilize after opening the solution.

dungpa commented 10 years ago

Here is the diff of exact scenario when XamlTypeProvider is removed.

image

As you can see, the amount of increased memory (Total Metric) and the number of created objects (Count) is much smaller.

@dsyme What are potential problems when FCS handles type providers?

@vasily-kirichenko If you include FSharpVSPowerTools.Logic (which has type providers in use) into the test console, it will exhibit the issue clearly.

dsyme commented 10 years ago

The XamlProvider is never disposing its file watchers (which are registered in order to trigger invalidation): https://github.com/fsprojects/FsXaml/blob/master/src/FsXaml.Wpf.TypeProvider/TypeProvider.Helper.fs#L28

These file watchers keep the type provider instances alive, which in turn keep FCS resources alive and cause the leak.

We should probably adjust FCS (and also the Visual F# Tools) to use a weak event handler when adding listeners to the "Invalidate" signals, which would make this kind of leak much, much less severe (leaking one FileWatcher per TP instance, instead of 200MB of data).

vasily-kirichenko commented 10 years ago

I've tested the console on FSharpVSPowerTools.Logic project (with ref to FSharpVSPowerTools.Core in ProjectOptions). After 5 iterations it occupy 500MB of memory. Wow. And yes, I added GC.Collect(); GC.Collect() after each iteration, see here https://github.com/vasily-kirichenko/FSharpVSPowerTools/commit/dab9a54eaf149a632f74a304a144a6ec6063f9f3

vasily-kirichenko commented 10 years ago

@dsyme That's very strange that Xaml TP does not implement IDisposable where it should dispose the watcher. I'll try to fix this.

vasily-kirichenko commented 10 years ago

https://github.com/fsprojects/FsXaml/pull/11

Results are not very clear though. I added debug output to FsXaml and for 3 iterations it write the following:

[FsXaml] Creating FileSystemWatcher.
[FsXaml] Disposing FileSystemWatcher.
[FsXaml] Creating FileSystemWatcher.
[FsXaml] Disposing FileSystemWatcher.
[FsXaml] Creating FileSystemWatcher.
[FsXaml] Creating FileSystemWatcher.
[FsXaml] Disposing FileSystemWatcher.
[FsXaml] Creating FileSystemWatcher.
[FsXaml] Disposing FileSystemWatcher.
[FsXaml] Creating FileSystemWatcher.
[FsXaml] Disposing FileSystemWatcher.
[FsXaml] Creating FileSystemWatcher.
[FsXaml] Disposing FileSystemWatcher.
[FsXaml] Creating FileSystemWatcher.
[FsXaml] Disposing FileSystemWatcher.
[FsXaml] Creating FileSystemWatcher.
[FsXaml] Disposing FileSystemWatcher.

The TP has been create 9 times (wow), but disposed only 8 times. So, FCS keep a reference to it? Why?

And yes, the test console consumes memory in the same way as before.

dsyme commented 10 years ago

Is that output when using your console app, and is that calling ClearLanguageServiceRootCachesAndCollectAndFinalizeAllTransients at the end to clear all caches (and hence all TP references)?

vasily-kirichenko commented 10 years ago

This is the console output. Press 'w':

Parsing: Trigger parse (fileName=l:\github\FSharpVSPowerTools\src\FSharpVSPower
Tools.Logic\Library.fs)
Worker: Parse and typecheck source...
[FsXaml] Creating FileSystemWatcher.
[FsXaml] Disposing FileSystemWatcher.
[FsXaml] Creating FileSystemWatcher.
[FsXaml] Disposing FileSystemWatcher.
[FsXaml] Creating FileSystemWatcher.
Worker: Parse completed
[LanguageService] Update typed info - HasFullTypeCheckInfo? true
Worker: Awaiting request
Worker: Starting background compilations
Got 1013 symbol uses in 00:00:13.4782304

Now press 'c':

[FsXaml] Disposing FileSystemWatcher.

So, yes, the last instance is disposed if we clear the caches.

vasily-kirichenko commented 10 years ago

After 10 iterations the console took 1GB RAM and it does not free any of it after

Checker.ClearLanguageServiceRootCachesAndCollectAndFinalizeAllTransients()
GC.Collect()
GC.Collect()
vasily-kirichenko commented 10 years ago

After one iteration followed by Checker.ClearLanguageServiceRootCachesAndCollectAndFinalizeAllTransients(); GC.Collect(); GC.Collect() half of the ram is allocated by a static var in Lexer:

image

(the other half is rooted to FsXaml).

dsyme commented 10 years ago

Is this with updated FCS (0.0.52) and XamlProvider (with your fix? is that published as a nuget package yet? See also this comment here: https://github.com/vasily-kirichenko/FsXaml/commit/cff5b83601f9e290104d75bd97e082045ecff769#commitcomment-6631529)

I noticed your console app still had a packages.config with referenced to FCS 0.0.50

I believe that Lexer data is just the lexer tables and is not significant.

vasily-kirichenko commented 10 years ago

Yes, this was with FCS 0.0.52 (I just pushed a commit) and with fixed FsXaml.

dungpa commented 10 years ago

Have you tried the combination of FCS 0.0.53 and patched FsXaml? Does it make any difference?

vasily-kirichenko commented 10 years ago

Tried 0.0.53 with patched FsXaml. Same behaviour. Now testing 0.0.54

vasily-kirichenko commented 10 years ago

One iteration + ClearLanguageServiceRootCachesAndCollectAndFinalizeAllTransients = 122MB Ten iterations = 284MB Ten iterations + ClearLanguageServiceRootCachesAndCollectAndFinalizeAllTransients = 162MB

So, it's a great improvement! Cool.

dungpa commented 10 years ago

Wow, great news :).

vasily-kirichenko commented 10 years ago

I'll update 0.0.52 PR to 0.0.54 and try to fix the failing tests.

vasily-kirichenko commented 10 years ago

Could anyone merge the FsXaml PR please?

fsgit commented 10 years ago

Resolved variously in FsXaml and FCS