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

AsyncLocal diagnostics, clean slate #16779

Merged
merged 98 commits into from
May 15, 2024
Merged
Show file tree
Hide file tree
Changes from 74 commits
Commits
Show all changes
98 commits
Select commit Hold shift + click to select a range
7a41d85
fix some inconsistencies
majocha Feb 23, 2024
22e955a
we don't intend to dispose this ever
majocha Feb 23, 2024
a0d5694
Merge branch 'main' into use-logger-fixes
majocha Feb 23, 2024
8768355
Merge branch 'main' into use-logger-fixes
majocha Feb 24, 2024
86db4d3
Merge branch 'main' into use-logger-fixes
majocha Feb 26, 2024
8be519e
removed
majocha Feb 26, 2024
7351011
Merge branch 'use-logger-fixes' into al-wip
majocha Feb 26, 2024
8e9a815
multiple loggers
majocha Feb 26, 2024
6dab107
fix RegisterAndImportReferencedAssemblies
majocha Feb 26, 2024
e0fbb8d
fix deadlock in fsi
majocha Feb 27, 2024
70beb11
fix some BuildGraphTests
majocha Feb 27, 2024
757d6cd
restore asyncmemoize
majocha Feb 27, 2024
3d56838
fix missing logger in test
majocha Feb 27, 2024
e6a1640
Merge remote-tracking branch 'dotnet/main' into al-wip
majocha Feb 27, 2024
cb72156
format
majocha Feb 27, 2024
afaf692
fix transparent compiler nre
majocha Feb 27, 2024
cc7a690
cleanup
majocha Feb 27, 2024
fd105bd
default value
majocha Feb 27, 2024
51ad4ce
add some comments
majocha Feb 28, 2024
c271c57
foramt and notes to make it green if it's green
majocha Feb 28, 2024
0ca7fe6
wrap any parallel computations that potentially push diagnostics
majocha Feb 28, 2024
ee46955
try to fix buildgraphtests
majocha Feb 28, 2024
7c3cd98
try to eradicate deadlocks in tests
majocha Feb 28, 2024
6afb997
Merge remote-tracking branch 'fsharp/main' into al-wip
majocha Feb 28, 2024
2d0a74e
fix buildgraph test
majocha Feb 28, 2024
adf0417
disable for a moment
majocha Feb 28, 2024
204ed81
Merge branch 'main' into al-wip
majocha Feb 28, 2024
3673b46
flatten exceptions
majocha Feb 28, 2024
cafa476
Merge remote-tracking branch 'dotnet/main' into al-wip
majocha Feb 28, 2024
fbb3a4a
reshuffle
majocha Feb 28, 2024
e793736
Merge remote-tracking branch 'dotnet/main' into al-wip
majocha Feb 28, 2024
46e81bd
prevent graphnode deadlock
majocha Feb 28, 2024
29563c1
yield
majocha Feb 29, 2024
24180d0
diff
majocha Feb 29, 2024
63492dc
add some comments
majocha Feb 29, 2024
b025759
Merge branch 'main' into al-wip
majocha Mar 1, 2024
0e3f726
add AsyncLocal test
majocha Mar 1, 2024
779b11b
more testing
majocha Mar 1, 2024
b44b6e0
Merge branch 'main' into al-wip
majocha Mar 1, 2024
c1ce15c
Merge branch 'main' into al-wip
majocha Mar 1, 2024
335b618
Merge branch 'main' into al-wip
majocha Mar 4, 2024
0c1f805
Merge branch 'main' into al-wip
majocha Mar 4, 2024
2882de9
Merge branch 'main' into al-wip
majocha Mar 5, 2024
0b367bd
Merge branch 'main' into al-wip
majocha Mar 5, 2024
ecba2ec
Merge branch 'main' into al-wip
majocha Mar 5, 2024
049e276
Merge branch 'main' into al-wip
majocha Mar 6, 2024
7093530
Merge branch 'main' into al-wip
majocha Mar 6, 2024
69e49a7
not needed after all?
majocha Mar 6, 2024
bd6f5c1
test
majocha Mar 6, 2024
fc1293b
Merge branch 'main' into al-wip
majocha Mar 7, 2024
a650e04
format
majocha Mar 7, 2024
6eba688
revert
majocha Mar 7, 2024
5ed3239
Merge branch 'main' into al-wip
majocha Mar 7, 2024
128ca82
Merge branch 'main' into al-wip
majocha Mar 7, 2024
fb91545
try RunContinuationsAsynchronously
majocha Mar 8, 2024
c96ff79
Merge branch 'main' into al-wip
majocha Mar 8, 2024
00821e3
try to deal with deadlock another way
majocha Mar 10, 2024
c570ff6
Merge branch 'main' into al-wip
majocha Mar 11, 2024
75e4457
nope
majocha Mar 11, 2024
0b43270
Merge branch 'al-wip' of https://github.com/majocha/fsharp into al-wip
majocha Mar 11, 2024
2bbddd8
Merge branch 'main' into al-wip
majocha Mar 11, 2024
6bd5a08
revert
majocha Mar 11, 2024
7ca795f
Merge branch 'al-wip' of https://github.com/majocha/fsharp into al-wip
majocha Mar 11, 2024
4574370
Merge branch 'main' into al-wip
majocha Mar 11, 2024
7783850
format
majocha Mar 12, 2024
8519c78
Merge branch 'main' into al-wip
majocha Mar 12, 2024
cc486c7
Merge branch 'al-wip' of https://github.com/majocha/fsharp into al-wip
majocha Mar 12, 2024
4508fe8
Merge branch 'main' into al-wip
majocha Mar 12, 2024
05c6298
Merge branch 'main' into al-wip
majocha Mar 13, 2024
4420340
Merge branch 'main' into al-wip
majocha Mar 14, 2024
153bfb8
merge main
majocha Mar 23, 2024
6200cbc
merge main
majocha Apr 29, 2024
862870b
restore release notes
majocha Apr 29, 2024
50fd92c
Merge branch 'main' into al-wip
majocha Apr 30, 2024
1f2c7f3
add test for ListParallel
majocha Apr 30, 2024
d94c8bd
merge main
majocha May 3, 2024
b7922f2
Merge remote-tracking branch 'dotnet/main' into al-wip
majocha May 3, 2024
e05cfe5
speed up test
majocha May 4, 2024
b9b01eb
improve test
majocha May 5, 2024
87ccfa3
simpler MultipleDiagnosticsLoggers
majocha May 6, 2024
f4cf7e6
rename and speedup test
majocha May 6, 2024
2dccbba
Merge branch 'main' into al-wip
majocha May 7, 2024
f94c3da
parallel logging perf
majocha May 7, 2024
e30dbde
Merge remote-tracking branch 'fsharp/main' into al-wip
majocha May 7, 2024
f4eeac9
Merge remote-tracking branch 'dotnet/main' into al-wip
majocha May 8, 2024
27dd989
remove SwitchToThreadPool
majocha May 8, 2024
8c77c0c
sequential
majocha May 9, 2024
76ae65f
Revert "sequential"
majocha May 9, 2024
684462e
Merge branch 'main' into al-wip
majocha May 9, 2024
4d5248f
Merge branch 'main' into al-wip
majocha May 13, 2024
1330fe0
Merge branch 'main' into al-wip
majocha May 13, 2024
45323a7
Merge branch 'main' into al-wip
psfinaki May 14, 2024
aec0451
revert spurious change
majocha May 14, 2024
f711156
fix comments
majocha May 14, 2024
a2641f5
test MultipleDiagnosticsLoggers
majocha May 14, 2024
afdd041
add comment and clean up
majocha May 14, 2024
f22f2f2
Merge branch 'main' into al-wip
majocha May 14, 2024
a67d898
Merge branch 'main' into al-wip
majocha May 15, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/release-notes/.FSharp.Compiler.Service/8.0.400.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,4 @@
### Added

