Skip to content

Conversation

@dsyme
Copy link
Contributor

@dsyme dsyme commented Mar 21, 2018

Addresses two memory problems in FSharp.Editor.dll:

  1. Too much data is saved in resident memory for tokenization, about 100bytes/token
  2. Several services save document-related data that can be recomputed if necessary in ConditionalWeakTable. This keeps it alive as long as the DocumentId is alive. This is too long - we don't need to strongly save all this all this information, we just need to save it for a sliding window of time using System.Runtime.Caching.MemoryCache

solutions:

  • Problem 1 is solved by reducing the saved data to 8 bytes/token. The implementation copes with single-line tokens up to 1MB long, which should be enough for anyone.
  • Problem 2 is solved by changing our uses of ConditionalWeakTable to System.Runtime.Caching.MemoryCache. This PR uses a sliding window of 10 seconds per document to reclaim the data. No per-document data is held after the sliding window has expired (from what I've seen it typically takes about 30 seconds for the reclaim to kick in in normal conditions, it will be faster when we are low on memory)

With one large file open this saves about 5% of the managed heap in active use, and about 7% of the managed heap after the sliding window has expired and all the data is reclaimed

With ~16 typical files opened this gave a reduction from 30MB to 6MB (active use) and 0MB (after sliding window has eexpired) of a total managed heap of around 120MB, i.e. about 25% of the managed heap.

Detail - single file

This is the usage now for this one-file scenario :

image

It replaces the sum of these two and a few more minor entries not at the top of the list

image

image

Detail - multiple files

For multiple-files, the new data is this:

image

Replacing approximately these four:

image
image
image
image

@dsyme dsyme changed the title Reduce long-saved memory used by FSharp.Editor tokenization Reduce resident memory used by FSharp.Editor tokenization Mar 21, 2018
@dsyme dsyme changed the title Reduce resident memory used by FSharp.Editor tokenization Reduce resident memory for VS tokenization and other caches Mar 21, 2018
@dsyme
Copy link
Contributor Author

dsyme commented Mar 21, 2018

For the record, here is a memory snapshot of using VisualFSharp.sln after some editing (using master + #4590 + #4586 ), with in-memory cross-project references disabled. This is the first time for a while I've been able to get an analyzed snapshot of this scenario. It still shows plenty of room for improvement.

image

I'm confident all of these can be eliminated:

Microsoft.FSharp.Compiler.AbstractIL.IL+ILAttributes	600,800	14,727,824	17,796,800
Microsoft.FSharp.Compiler.AbstractIL.ILBinaryReader+seekReadCustomAttrs@2515	572,532	11,450,640	8,447,272
Func<Microsoft.FSharp.Collections.FSharpList<Microsoft.FSharp.Compiler.AbstractIL.IL+ILPermission>>	251,455	8,046,560	3,178,432
Func<Microsoft.FSharp.Compiler.AbstractIL.IL+MethodBody>	202,734	6,487,488	2,242,440
Lazy<Microsoft.FSharp.Collections.FSharpList<Microsoft.FSharp.Compiler.AbstractIL.IL+ILPermission>>	251,455	5,625,008	4,767,600
Microsoft.FSharp.Compiler.AbstractIL.ILBinaryReader+seekReadSecurityDecls@2537	251,455	5,029,100	993,260
Lazy<Microsoft.FSharp.Compiler.AbstractIL.IL+MethodBody>	202,734	4,535,220	3,523,880

@dsyme
Copy link
Contributor Author

dsyme commented Mar 21, 2018

This is ready

@dsyme
Copy link
Contributor Author

dsyme commented Mar 22, 2018

@vasily-kirichenko If you feel like popping back for a visit I'd be happy for your review on this :)

@KevinRansom @cartermp If you get a chance to review that would be great

@dsyme dsyme merged commit b6dfbb6 into dotnet:master Mar 22, 2018
@auduchinok auduchinok mentioned this pull request Mar 28, 2018
10 tasks
nosami pushed a commit to xamarin/visualfsharp that referenced this pull request Jan 26, 2022
)

* improve tokenizer memory performance

* add comments

* use Memory Cache with sliding window for tokeniztion information

* use MemoryCache
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

Successfully merging this pull request may close these issues.

1 participant