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

Turn off closed files diagnostic analysis & fix creating IncrementalBuilder when a file opens #2395

Merged

Conversation

vasily-kirichenko
Copy link
Contributor

@vasily-kirichenko vasily-kirichenko commented Feb 9, 2017

This fixes #2389

In short, Roslyn by default performs "closed files diagnostic" analysis, which means it calls all DocumentDiagnosticsAnalyzer derivatives for all file in current project, opened and closed, even if a single file changed. This resulted with calling ParseAndCheckFileInProject for all files, which in turn made parseAndCheckFileInProjectCache useless.

  • use predefined BlockForCompletionItems option instead of hand made one. Now completion feels really non-blocking.

This also fixes #2324

  1. LanguageService.SetupProjectFile is called every time a file is opening https://github.com/Microsoft/visualfsharp/blob/master/vsintegration/src/FSharp.Editor/Common/LanguageService.fs#L298
  2. It unconditionally called UpdateProjectInfo https://github.com/Microsoft/visualfsharp/blob/master/vsintegration/src/FSharp.Editor/Common/LanguageService.fs#L307
  3. The latter, in turn, called Checker.InvalidateConfiguration https://github.com/Microsoft/visualfsharp/blob/master/vsintegration/src/FSharp.Editor/Common/LanguageService.fs#L117
  4. It recreated IncrementalBuilder https://github.com/Microsoft/visualfsharp/blob/master/src/fsharp/vs/service.fs#L2716 and replaced the old one.

I'm surprised anything worked at all :(

use predefined BlockForCompletionItems option instead of hand made one
@Pilchie
Copy link
Member

Pilchie commented Feb 9, 2017

LGTM, but also tagging @heejaechang to take a look.

@dsyme
Copy link
Contributor

dsyme commented Feb 9, 2017

@Pilchie @cartermp @vasily-kirichenko This feels like it might cause considerable pain on VS2017 RTM?

@dsyme
Copy link
Contributor

dsyme commented Feb 9, 2017

@vasily-kirichenko I think this also addresses #1856? What do you think? thanks

@vasily-kirichenko
Copy link
Contributor Author

Yes, it's a very annoying. Having your CPU busy all time is not what you like to have.

I'm not sure it turns off ProjectDiagnosticsAnalyzer (yes, it's commented out, but we will want to turn it on at some point).

@cartermp
Copy link
Contributor

cartermp commented Feb 9, 2017

@dsyme Do you mean this PR, or the symptom it's addressing? Closed File Diagnostics are off by default in C# as well, so I think this is a good change.

@dsyme
Copy link
Contributor

dsyme commented Feb 9, 2017

@cartermp #1856 is not very clear and I didn't record a repro, I will close it since I think you guys are much clearer on the exact work that needs to be done :)

@heejaechang
Copy link

👍 and yes, it turns off ProjectDiagnosticAnalyzer now, but I am planning to turn it on soon.

@vasily-kirichenko vasily-kirichenko changed the title Turn off closed files diagnostic analysis Turn off closed files diagnostic analysis & fix creating IncrementalBuilder when a file opens Feb 10, 2017
@vasily-kirichenko
Copy link
Contributor Author

:)

image

@dsyme
Copy link
Contributor

dsyme commented Feb 10, 2017

@vasily-kirichenko That's fantastic!

@vasily-kirichenko
Copy link
Contributor Author

@dsyme yeah, a couple of lines of code changes, but the effect is huge :)

@KevinRansom KevinRansom merged commit 293b115 into dotnet:master Feb 13, 2017
nosami pushed a commit to xamarin/visualfsharp that referenced this pull request Jan 26, 2022
…uilder when a file opens (dotnet#2395)

* turn off closed files diagnostic analysis

use predefined BlockForCompletionItems option instead of hand made one

* fixed: IncrementalBuilder is created every time a file in project opened / reopened
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.

Endless type checking FCS caches does not work when a file opens
7 participants