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

Incremental builders cache thrashing and cross project references #10217

Closed
TIHan opened this issue Oct 3, 2020 · 0 comments
Closed

Incremental builders cache thrashing and cross project references #10217

TIHan opened this issue Oct 3, 2020 · 0 comments

Comments

@TIHan
Copy link
Member

TIHan commented Oct 3, 2020

Right now, the incremental builders cache in FCS is defined as follows:

let incrementalBuildersCache = 
        MruCache<CompilationThreadToken, FSharpProjectOptions, (IncrementalBuilder option * FSharpErrorInfo[])>
                (keepStrongly=projectCacheSize, keepMax=projectCacheSize, 
                 areSame =  FSharpProjectOptions.AreSameForChecking, 
                 areSimilar =  FSharpProjectOptions.UseSameProject)

The Mru in MruCache stands for 'most recently used'. It's supposed to be a cache with the idea that the longer an item lives, the more likely it is to be accessed. In this case, the item is a F# project. Roslyn does almost the same thing: http://sourceroslyn.io/#Microsoft.CodeAnalysis.Features/Workspace/ProjectCacheService.SimpleMRUCache.cs,63f2eb9a39dd5bff,references

At the moment, this cache does not work well when cross-project references is turned on and the project count is greater than the projectCacheSize. Cache thrashing can and most likely will occur in that case - which results in awful performance issues. So far, most of us have not experienced this because the default value for the projectCacheSize in VS is '200' - usually we work in solutions where the count is below that. But, if you set that value to say, '3', with a solution that has something like '50' projects, the IDE becomes unusable.

The reason why cache thrashing occurs is because if the current project you are checking, if its project dependencies keep getting evicted, it results in re-checking of those project dependencies - they are needed in order to check the files in your current project!

There are a few approaches to resolve this issue:

  1. IncrementalBuilder instances should have concrete references to other IncrementalBuilder instances they rely on. This should keep other instances alive in the cache as they would become weak references if they are marked for eviction. This could also be done using a ConditionalWeakTable; can be thought of like a secondary cache layer.

  2. Remove MruCache entirely, just use a ConcurrentDictionary and make everything strong. We've basically been doing this anyway with the project cache size set to '200' - most solutions don't have that many projects. In the worst case, when all VisualFSharp.sln projects are checked, memory was sitting around 1.6GB, (used to be well over 2GB before our memory fix with find-all refs).

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

3 participants