AsyncLocal diagnostics, clean slate (#16779)

* fix some inconsistencies

* we don't intend to dispose this ever

* removed

* multiple loggers

* fix RegisterAndImportReferencedAssemblies

* fix deadlock in fsi

* fix some BuildGraphTests

* restore asyncmemoize

* fix missing logger in test

* format

* fix transparent compiler nre

* cleanup

* default value

* add some comments

* foramt and notes to make it green if it's green

* wrap any parallel computations that potentially push diagnostics

* try to fix buildgraphtests

* try to eradicate deadlocks in tests

* fix buildgraph test

* disable for a moment

* flatten exceptions

* reshuffle

* prevent graphnode deadlock

* yield

* diff

* add some comments

* add AsyncLocal test

* more testing

* not needed after all?

* test

* format

* revert

* try RunContinuationsAsynchronously

* try to deal with deadlock another way

* nope

* revert

* format

* restore release notes

* add test for ListParallel

* speed up test

* improve test

* simpler MultipleDiagnosticsLoggers

* rename and speedup test

* parallel logging perf

* remove SwitchToThreadPool

* sequential

* Revert "sequential"

This reverts commit 8c77c0c.

* revert spurious change

* fix comments

* test MultipleDiagnosticsLoggers

* add comment and clean up

---------

Co-authored-by: Petr <[email protected]>

Author: majocha

docs/release-notes/.FSharp.Compiler.Service/8.0.400.md

@@ -22,4 +22,5 @@
 
 * Minor compiler perf improvements. ([PR #17130](https://github.com/dotnet/fsharp/pull/17130))
 * Improve error of Active Pattern case Argument Count Not Match ([PR #16846](https://github.com/dotnet/fsharp/pull/16846))
-* Reduce allocations in compiler checking via `ValueOption` usage ([PR #16822](https://github.com/dotnet/fsharp/pull/16822))
+* AsyncLocal diagnostics context. ([PR #16779](https://github.com/dotnet/fsharp/pull/16779))
+* Reduce allocations in compiler checking via `ValueOption` usage ([PR #16822](https://github.com/dotnet/fsharp/pull/16822))

docs/release-notes/.VisualStudio/17.11.md

@@ -1,3 +1,7 @@
 ### Fixed
 
 * Make tooltips work in file with no solution. ([PR #17054](https://github.com/dotnet/fsharp/pull/17054))
+
+### Changed
+
+* Use AsyncLocal diagnostics context. ([PR #16779])(https://github.com/dotnet/fsharp/pull/16779))

src/Compiler/Driver/CompilerConfig.fs

@@ -252,7 +252,7 @@ and IProjectReference =
     abstract FileName: string
 
     /// Evaluate raw contents of the assembly file generated by the project
-    abstract EvaluateRawContents: unit -> NodeCode<ProjectAssemblyDataResult>
+    abstract EvaluateRawContents: unit -> Async<ProjectAssemblyDataResult>
 
     /// Get the logical timestamp that would be the timestamp of the assembly file generated by the project
     ///

src/Compiler/Driver/CompilerConfig.fsi

@@ -86,7 +86,7 @@ and IProjectReference =
     /// Evaluate raw contents of the assembly file generated by the project.
     /// 'None' may be returned if an in-memory view of the contents is, for some reason,
     /// not available.  In this case the on-disk view of the contents will be preferred.
-    abstract EvaluateRawContents: unit -> NodeCode<ProjectAssemblyDataResult>
+    abstract EvaluateRawContents: unit -> Async<ProjectAssemblyDataResult>
 
     /// Get the logical timestamp that would be the timestamp of the assembly file generated by the project.
     ///

src/Compiler/Driver/CompilerImports.fs

@@ -2159,14 +2159,14 @@ and [<Sealed>] TcImports
         (
             ctok,
             r: AssemblyResolution
-        ) : NodeCode<(_ * (unit -> AvailableImportedAssembly list)) option> =
-        node {
+        ) : Async<(_ * (unit -> AvailableImportedAssembly list)) option> =
+        async {
             CheckDisposed()
             let m = r.originalReference.Range
             let fileName = r.resolvedPath
 
             let! contentsOpt =
-                node {
+                async {
                     match r.ProjectReference with
                     | Some ilb -> return! ilb.EvaluateRawContents()
                     | None -> return ProjectAssemblyDataResult.Unavailable true
@@ -2229,21 +2229,23 @@ and [<Sealed>] TcImports
 
     // NOTE: When used in the Language Service this can cause the transitive checking of projects. Hence it must be cancellable.
     member tcImports.RegisterAndImportReferencedAssemblies(ctok, nms: AssemblyResolution list) =
-        node {
+        async {
             CheckDisposed()
 
+
             let tcConfig = tcConfigP.Get ctok
 
             let runMethod =
                 match tcConfig.parallelReferenceResolution with
-                | ParallelReferenceResolution.On -> NodeCode.Parallel
-                | ParallelReferenceResolution.Off -> NodeCode.Sequential
+                | ParallelReferenceResolution.On -> MultipleDiagnosticsLoggers.Parallel
+                | ParallelReferenceResolution.Off -> MultipleDiagnosticsLoggers.Sequential
 
             let! results =
                 nms
                 |> List.map (fun nm ->
-                    node {
+                    async {
                         try
+                            use _ = new CompilationGlobalsScope()
                             return! tcImports.TryRegisterAndPrepareToImportReferencedDll(ctok, nm)
                         with e ->
                             errorR (Error(FSComp.SR.buildProblemReadingAssembly (nm.resolvedPath, e.Message), nm.originalReference.Range))
@@ -2283,7 +2285,7 @@ and [<Sealed>] TcImports
                     ReportWarnings warns
 
                     tcImports.RegisterAndImportReferencedAssemblies(ctok, res)
-                    |> NodeCode.RunImmediateWithoutCancellation
+                    |> Async.RunImmediate
                     |> ignore
 
                     true
@@ -2377,7 +2379,7 @@ and [<Sealed>] TcImports
     // we dispose TcImports is because we need to dispose type providers, and type providers are never included in the framework DLL set.
     // If a framework set ever includes type providers, you will not have to worry about explicitly calling Dispose as the Finalizer will handle it.
     static member BuildFrameworkTcImports(tcConfigP: TcConfigProvider, frameworkDLLs, nonFrameworkDLLs) =
-        node {
+        async {
             let ctok = CompilationThreadToken()
             let tcConfig = tcConfigP.Get ctok
 
@@ -2454,7 +2456,7 @@ and [<Sealed>] TcImports
                 resolvedAssemblies |> List.choose tryFindEquivPrimaryAssembly
 
             let! fslibCcu, fsharpCoreAssemblyScopeRef =
-                node {
+                async {
                     if tcConfig.compilingFSharpCore then
                         // When compiling FSharp.Core.dll, the fslibCcu reference to FSharp.Core.dll is a delayed ccu thunk fixed up during type checking
                         return CcuThunk.CreateDelayed getFSharpCoreLibraryName, ILScopeRef.Local
@@ -2548,7 +2550,7 @@ and [<Sealed>] TcImports
             dependencyProvider
         ) =
 
-        node {
+        async {
             let ctok = CompilationThreadToken()
             let tcConfig = tcConfigP.Get ctok
 
@@ -2566,7 +2568,7 @@ and [<Sealed>] TcImports
         }
 
     static member BuildTcImports(tcConfigP: TcConfigProvider, dependencyProvider) =
-        node {
+        async {
             let ctok = CompilationThreadToken()
             let tcConfig = tcConfigP.Get ctok
 
@@ -2598,7 +2600,7 @@ let RequireReferences (ctok, tcImports: TcImports, tcEnv, thisAssemblyName, reso
 
     let ccuinfos =
         tcImports.RegisterAndImportReferencedAssemblies(ctok, resolutions)
-        |> NodeCode.RunImmediateWithoutCancellation
+        |> Async.RunImmediate
 
     let asms =
         ccuinfos

src/Compiler/Driver/CompilerImports.fsi

@@ -199,14 +199,14 @@ type TcImports =
     member internal Base: TcImports option
 
     static member BuildFrameworkTcImports:
-        TcConfigProvider * AssemblyResolution list * AssemblyResolution list -> NodeCode<TcGlobals * TcImports>
+        TcConfigProvider * AssemblyResolution list * AssemblyResolution list -> Async<TcGlobals * TcImports>
 
     static member BuildNonFrameworkTcImports:
         TcConfigProvider * TcImports * AssemblyResolution list * UnresolvedAssemblyReference list * DependencyProvider ->
-            NodeCode<TcImports>
+            Async<TcImports>
 
     static member BuildTcImports:
-        tcConfigP: TcConfigProvider * dependencyProvider: DependencyProvider -> NodeCode<TcGlobals * TcImports>
+        tcConfigP: TcConfigProvider * dependencyProvider: DependencyProvider -> Async<TcGlobals * TcImports>
 
 /// Process a group of #r in F# Interactive.
 /// Adds the reference to the tcImports and add the ccu to the type checking environment.

src/Compiler/Driver/fsc.fs

@@ -615,7 +615,7 @@ let main1
     // Import basic assemblies
     let tcGlobals, frameworkTcImports =
         TcImports.BuildFrameworkTcImports(foundationalTcConfigP, sysRes, otherRes)
-        |> NodeCode.RunImmediateWithoutCancellation
+        |> Async.RunImmediate
 
     let ilSourceDocs =
         [
@@ -664,7 +664,7 @@ let main1
 
     let tcImports =
         TcImports.BuildNonFrameworkTcImports(tcConfigP, frameworkTcImports, otherRes, knownUnresolved, dependencyProvider)
-        |> NodeCode.RunImmediateWithoutCancellation
+        |> Async.RunImmediate
 
     // register tcImports to be disposed in future
     disposables.Register tcImports

src/Compiler/Facilities/AsyncMemoize.fs

@@ -51,11 +51,11 @@ type internal MemoizeReply<'TValue> =
     | New of CancellationToken
     | Existing of Task<'TValue>
 
-type internal MemoizeRequest<'TValue> = GetOrCompute of NodeCode<'TValue> * CancellationToken
+type internal MemoizeRequest<'TValue> = GetOrCompute of Async<'TValue> * CancellationToken
 
 [<DebuggerDisplay("{DebuggerDisplay}")>]
 type internal Job<'TValue> =
-    | Running of TaskCompletionSource<'TValue> * CancellationTokenSource * NodeCode<'TValue> * DateTime * ResizeArray<DiagnosticsLogger>
+    | Running of TaskCompletionSource<'TValue> * CancellationTokenSource * Async<'TValue> * DateTime * ResizeArray<DiagnosticsLogger>
     | Completed of 'TValue * (PhasedDiagnostic * FSharpDiagnosticSeverity) list
     | Canceled of DateTime
     | Failed of DateTime * exn // TODO: probably we don't need to keep this
@@ -286,7 +286,13 @@ type internal AsyncMemoize<'TKey, 'TVersion, 'TValue when 'TKey: equality and 'T
                             key.Key,
                             key.Version,
                             key.Label,
-                            (Running(TaskCompletionSource(), cts, computation, DateTime.Now, ResizeArray()))
+                            (Running(
+                                TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously),
+                                cts,
+                                computation,
+                                DateTime.Now,
+                                ResizeArray()
+                            ))
                         )
 
                         otherVersions
@@ -314,7 +320,7 @@ type internal AsyncMemoize<'TKey, 'TVersion, 'TValue when 'TKey: equality and 'T
 
     let processStateUpdate post (key: KeyData<_, _>, action: StateUpdate<_>) =
         task {
-            do! Task.Delay 0
+            do! Task.Yield()
 
             do!
                 lock.Do(fun () ->
@@ -359,7 +365,7 @@ type internal AsyncMemoize<'TKey, 'TVersion, 'TValue when 'TKey: equality and 'T
                                                 DiagnosticsThreadStatics.DiagnosticsLogger <- cachingLogger
 
                                                 try
-                                                    let! result = computation |> Async.AwaitNodeCode
+                                                    let! result = computation
                                                     post (key, (JobCompleted(result, cachingLogger.CapturedDiagnostics)))
                                                     return ()
                                                 finally
@@ -482,14 +488,14 @@ type internal AsyncMemoize<'TKey, 'TVersion, 'TValue when 'TKey: equality and 'T
                 Version = key.GetVersion()
             }
 
-        node {
-            let! ct = NodeCode.CancellationToken
+        async {
+            let! ct = Async.CancellationToken
 
             let callerDiagnosticLogger = DiagnosticsThreadStatics.DiagnosticsLogger
 
             match!
                 processRequest post (key, GetOrCompute(computation, ct)) callerDiagnosticLogger
-                |> NodeCode.AwaitTask
+                |> Async.AwaitTask
             with
             | New internalCt ->
 
@@ -507,15 +513,15 @@ type internal AsyncMemoize<'TKey, 'TVersion, 'TValue when 'TKey: equality and 'T
                                 log (Started, key)
 
                                 try
-                                    let! result = computation |> Async.AwaitNodeCode
+                                    let! result = computation
                                     post (key, (JobCompleted(result, cachingLogger.CapturedDiagnostics)))
                                     return result
                                 finally
                                     DiagnosticsThreadStatics.DiagnosticsLogger <- currentLogger
                             },
                             cancellationToken = linkedCtSource.Token
                         )
-                        |> NodeCode.AwaitTask
+                        |> Async.AwaitTask
                 with
                 | TaskCancelled ex ->
                     // TODO: do we need to do anything else here? Presumably it should be done by the registration on
@@ -531,7 +537,7 @@ type internal AsyncMemoize<'TKey, 'TVersion, 'TValue when 'TKey: equality and 'T
                     post (key, (JobFailed(ex, cachingLogger.CapturedDiagnostics)))
                     return raise ex
 
-            | Existing job -> return! job |> NodeCode.AwaitTask
+            | Existing job -> return! job |> Async.AwaitTask
 
         }
 

src/Compiler/Facilities/AsyncMemoize.fsi

@@ -66,9 +66,9 @@ type internal AsyncMemoize<'TKey, 'TVersion, 'TValue when 'TKey: equality and 'T
 
     member Clear: predicate: ('TKey -> bool) -> unit
 
-    member Get: key: ICacheKey<'TKey, 'TVersion> * computation: NodeCode<'TValue> -> NodeCode<'TValue>
+    member Get: key: ICacheKey<'TKey, 'TVersion> * computation: Async<'TValue> -> Async<'TValue>
 
-    member Get': key: 'TKey * computation: NodeCode<'TValue> -> NodeCode<'TValue>
+    member Get': key: 'TKey * computation: Async<'TValue> -> Async<'TValue>
 
     member TryGet: key: 'TKey * predicate: ('TVersion -> bool) -> 'TValue option