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

FCS uses too much memory #158

Closed
vasily-kirichenko opened this issue Jun 8, 2014 · 39 comments
Closed

FCS uses too much memory #158

vasily-kirichenko opened this issue Jun 8, 2014 · 39 comments

Comments

@vasily-kirichenko
Copy link
Contributor

See the VFPT issue fsprojects-archive/zzarchive-VisualFSharpPowerTools#474 (comment)

@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
Copy link
Contributor Author

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
Copy link
Contributor

dsyme commented Jun 10, 2014

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
Copy link
Contributor Author

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 vasily-kirichenko changed the title Serious memory leakage FCS uses too much memory Jun 10, 2014
@vasily-kirichenko
Copy link
Contributor Author

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
Copy link
Contributor

dsyme commented Jun 10, 2014

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

  • project sizes (amount of source code, size of referenced DLLs, number of transitive project references etc.)
  • the projectCacheSize parameter

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
Copy link
Contributor

dsyme commented Jun 10, 2014

@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

@7sharp9
Copy link
Member

7sharp9 commented Jun 10, 2014

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

On 10 Jun 2014, at 09:42, Don Syme [email protected] 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
Copy link
Contributor

dsyme commented Jun 10, 2014

@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
Copy link
Contributor

dsyme commented Jun 10, 2014

@vasily-kirichenko I just read this comment (#158 (comment)). 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
Copy link
Contributor

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

quasilord added a commit that referenced this issue Jun 10, 2014
@vasily-kirichenko
Copy link
Contributor Author

  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 Bump version number #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
Copy link
Contributor

Fixed in 0.0.52, published, see d6a0644.

@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
Copy link
Contributor

dsyme commented Jun 10, 2014

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

@vasily-kirichenko
Copy link
Contributor Author

Yes, I'm doing the test right now.

@dsyme
Copy link
Contributor

dsyme commented Jun 10, 2014

Great

@vasily-kirichenko
Copy link
Contributor Author

  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
Copy link
Contributor

dsyme commented Jun 10, 2014

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

@dsyme
Copy link
Contributor

dsyme commented Jun 10, 2014

@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
Copy link
Contributor Author

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
Copy link
Contributor

dsyme commented Jun 10, 2014

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
Copy link
Contributor

dungpa commented Jun 11, 2014

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
Copy link
Contributor

dungpa commented Jun 11, 2014

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
Copy link
Contributor

dsyme commented Jun 11, 2014

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
Copy link
Contributor Author

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 vasily-kirichenko/FSharpVSPowerTools@dab9a54

@vasily-kirichenko
Copy link
Contributor Author

@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
Copy link
Contributor Author

fsprojects/FsXaml#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
Copy link
Contributor

dsyme commented Jun 11, 2014

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor

dsyme commented Jun 11, 2014

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: vasily-kirichenko/FsXaml@cff5b83#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
Copy link
Contributor Author

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

@dungpa
Copy link
Contributor

dungpa commented Jun 11, 2014

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

@vasily-kirichenko
Copy link
Contributor Author

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

@vasily-kirichenko
Copy link
Contributor Author

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

So, it's a great improvement! Cool.

@dungpa
Copy link
Contributor

dungpa commented Jun 11, 2014

Wow, great news :).

@vasily-kirichenko
Copy link
Contributor Author

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

@vasily-kirichenko
Copy link
Contributor Author

Could anyone merge the FsXaml PR please?

@fsgit
Copy link
Contributor

fsgit commented Jun 24, 2014

Resolved variously in FsXaml and FCS

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

No branches or pull requests

6 participants