Possible fix for script processing

[dsyme]

For @cartermp to try, #6250

[dsyme]

Basically the call to InvalidateConfiguration looks wrong.

We do recompute the project options for the script on each file change - this is ok - it's not optimal - but exactly what we did before.

But it's not OK to blow away the incremental builder that does the type checking, which is what InvalidateCOnfiguration does. AFAIK there was no InvalidateConfiguration on this path before.

Separately I move three lines under a cache lookup, which I think is correct, it will reduce the amount of recomputation on each change

Haven't compiled this (don't have dev16.0 installed) but I think it's right

[TIHan]

This will probably fix the regression, but we should also test that adding additional load script directives will give us completions based on what is inside the loaded script files.

[dsyme]

@TIHan Hey you're meant to be on holidays :)

This will probably fix the regression, but we should also test that adding additional load script directives will give us completions based on what is inside the loaded script files.

Yes you're right.

There are other incremental scenarios to test, e.g. that editing a #load file in another editor and injecting an error into it results in a red-squiggly in the file that contains the #load directive.

These should all work, at least the did in VS2015 and I believe they do in VS2017. If they don't, then we have to dig into why - the use of InvalidateConfiguration isn't the right way to implement this, rather we should be getting a BeforeBackgroundFileCheck event to indicate it's time to refresh the errors for a file. That code appears unchanged in dev16.0

A possibility is that the InvalidateConfiguration(startBackgroundCompileIfAlreadySeen=true) call should be replaced by a call to CheckProjectInBackground which is similar but doesn't invalidate. It says to FCS "hey, background checking should be processing this particular script and its #load". If there are problems with refresh or intellisense then try that.

[KevinRansom]

@dsyme, this says possible, let me know when it is done. Thx

[smoothdeveloper]

Hi Guys, a bit of overall feedback, I've updated to VS 2019 preview 3 to 4 and it seems to be worse, I was happy so far with the performance in script editing versus VS 2017 / 2015 but now I can't use it, hope a fix makes it soon.

In context of editing a script, is it worth to have the error reporting on first issue on #load or #r just break the full type check until those scripting directives are properly checked?

I edit scripts which may have large set of dependency (assemblies and other scripts) and keeping the editor snappy at all times even if I lose aspects of the tooling for a bit is better than freezing like VS tend to do.

I've noticed when it gets really slow, even manipulating the selection span (without dragging the text) was causing delays, so some of the issues may stem further down in the VS editor.

What is also missing in script editing, compared to what VS2015 + VFPT provides is a better navigation to symbols defined in loaded scripts and assemblies, without the need to have a project loaded or the even code, VS can display the assembly in object browser as a fallback.

I'm very positive about VS 2019 + F# release, the issues seems to get ironed out 👍

[cartermp]

As of latest commit the perf with #load is now performing as well as vs2017 in the (huge) perf testing scripts.

[dsyme]

@cartermp Did you do some more testing? From our session on Tuesday my impression is we made progress, but we may still be significantly slower?

[dsyme]

(In the perf traces I certainly noticed that we are burning a lot of UI thread time recomputing project options for scripts. We can make that much cheaper with an FCS cache on parsing #load script files I think)

Also the synchronous completion and editor dialog on Ctrl-space is a particular worry - in vs2017 this completion is never UI blocking and the difference is noticeable - perhaps this bug is being fixed)

[cartermp]

The user-visible perf on autocompletion + dot completion seemed similar. The forced completion blocking isn't us, so it shouldn't block merging this IMO. Though a cache in FCS isn't a bad idea.

[dsyme]

The user-visible perf on autocompletion + dot completion seemed similar. The forced completion blocking isn't us, so it shouldn't block merging this IMO. Though a cache in FCS isn't a bad idea.

Okey dokey. Definitely this is a considerable improvement. I'll see if I can add the cache.