Skip to content

Conversation

@TIHan
Copy link
Contributor

@TIHan TIHan commented May 21, 2019

We had a bug where we didn't clear out FCS's caches when a solution was changed. This means, for example, switching configurations between Debug to Release, and vice versa, would cause a new solution to be created with completely different project ids; it would not clear the old projects from FCS out of its cache, effectively keeping them around.

This PR addresses the issue. It also adds a new API, InvalidateProject, to the compiler service that allows you to clear out a specific project from the incrementalbuilder cache. The API is used in FSharp.Editor when a solution has removed projects from itself.

This also does various cleanup; unified handling of single files with normal projects which means one cache in FSharpProjectOptionsManager.

Copy link
Contributor

@dsyme dsyme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one minor change requested

I'd be interested to know if this can be unit tested?

@TIHan
Copy link
Contributor Author

TIHan commented May 22, 2019

Good question on the testing part. I think it's possible if I only split out the functions. I'll see if it can be done.

Copy link
Contributor

@cartermp cartermp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor suggestions and a question

@TIHan TIHan changed the title Added cache invalidation policy when a solution changes [WIP] Added cache invalidation policy when a solution changes May 24, 2019
@TIHan
Copy link
Contributor Author

TIHan commented May 24, 2019

Marking this as WIP as we first need to fix the reference counting problem with IB (IncrementalBuilder) by pushing disposing of type providers and ilbinary readers down the TcImports' Finalizer.

@TIHan TIHan self-assigned this May 24, 2019
@cartermp cartermp mentioned this pull request May 26, 2019
10 tasks
@TIHan
Copy link
Contributor Author

TIHan commented May 29, 2019

I still would like to finish the solution cache invalidation strategy; but it's a little tricky. My fear is that it may make behavior worse. It seems everytime I want to remove something from the incrementalBuildersCache, it can make things worse because it causes cache thrashing; basically it can get into a state where it is constantly removing from the cache and rebuilding when it doesn't need to, multiple times. I think it has to do with IProjectReference and background builds; it's still a bit unclear to me what I can do without hurting existing behavior. I don't want to hurt current users of FCS.

@cartermp
Copy link
Contributor

@TIHan I'm going to close this since it appears #9124 accomplishes the same goal.

@cartermp cartermp closed this Jun 27, 2020
@cartermp cartermp mentioned this pull request Sep 6, 2020
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants