Transparent compiler by TheAngryByrd · Pull Request #1240 · ionide/FsAutoComplete

TheAngryByrd · 2024-03-01T15:28:37Z

Adds support for Transparent Compiler in FSAC.

src/FsAutoComplete.Core/CompilerServiceInterface.fsi

src/FsAutoComplete.Core/CompilerServiceInterface.fs

src/FsAutoComplete.Core/SymbolLocation.fs

src/FsAutoComplete/LspServers/AdaptiveServerState.fs

src/FsAutoComplete.Core/AdaptiveExtensions.fs

  let bind (mapping: 'a -> CancellationToken -> asyncaval<'b>) (value: asyncaval<'a>) =
    let mutable cache: option<_> = None
    let mutable innerCache: option<_> = None
+    let mutable outerDataCache: option<_> = None


src/FsAutoComplete.Core/AdaptiveExtensions.fs

src/FsAutoComplete.Core/FCSPatches.fsi

  val defaultLanguageVersion: Lazy<LanguageVersionShim>

+
+  val fromOtherOptions: options: seq<string> -> LanguageVersionShim


src/FsAutoComplete.Core/CompilerServiceInterface.fsi


  member RemoveFileFromCache: file: string<LocalPath> -> unit

+  member ClearCache: snap: seq<FSharpProjectSnapshot> -> unit


src/FsAutoComplete.Core/CompilerServiceInterface.fsi

src/FsAutoComplete.Core/CompilerServiceInterface.fs

src/FsAutoComplete.Core/CompilerServiceInterface.fsi

.github/workflows/build.yml

TheAngryByrd · 2024-04-17T14:13:36Z

src/FsAutoComplete.Core/AdaptiveExtensions.fs

+      real.ContinueWith(fun (task: Task<_>) ->
+        match task.Status with
+        | TaskStatus.RanToCompletion -> tcs.TrySetResult task.Result |> ignore<bool>
+        | TaskStatus.Canceled ->
+          tcs.TrySetCanceled(TaskCanceledException(task).CancellationToken)
+          |> ignore<bool>
+        | TaskStatus.Faulted -> tcs.TrySetException(task.Exception.InnerExceptions) |> ignore<bool>
+
+        | _ -> ())
+      |> ignore<Task>


Changed back to ContinueWith style as getting stack traces weren't working properly. Wish I had a good idea how to make a test for that.

src/FsAutoComplete.Core/AdaptiveExtensions.fs

src/FsAutoComplete/LspServers/ProjectWorkspace.fs

TheAngryByrd · 2024-04-17T14:43:08Z

src/FsAutoComplete/LspServers/ProjectWorkspace.fs

+            match! inMemorySourceFiles |> AMap.tryFind sourcePath with
+            | Some volatileFile -> return! volatileFile |> AVal.map createFSharpFileSnapshotInMemory
+            | None -> return! createFSharpFileSnapshotOnDisk sourceTextFactory sourcePath


Changes to source files, either in memory or on disk will cause snapshots to be recreated.

TheAngryByrd · 2024-04-17T14:43:43Z

src/FsAutoComplete/LspServers/ProjectWorkspace.fs

+        >> Log.addContextDestructured "projectFileName" p.ProjectFileName
+      )
+
+      snapshot


Finding an already created snapshot keeps memory use down.

TheAngryByrd · 2024-04-17T14:44:37Z

src/FsAutoComplete/LspServers/ProjectWorkspace.fs

+      let! projectFileName = projectFileName
+      and! projectId = projectId
+      and! sourceFiles = sourceFiles
+      and! referencePaths = referencePaths
+      and! otherOptions = otherOptions
+      and! referencedProjects = referencedProjects
+      and! isIncompleteTypeCheckEnvironment = isIncompleteTypeCheckEnvironment
+      and! useScriptResolutionRules = useScriptResolutionRules
+      and! loadTime = loadTime
+      and! unresolvedReferences = unresolvedReferences
+      and! originalLoadReferences = originalLoadReferences


Note, any changes to any of these could recreate a snapshot. In practice only a few of these change

TheAngryByrd · 2024-04-17T14:45:57Z

src/FsAutoComplete/LspServers/ProjectWorkspace.fs

+          referencePath.Substring(3) // remove "-r:"
+          |> createReferenceOnDisk)
+
+      let referencedProjects = mapReferences p


This is where it creates/finds referenced snapshots, and when those change it will recreate the current snapshot.

src/FsAutoComplete.Core/CompilerServiceInterface.fsi

    file: string<LocalPath> ->
-    options: seq<string * FSharpProjectOptions> ->
-      (FSharpProjectOptions * FSharpProjectOptions list) option
+    snapshots: seq<string * CompilerProjectOption> ->


src/FsAutoComplete.Core/CompilerServiceInterface.fsi

-    options: seq<string * FSharpProjectOptions> ->
-      (FSharpProjectOptions * FSharpProjectOptions list) option
+    snapshots: seq<string * CompilerProjectOption> ->
+      option<CompilerProjectOption * list<CompilerProjectOption>>


src/FsAutoComplete.Core/CompilerServiceInterface.fsi

-    options: seq<string * FSharpProjectOptions> ->
-      (FSharpProjectOptions * FSharpProjectOptions list) option
+    snapshots: seq<string * CompilerProjectOption> ->
+      option<CompilerProjectOption * list<CompilerProjectOption>>


src/FsAutoComplete.Core/CompilerServiceInterface.fsi

  member GetProjectOptionsFromScript:
    file: string<LocalPath> * source: ISourceText * tfm: FSIRefs.TFM ->
-      Async<FSharpProjectOptions * FSharpDiagnostic list>
+      Async<FSharpProjectOptions * list<FSharp.Compiler.Diagnostics.FSharpDiagnostic>>


src/FsAutoComplete.Core/CompilerServiceInterface.fsi

+    file: string<LocalPath> * snapshot: FSharpProjectSnapshot -> ParseAndCheckResults option
+
+  member TryGetRecentCheckResultsForFile:
+    file: string<LocalPath> * options: FSharpProjectOptions * source: ISourceText -> option<ParseAndCheckResults>


src/FsAutoComplete.Core/CompilerServiceInterface.fsi


  member FindReferencesForSymbolInFile:
-    file: string * project: FSharpProjectOptions * symbol: FSharpSymbol -> Async<seq<range>>
+    file: string<LocalPath> * project: FSharpProjectSnapshot * symbol: FSharpSymbol -> Async<seq<range>>


src/FsAutoComplete.Core/CompilerServiceInterface.fsi

+    file: string<LocalPath> * project: FSharpProjectSnapshot * symbol: FSharpSymbol -> Async<seq<range>>
+
+  member FindReferencesForSymbolInFile:
+    file: string<LocalPath> * project: FSharpProjectOptions * symbol: FSharpSymbol -> Async<seq<range>>


src/FsAutoComplete.Core/CompilerServiceInterface.fs

    and set (value) = disableInMemoryProjectReferences <- value

-  static member GetDependingProjects (file: string<LocalPath>) (options: seq<string * FSharpProjectOptions>) =
+  static member GetDependingProjects (file: string<LocalPath>) (snapshots: seq<string * CompilerProjectOption>) =


src/FsAutoComplete.Core/SymbolLocation.fs

 type SymbolDeclarationLocation =
  | CurrentDocument
-  | Projects of FSharpProjectOptions list * isLocalForProject: bool
+  | Projects of CompilerProjectOption list * isLocalForProject: bool


TheAngryByrd · 2024-04-22T03:08:43Z

Ok I think this is finally ready. This can now be turned on/off so we can merge safely without fear of being able to use the old stuff since transparent compiler may still have issues that we have no control of directly.

nojaf · 2024-04-22T06:29:13Z

How do we play with this? I checked out your branch, built and tried the following:

Or should --use-fcs-transparent-compiler be added somewhere else?

nojaf · 2024-04-22T07:08:34Z

Ok, $env:USE_TRANSPARENT_COMPILER = 'TransparentCompiler' did the trick.
I tried this on three projects, it looks very promising!
You can tell by how fast semantic highlighting is kicking in.

I'd be a fan of merging this into the nightly branch and playing with it for a while.

TheAngryByrd · 2024-04-22T12:55:38Z

How do we play with this? I checked out your branch, built and tried the following:

Or should --use-fcs-transparent-compiler be added somewhere else?

Unfortunately there isn't a way to add generic parameters via Ionide. (means that should get added at some point). I do have a PR available in Ionide to add it but you'll have to build a run a custom version of Ionide to access the setting.

Ok, $env:USE_TRANSPARENT_COMPILER = 'TransparentCompiler' did the trick. I tried this on three projects, it looks very promising! You can tell by how fast semantic highlighting is kicking in.

Are you sure that actually works? I'm only using USE_TRANSPARENT_COMPILER it in the tests to divide up test runs.

vzarytovskii · 2024-04-22T12:59:48Z

USE_TRANSPARENT_COMPILER has no effect on anything in the compiler/service itself, we for sure don't use this particular variable anywhere. I think this PR uses it in tests only.

src/FsAutoComplete/FsAutoComplete.fsproj

baronfel

Left a few notes and a couple questions as I was reading through this.

I'm surprised by how understandable this was, despite the large amount of changes. I think this says a lot about the comments you left and the places you unified previous logic.

Directory.Build.props

src/FsAutoComplete.Core/AdaptiveExtensions.fs

baronfel · 2024-04-22T15:10:44Z

src/FsAutoComplete.Core/AdaptiveExtensions.fs

-                    let! ct = CancellableTask.getCancellationToken ()
-                    let inner = mapping i ct
-                    return inner
+                RefCountingTaskCreator(fun ct ->


is this the same throughout this file? moving from cancellableTask to task for better stack traces?

Yeah I need to rundown why I was still getting weird stacktraces but I'll do that later.

src/FsAutoComplete.Core/Commands.fs

src/FsAutoComplete.Core/FileSystem.fs

baronfel · 2024-04-22T15:29:32Z

src/FsAutoComplete.Core/SemaphoreSlimLocks.fs

+/// <summary>
+/// An awaitable wrapper around a task whose result is disposable. The wrapper is not disposable, so this prevents usage errors like "use _lock = myAsync()" when the appropriate usage should be "use! _lock = myAsync())".
+/// </summary>
+[<Struct>]


beautiful type - thanks for this

src/FsAutoComplete.Core/SymbolLocation.fs

src/FsAutoComplete/CodeFixes.fsi

src/FsAutoComplete/LspServers/AdaptiveServerState.fs

baronfel · 2024-04-22T15:33:08Z

src/FsAutoComplete/LspServers/AdaptiveServerState.fs


  let runAnalyzers (config: FSharpConfig) (parseAndCheck: ParseAndCheckResults) (volatileFile: VolatileFile) =
-    async {
+    asyncEx {


should the direct use of async be a code smell for us now?

Probably, though we can make additional PRs to hit all those places.

src/FsAutoComplete.Core/SemaphoreSlimLocks.fs

nojaf · 2024-04-22T16:16:18Z

test/FsAutoComplete.Tests.Lsp/Program.fs

+
+
+let compilers =
+  match getEnvVarAsStr "USE_TRANSPARENT_COMPILER" with


@TheAngryByrd, hmm, ok now I'm doubting whether that did work or not.
It might not have and I was seeing things

So, I hardcoded the useTransparentCompiler to true locally and I remain with my past statements: all goes well for me, ship it.

Co-authored-by: Chet Husk <baronfel@users.noreply.github.com>

github-advanced-security bot found potential problems Mar 1, 2024

View reviewed changes

TheAngryByrd force-pushed the transparent-compiler branch from 4f6ccba to 0242c14 Compare March 15, 2024 19:17

github-advanced-security bot found potential problems Mar 15, 2024

View reviewed changes

TheAngryByrd force-pushed the transparent-compiler branch 2 times, most recently from 42c2357 to bcaabe9 Compare March 15, 2024 22:01

TheAngryByrd force-pushed the transparent-compiler branch from e0dbfbf to 8226064 Compare March 26, 2024 03:05

github-advanced-security bot found potential problems Mar 26, 2024

View reviewed changes

TheAngryByrd mentioned this pull request Apr 2, 2024

Transparent Compiler: High memory usage in FSAC dotnet/fsharp#16979

Closed

TheAngryByrd force-pushed the transparent-compiler branch 2 times, most recently from 3a5d660 to b0c079d Compare April 15, 2024 14:10

github-advanced-security bot found potential problems Apr 15, 2024

View reviewed changes

src/FsAutoComplete.Core/CompilerServiceInterface.fsi Fixed Show fixed Hide fixed

TheAngryByrd force-pushed the transparent-compiler branch 4 times, most recently from 381198b to acbfe87 Compare April 17, 2024 03:20

TheAngryByrd commented Apr 17, 2024

View reviewed changes

TheAngryByrd added 14 commits April 21, 2024 15:38

Ignore warn on debug

0f513d8

global.json nonsense

1e365e9

Transparent compiler changes

973d47a

formatting

fe41e48

Fixing snapshots/caching

3732365

prototype adaptive snapshotting

058f2cf

Cleanup adaptive snapshots

a237930

Much refactoring snapshot tests

b946e2c

Much refactoring snapshot tests

6e6ef62

Add adaptive snapshots to lsp server

fa38c5f

Make source to snapshot lookup adaptive

316d371

workspace cleanups

e5c28ef

Fixup much stuff

5c5f108

More cleanups

eee5675

github-advanced-security bot found potential problems Apr 21, 2024

View reviewed changes

Make background/transparent compiler switchable

ae52948

TheAngryByrd force-pushed the transparent-compiler branch from 43076cc to ae52948 Compare April 21, 2024 22:16

Up timeout again

aa4f46e

TheAngryByrd mentioned this pull request Apr 21, 2024

Add flag for enabling transparent compiler support ionide/ionide-vscode-fsharp#2003

Merged

TheAngryByrd marked this pull request as ready for review April 22, 2024 01:14

TheAngryByrd added 5 commits April 21, 2024 22:02

Split up tests

096cb02

Retry workspace loader tests

88e430f

skip flakey test

cea27d2

disable projectgraph tests for perf reasons

53baffe

Fix description of transparent compiler flag

c9e2ae0

TheAngryByrd added 2 commits April 21, 2024 23:11

cleanup dead code

560a44b

Comment project workspace more

8696c11

TheAngryByrd force-pushed the transparent-compiler branch from 8e6b98e to 8696c11 Compare April 22, 2024 03:26

TheAngryByrd commented Apr 22, 2024

View reviewed changes

src/FsAutoComplete/FsAutoComplete.fsproj Outdated Show resolved Hide resolved

revert fsproj change

bba20b6

baronfel approved these changes Apr 22, 2024

View reviewed changes

vzarytovskii reviewed Apr 22, 2024

View reviewed changes

src/FsAutoComplete.Core/SemaphoreSlimLocks.fs Show resolved Hide resolved

nojaf reviewed Apr 22, 2024

View reviewed changes

TheAngryByrd and others added 5 commits April 22, 2024 13:43

Update src/FsAutoComplete.Core/AdaptiveExtensions.fs

47bc364

Co-authored-by: Chet Husk <baronfel@users.noreply.github.com>

Update Directory.Build.props

00b911e

Co-authored-by: Chet Husk <baronfel@users.noreply.github.com>

Remove nowarn since it's in pojectfile now

48ad332

Remove redundant normalize call

a6692eb

Add noequality no comparison

5223f80

TheAngryByrd merged commit c5a5812 into ionide:nightly Apr 23, 2024

		val defaultLanguageVersion: Lazy<LanguageVersionShim>


		val fromOtherOptions: options: seq<string> -> LanguageVersionShim


		member RemoveFileFromCache: file: string<LocalPath> -> unit

		member ClearCache: snap: seq<FSharpProjectSnapshot> -> unit



		let compilers =
		match getEnvVarAsStr "USE_TRANSPARENT_COMPILER" with

Conversation

TheAngryByrd commented Mar 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Check notice

Uh oh!

Check notice

Uh oh!

Uh oh!

Uh oh!

Check notice

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Check notice

Check notice

Check notice

Check notice

Check notice

Check notice

Check notice

Check notice

Check notice

TheAngryByrd commented Apr 22, 2024

Uh oh!

nojaf commented Apr 22, 2024

Uh oh!

nojaf commented Apr 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TheAngryByrd commented Apr 22, 2024

Uh oh!

vzarytovskii commented Apr 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

baronfel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

TheAngryByrd commented Mar 1, 2024 •

edited

Loading

nojaf commented Apr 22, 2024 •

edited

Loading

vzarytovskii commented Apr 22, 2024 •

edited

Loading