High memory consumption

[vasily-kirichenko]

Open our solution, build it, switch to another branch, build again, switch to one other branch, build one more time. All this in 5 minutes interval. The result is as following:

This is all without executing Find all references / Rename refactoring at all. As a result, everything is greatly slowed down, VS periodically freezes for some seconds, then redraw itself and so on.

When I profiled it with PerfView I didn't see any leakage in our code, all these hundreds of megabytes were occupied by byte[] arrays related to "IL cache" inside FCS (don't know what it is though). So, I suspect FCS caching does not behave nicely for our scenarios.

As our solution is small, I think we have a big problem here.

[vasily-kirichenko]

I'll try to revert ClearLanguageServiceRootCachesAndCollectAndFinalizeAllTransients call and recheck.

[dungpa]

We recently cache many things aggressively. It means trading memory for performance. These caches may cause memory leaks as well.

[vasily-kirichenko]

Calling ClearLanguageServiceRootCachesAndCollectAndFinalizeAllTransients on solution opened event has not helped at all :(

Trying to get VS snapshot with PerfView.

[dungpa]

We don't unsubscribe this event https://github.com/fsprojects/VisualFSharpPowerTools/blob/master/src/FSharpVSPowerTools.Logic/ProjectProvider.fs#L268 so projectFactory always holds references of ProjectProvider instances. I guess this prevents garbage collector to collect those project providers when they are disposed.

[vasily-kirichenko]

As far as I know we don't have to unsubscribe from events if we subscribed with IEvent.Add method. See for example http://codebetter.com/matthewpodwysocki/2009/08/21/f-first-class-events-creating-and-disposing-handlers/

[rojepp]

@vasily-kirichenko Yes, you do need to unsubscribe. In the blog there was a nice extension method Subscribe that returns an IDisposable, so that you can unsubscribe by disposing. This is pretty neat, in that it allows you to unsubscribe even when using a lambda. This is not possible when passing a lambda directly to Add.

[vasily-kirichenko]

@rojepp Thanks! Will fix it just now.

[vasily-kirichenko]

As memory profiling entire VS is practically impossible, I created a simple console in this branch https://github.com/vasily-kirichenko/FSharpVSPowerTools/tree/FCS-memory-profiling and profile it with dotMemory.

Scenario:

• Run the console (nothing happened yet) -> Snapshot #1
• Call GetAllUsesOfAllSymbolsInFile (press 'w') -> Snapshot #2
• Call GetAllUsesOfAllSymbolsInFile -> Snapshot #3
• Call GetAllUsesOfAllSymbolsInFile -> Snapshot #4
• Call GetAllUsesOfAllSymbolsInFile -> Snapshot #5
• Call ClearLanguageServiceRootCachesAndCollectAndFinalizeAllTransients (press 'c') -> Snapshot #6

Before taking each snapshot "Force GC" command was executed several times (in dotMemory).

Results:

• ProjectOptions.LoadTime is different in each case (DateTime.Now) so it forces FCS to recheck the project
• It seems that FCS cache same project several times for each LoadTime (!)
• FCS keeps ~42MB of RAM after ClearLanguageServiceRootCachesAndCollectAndFinalizeAllTransients is called.

@dsyme can you comment on this, please?

[dsyme]

That definitely looks suspicious. Add an FCS bug on it?

[dsyme]

Yes, it looks like this cache here:

https://github.com/fsharp/FSharp.Compiler.Service/blob/master/src/fsharp/vs/service.fs#L2235

It may need to be redesigned or tweaked to discard subsumed entries.

[ Correction, the problem may be here - https://github.com/fsharp/FSharp.Compiler.Service/blob/master/src/fsharp/vs/service.fs#L1922 - however that cache appears to discard subsumed entries ]

BTW I noticed the other dat that renaming a file in a very large project seemed to grind VFPT into the ground - this would also explain that.

[vasily-kirichenko]

About https://github.com/fsharp/FSharp.Compiler.Service/blob/master/src/fsharp/vs/service.fs#L1922 - I added debug output into it yesterday and yes, it works correctly - the cache size stayed equals to 1 during my test, so the problem isn't there.

[vasily-kirichenko]

OK, now when the leak in FCS seems to be fixed I've tested VFPT in the old scenario - reloading VFPT solution several times. After 15 reloads VS took 2.3GB ram. After "force GC" - 1.8GB. So, it looks like a leak in VFPT itself.

[dungpa]

It seems Visual F# Tools leaks quite a large amount of memory when type providers are enabled.

Consider the following scenario:

• Disable VFPT
• Open VFPT solution
• Repeat these steps 15 times: (1) Switch git branch (2) Reload the solution (3) Rebuild the solution

Visual Studio occupies 1.7 GB RAM. I also tried to profile VFPT using PerfView and there are dominated entries from Visual F# Tools which make it much harder to analyze.

@dsyme Are Visual F# Team aware of this issue? Shall we open an issue on Codeplex?

[dsyme]

Is this with the leaky XamlTypeProvider 0.9.3? If so then yes, that bug would cause the Visual F# Tools to leak. Even with the "FileWatcher" bug fixed it's possible that the XamlTypeProvider leaks in other way too.

[dungpa]

No, it was with Xaml type provider 0.9.4.

Here is a diff after two reloads and two rebuilds of the solution (VFPT is disabled):

There is a trace to XAML type provider as well:

[dungpa]

I couldn't see any obvious leaks from VFPT. Opened an issue at https://visualfsharp.codeplex.com/workitem/84.

[dsyme]

Am still wondering if XamlProvider is the cause of the leak (though a VFT fix could make it less severe). Particularly because its a generative provider. Will take a look on Monday.

[dungpa]

Hi Don, did you have further investigation on this?

[vasily-kirichenko]

With VFPT 1.2.0 installed VS still consumes 2.5GB ram after a couple of dozens solution builds, even without solution reloading.

[dsyme]

Looking at PerfView, it seems the XamlTypeProvider is still leaky,

In particular, the registration of the event handler here https://github.com/fsprojects/FsXaml/blob/master/src/FsXaml.Wpf.TypeProvider/ProvidedTypes-head.fs#L2095 is "disposed" in the implementation of IDisposable provided here: https://github.com/fsprojects/FsXaml/blob/master/src/FsXaml.Wpf.TypeProvider/ProvidedTypes-head.fs#L2126. However this disposal implementation is overriden by the XamlTypeProvider implementation here: https://github.com/fsprojects/FsXaml/blob/master/src/FsXaml.Wpf.TypeProvider/XamlTypeProvider.fs#L300

Both the ProvidedTypes implementation and the XamlTypeProvider implementation should be rewritten to use the "Dispose" pattern (where subclasses call the Dispose logic in the base class) or some similar technique to make sure the inherited disposal logic is called. Fun with OO....

[dungpa]

@ReedCopsey Could you help with this problem?

[ReedCopsey]

I'll look into it ASAP

Reed

On Jun 19, 2014, at 6:34 AM, Anh-Dung Phan notifications@github.com wrote:

@ReedCopsey Could you help with this problem?

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

[ReedCopsey]

@dsyme I'll fix in FsXaml today. Where is the "main" repo/source for the ProvidedTypes API living now? I'd like to do a PR to address this in future TPs as well, since it's a potential issue for any type provider using that which needs its own disposal mechanism.

[dsyme]

AFAIK the F# TypeProvider StarterPack https://github.com/fsprojects/FSharp.TypeProviders.StarterPack/ is now the canonical home of the provided types API implementation.

There may be numerous fixes that need to be merged into that, especially the high-value multi-targeting code in FSharp.Data

[ReedCopsey]

Just published a new FsXaml that corrects this. In addition to the leak @dsyme found, there was an additional leak I tracked down in there.

PR #511 has been updated to include this (as its also required for my update there), which hopefully will address all of the issues due to the the XAML Type Provider.

[dungpa]

After #511, memory leak is less severe now. I think Visual F# Tools is still leaking memory; particularly it doesn't invalidate caches soon enough after reloading solutions.

[vasily-kirichenko]

On my solution at work

• Build it
• Switch configuration from Debug to Release
• Build
• devenv.exe allocates 2.6GB of RAM

It seems it's not solvable in any near future, so I close it.

[vasily-kirichenko]

I finally managed to profile my 2.5GB+ VS setup this night. It seems that the problem in FSharp.Configuration provider (it's used in a project in my solution, which in turn is used in almost all the other projects). This is the dotMemory screenshot:

Another view of the same snapshot:

I'm struggling to understand how to fix the provider (it's actually mine YamlConfig).

@dsyme Could you give a tip what's wrong with it?

[dsyme]

We've seen variations on this bug before. It seems that your copy of ProvidedTypes.fs has been adjusted to give TypeProvidersForNamespace has an implementation of IDisposable. I don't exactly know who or where added this - it's in the Type Provider Starter Pack, which also adds a "Disposing" event where you can listen in to hook more work into - but I don't know its progeny before that - perhaps it was in FSharpx.

In the Config type providers, this implementation is then overriden (replaced) by your implementation of IDisposable which only disposes "config" but not the resources - i.e. the globally registered event handler - of the TypeProvidersForNamespace object. This single event handler registration is enough to keep a massive amount of stuff alive. (The bug would be much less memory-devestating if a weak registration was used for this event handler, as only a few objects would leak)

So it looks like the solution is to update your TypeProviders.fs to the latest, and hook into the "Disposing" event rather than replacing IDisposable yourself.

This design pattern used by TypeProvidersForNamespace is not great, because it's so easy to make this mistake. I'm not even sure what the assembly resolver is doing and I don't really like that it's there at ll. If TypeProvidersForNamespace is going to have a Disposing event, then really no subclass of TypeProvidersForNamespace should implement IDisposable and that should somehow be made more clear (it would be great if it could prevented statically. If TypeProvidersForNamespace doesn't have a Disposing event, then it should at least have followed the .NET standard pattern of having a Dispose on the base class that the subclass can call.

It would be great if someone could review all type provider implementations under "fsprojects" and "FSharp.Data" to see if they contain manifestations of this bug. The tell-tale signs are type providers that implement IDisposable themselves, where they are using a ProvidedTypes.fs that also implements IDisposable.

Cheers Don

[vasily-kirichenko]

Thanks a lot, Don!

I made the changes here https://github.com/fsprojects/FSharp.Configuration/pull/48 Will test it then publish the package.

[dungpa]

@dsyme It would be great if there is a written blog post about Disposing patterns for type providers. There are still some type providers out there suffering from this exact issue.

[dsyme]

The rule is very simple: don't implement IDisposable on your type provider, if you inherit from TypeProvidersForNamespace. Instead hook into the Disposing event on TypeProvidersForNamespace. If that event isn't available then update to the latest ProvidedTypes.fs in the Type Provider Starter Pack.

The problem is with getting this information to the people who need it. I think a full review of existing type providers is the best way forward, entering github issues and/or fixes where appropriate. That will disseminate the issue to the right people. Then the Type Provider Starter Pack docs should be fixed too.

It would be immensely helpful if someone at least changed the Type Provider Starter Pack implementation to use a weak registration of the event handler registered here. That would make the issue much less severe.

[vasily-kirichenko]

With the updated FSharp.Configuration the issue has finally gone! :)

[dsyme]

Fabulous!