* Generate new `Equals` overload to avoid boxing for structural comparison ([PR #16857](https://github.com/dotnet/fsharp/pull/16857))
* AsyncLocal diagnostics context. ([PR #16779](https://github.com/dotnet/fsharp/pull/16779))
4 changes: 4 additions & 0 deletions docs/release-notes/.VisualStudio/17.11.md
Original file line number Diff line number Diff line change
@@ -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))
2 changes: 1 addition & 1 deletion src/Compiler/Driver/CompilerConfig.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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
///
Expand Down
2 changes: 1 addition & 1 deletion src/Compiler/Driver/CompilerConfig.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down
30 changes: 17 additions & 13 deletions src/Compiler/Driver/CompilerImports.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -2229,21 +2229,25 @@ 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()

// Prevent deadlocks in FSI.
majocha marked this conversation as resolved.
Show resolved Hide resolved
do! Async.SwitchToThreadPool()

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))
Expand Down Expand Up @@ -2283,7 +2287,7 @@ and [<Sealed>] TcImports
ReportWarnings warns

tcImports.RegisterAndImportReferencedAssemblies(ctok, res)
|> NodeCode.RunImmediateWithoutCancellation
|> Async.RunImmediate
|> ignore

true
Expand Down Expand Up @@ -2377,7 +2381,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

