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

FCS caches does not work when a file opens #2324

Closed
vasily-kirichenko opened this issue Jan 26, 2017 · 6 comments · Fixed by #2395
Closed

FCS caches does not work when a file opens #2324

vasily-kirichenko opened this issue Jan 26, 2017 · 6 comments · Fixed by #2395
Labels
Area-LangService-API Bug Impact-Medium (Internal MS Team use only) Describes an issue with moderate impact on existing code.
Milestone

Comments

@vasily-kirichenko
Copy link
Contributor

vasily-kirichenko commented Jan 26, 2017

  1. Open FCS solution
  2. Open service.fs
  3. Wait until colorization appears
  4. Close the file
  5. Reopen it
  6. It takes equally long time for colorization and even structure to appear

The same behavior is observed when another file, even small one, is opened after the first one is fully checked. Looks like https://github.com/Microsoft/visualfsharp/blob/master/src/fsharp/vs/service.fs#L2413 checks all files laying prior to the current every time (however, I've not looked at it closely yet).

@dsyme
Copy link
Contributor

dsyme commented Jan 26, 2017

Looks like https://github.com/Microsoft/visualfsharp/blob/master/src/fsharp/vs/service.fs#L2413 checks all files laying prior to the current every time (however, I've not looked at it closely yet).

This fetches the results cached by the IncrementalBuilder (i.e. the background build process). I believe we have quite a lot of tests checking that re-checking doesn't happen (counting the number of checks).

That's not to say there isn't some kind of bug. For the repro steps above I would expect considerably less time on second opening, as long as the project itself has not been closed.

@dsyme dsyme added Area-LangService-API Bug Impact-Medium (Internal MS Team use only) Describes an issue with moderate impact on existing code. Tenet-Performance labels Feb 2, 2017
@cartermp cartermp added this to the vFuture milestone Feb 5, 2017
@vasily-kirichenko
Copy link
Contributor Author

I've investigated it. Create a hello world app, open a file, wait for coloring, close it, reopen.
It turns out, this https://github.com/Microsoft/visualfsharp/blob/master/src/fsharp/vs/service.fs#L2530 returns None, digging into it shows this path: https://github.com/Microsoft/visualfsharp/blob/master/src/fsharp/vs/IncrementalBuild.fs#L1704 => https://github.com/Microsoft/visualfsharp/blob/master/src/fsharp/vs/IncrementalBuild.fs#L840 => https://github.com/Microsoft/visualfsharp/blob/master/src/fsharp/vs/IncrementalBuild.fs#L413

I know nothing about this vectors stuff. @dsyme any ideas what's going wrong?

@vasily-kirichenko
Copy link
Contributor Author

It looks like GetCheckResultsBeforeFileInProject does not return cached background check result for any just opened file. It means that everything up to a opening file is rechecked (in one fat Reactor op, but it's a different issue).

I think it's a huge bug.

@vasily-kirichenko
Copy link
Contributor Author

Fixed.

  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 :(

@vasily-kirichenko vasily-kirichenko changed the title FCS caches does not work when a file opens FCS caches does not work when a file opens & fix recreating IncrementalBuilder when a file opens Feb 10, 2017
@vasily-kirichenko vasily-kirichenko changed the title FCS caches does not work when a file opens & fix recreating IncrementalBuilder when a file opens FCS caches does not work when a file opens Feb 10, 2017
@vasily-kirichenko
Copy link
Contributor Author

Fixed in #2395

@cartermp cartermp modified the milestones: VS 2017 Updates, vFuture Feb 10, 2017
@cartermp
Copy link
Contributor

Moving to VS Updates now 😄

@cartermp cartermp modified the milestones: VS 2017 Updates, 15.2 Jun 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-LangService-API Bug Impact-Medium (Internal MS Team use only) Describes an issue with moderate impact on existing code.
Projects
None yet
3 participants