Expand Down Expand Up @@ -2454,7 +2458,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
Expand Down Expand Up @@ -2548,7 +2552,7 @@ and [<Sealed>] TcImports
dependencyProvider
) =

node {
async {
let ctok = CompilationThreadToken()
let tcConfig = tcConfigP.Get ctok

Expand All @@ -2566,7 +2570,7 @@ and [<Sealed>] TcImports
}

static member BuildTcImports(tcConfigP: TcConfigProvider, dependencyProvider) =
node {
async {
let ctok = CompilationThreadToken()
let tcConfig = tcConfigP.Get ctok

Expand Down Expand Up @@ -2598,7 +2602,7 @@ let RequireReferences (ctok, tcImports: TcImports, tcEnv, thisAssemblyName, reso

let ccuinfos =
tcImports.RegisterAndImportReferencedAssemblies(ctok, resolutions)
|> NodeCode.RunImmediateWithoutCancellation
|> Async.RunImmediate

let asms =
ccuinfos
Expand Down
6 changes: 3 additions & 3 deletions src/Compiler/Driver/CompilerImports.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
3 changes: 2 additions & 1 deletion src/Compiler/Driver/GraphChecking/GraphProcessing.fs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,8 @@ let processGraphAsync<'Item, 'Result when 'Item: equality and 'Item: comparison>
let! parentCt = Async.CancellationToken
use localCts = new CancellationTokenSource()

let completionSignal = TaskCompletionSource()
let completionSignal =
TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously)
majocha marked this conversation as resolved.
Show resolved Hide resolved

use _ = parentCt.Register(fun () -> completionSignal.TrySetCanceled() |> ignore)

Expand Down
4 changes: 2 additions & 2 deletions src/Compiler/Driver/fsc.fs
Original file line number Diff line number Diff line change
Expand Up @@ -615,7 +615,7 @@ let main1
// Import basic assemblies
let tcGlobals, frameworkTcImports =
TcImports.BuildFrameworkTcImports(foundationalTcConfigP, sysRes, otherRes)
|> NodeCode.RunImmediateWithoutCancellation
|> Async.RunImmediate

let ilSourceDocs =
[
Expand Down Expand Up @@ -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
Expand Down
28 changes: 17 additions & 11 deletions src/Compiler/Facilities/AsyncMemoize.fs
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,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
Expand Down Expand Up @@ -285,7 +285,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
Expand Down Expand Up @@ -313,7 +319,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 () ->
Expand Down Expand Up @@ -358,7 +364,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
Expand Down Expand Up @@ -481,14 +487,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 ->

Expand All @@ -506,15 +512,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
Expand All @@ -530,7 +536,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

}

Expand Down
4 changes: 2 additions & 2 deletions src/Compiler/Facilities/AsyncMemoize.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,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

Expand Down
Loading
Loading