From 9709fe0797103c2bfce20657dc5b6aba43224cd4 Mon Sep 17 00:00:00 2001 From: Jakub Majocha Date: Tue, 27 Feb 2024 11:14:24 +0100 Subject: [PATCH 1/6] start all over --- src/Compiler/Facilities/BuildGraph.fs | 18 +++++++++--------- src/Compiler/Facilities/DiagnosticsLogger.fs | 18 ++++++++++++++++++ src/Compiler/Facilities/DiagnosticsLogger.fsi | 2 ++ 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/Compiler/Facilities/BuildGraph.fs b/src/Compiler/Facilities/BuildGraph.fs index b4abe3ad1ed..4ddc45696fc 100644 --- a/src/Compiler/Facilities/BuildGraph.fs +++ b/src/Compiler/Facilities/BuildGraph.fs @@ -15,13 +15,13 @@ type NodeCode<'T> = Node of Async<'T> let wrapThreadStaticInfo computation = async { - let diagnosticsLogger = DiagnosticsThreadStatics.DiagnosticsLogger + let diagnosticsLogger = DiagnosticsThreadStatics.DiagnosticsLoggerNodeCode let phase = DiagnosticsThreadStatics.BuildPhase try return! computation finally - DiagnosticsThreadStatics.DiagnosticsLogger <- diagnosticsLogger + DiagnosticsThreadStatics.DiagnosticsLoggerNodeCode <- diagnosticsLogger DiagnosticsThreadStatics.BuildPhase <- phase } @@ -98,7 +98,7 @@ type NodeCodeBuilder() = member _.Using(value: CompilationGlobalsScope, binder: CompilationGlobalsScope -> NodeCode<'U>) = Node( async { - DiagnosticsThreadStatics.DiagnosticsLogger <- value.DiagnosticsLogger + DiagnosticsThreadStatics.DiagnosticsLoggerNodeCode <- value.DiagnosticsLogger DiagnosticsThreadStatics.BuildPhase <- value.BuildPhase try @@ -125,21 +125,21 @@ type NodeCode private () = static let cancellationToken = Node(wrapThreadStaticInfo Async.CancellationToken) static member RunImmediate(computation: NodeCode<'T>, ct: CancellationToken) = - let diagnosticsLogger = DiagnosticsThreadStatics.DiagnosticsLogger + let diagnosticsLogger = DiagnosticsThreadStatics.DiagnosticsLoggerNodeCode let phase = DiagnosticsThreadStatics.BuildPhase try try let work = async { - DiagnosticsThreadStatics.DiagnosticsLogger <- diagnosticsLogger + DiagnosticsThreadStatics.DiagnosticsLoggerNodeCode <- diagnosticsLogger DiagnosticsThreadStatics.BuildPhase <- phase return! computation |> Async.AwaitNodeCode } Async.StartImmediateAsTask(work, cancellationToken = ct).Result finally - DiagnosticsThreadStatics.DiagnosticsLogger <- diagnosticsLogger + DiagnosticsThreadStatics.DiagnosticsLoggerNodeCode <- diagnosticsLogger DiagnosticsThreadStatics.BuildPhase <- phase with :? AggregateException as ex when ex.InnerExceptions.Count = 1 -> raise (ex.InnerExceptions[0]) @@ -148,20 +148,20 @@ type NodeCode private () = NodeCode.RunImmediate(computation, CancellationToken.None) static member StartAsTask_ForTesting(computation: NodeCode<'T>, ?ct: CancellationToken) = - let diagnosticsLogger = DiagnosticsThreadStatics.DiagnosticsLogger + let diagnosticsLogger = DiagnosticsThreadStatics.DiagnosticsLoggerNodeCode let phase = DiagnosticsThreadStatics.BuildPhase try let work = async { - DiagnosticsThreadStatics.DiagnosticsLogger <- diagnosticsLogger + DiagnosticsThreadStatics.DiagnosticsLoggerNodeCode <- diagnosticsLogger DiagnosticsThreadStatics.BuildPhase <- phase return! computation |> Async.AwaitNodeCode } Async.StartAsTask(work, cancellationToken = defaultArg ct CancellationToken.None) finally - DiagnosticsThreadStatics.DiagnosticsLogger <- diagnosticsLogger + DiagnosticsThreadStatics.DiagnosticsLoggerNodeCode <- diagnosticsLogger DiagnosticsThreadStatics.BuildPhase <- phase static member CancellationToken = cancellationToken diff --git a/src/Compiler/Facilities/DiagnosticsLogger.fs b/src/Compiler/Facilities/DiagnosticsLogger.fs index 75dfaaef39e..933d2fe26f7 100644 --- a/src/Compiler/Facilities/DiagnosticsLogger.fs +++ b/src/Compiler/Facilities/DiagnosticsLogger.fs @@ -375,6 +375,15 @@ type CapturingDiagnosticsLogger(nm, ?eagerFormat) = let errors = diagnostics.ToArray() errors |> Array.iter diagnosticsLogger.DiagnosticSink + +let currentDiagnosticsLogger = AsyncLocal() + +let checkIfSame v = + match currentDiagnosticsLogger.Value, v with + | c, _ when box c |> isNull -> v + | c, v when c = v -> v + | _ -> failwith "AsyncLocal does not match ThreadStatic." + /// Type holds thread-static globals for use by the compiler. type internal DiagnosticsThreadStatics = [] @@ -393,6 +402,15 @@ type internal DiagnosticsThreadStatics = and set v = DiagnosticsThreadStatics.buildPhase <- v static member DiagnosticsLogger + with get () = + match box DiagnosticsThreadStatics.diagnosticsLogger with + | Null -> AssertFalseDiagnosticsLogger + | _ -> checkIfSame DiagnosticsThreadStatics.diagnosticsLogger + and set v = + currentDiagnosticsLogger.Value <- v + DiagnosticsThreadStatics.diagnosticsLogger <- v + + static member DiagnosticsLoggerNodeCode with get () = match box DiagnosticsThreadStatics.diagnosticsLogger with | Null -> AssertFalseDiagnosticsLogger diff --git a/src/Compiler/Facilities/DiagnosticsLogger.fsi b/src/Compiler/Facilities/DiagnosticsLogger.fsi index bcbdd197b73..ddd7b871abc 100644 --- a/src/Compiler/Facilities/DiagnosticsLogger.fsi +++ b/src/Compiler/Facilities/DiagnosticsLogger.fsi @@ -236,6 +236,8 @@ type DiagnosticsThreadStatics = static member DiagnosticsLogger: DiagnosticsLogger with get, set + static member DiagnosticsLoggerNodeCode: DiagnosticsLogger with get, set + [] module DiagnosticsLoggerExtensions = From 617a2d0312f09be5cd306a1572d2cdd9723c4551 Mon Sep 17 00:00:00 2001 From: Jakub Majocha Date: Tue, 27 Feb 2024 11:16:54 +0100 Subject: [PATCH 2/6] wrap TryRegisterAndPrepareToImportReferencedDll --- src/Compiler/Driver/CompilerImports.fs | 2 ++ src/Compiler/Facilities/DiagnosticsLogger.fs | 2 ++ src/Compiler/Facilities/DiagnosticsLogger.fsi | 2 ++ 3 files changed, 6 insertions(+) diff --git a/src/Compiler/Driver/CompilerImports.fs b/src/Compiler/Driver/CompilerImports.fs index d8d9ccd9866..29169bf05cf 100644 --- a/src/Compiler/Driver/CompilerImports.fs +++ b/src/Compiler/Driver/CompilerImports.fs @@ -2243,6 +2243,8 @@ and [] TcImports |> List.map (fun nm -> node { try + use _ = new CompilationGlobalsScope() + return! tcImports.TryRegisterAndPrepareToImportReferencedDll(ctok, nm) with e -> errorR (Error(FSComp.SR.buildProblemReadingAssembly (nm.resolvedPath, e.Message), nm.originalReference.Range)) diff --git a/src/Compiler/Facilities/DiagnosticsLogger.fs b/src/Compiler/Facilities/DiagnosticsLogger.fs index 933d2fe26f7..aa30edc525e 100644 --- a/src/Compiler/Facilities/DiagnosticsLogger.fs +++ b/src/Compiler/Facilities/DiagnosticsLogger.fs @@ -551,6 +551,8 @@ type CompilationGlobalsScope(diagnosticsLogger: DiagnosticsLogger, buildPhase: B let unwindEL = UseDiagnosticsLogger diagnosticsLogger let unwindBP = UseBuildPhase buildPhase + new() = CompilationGlobalsScope(DiagnosticsThreadStatics.DiagnosticsLogger, DiagnosticsThreadStatics.BuildPhase) + member _.DiagnosticsLogger = diagnosticsLogger member _.BuildPhase = buildPhase diff --git a/src/Compiler/Facilities/DiagnosticsLogger.fsi b/src/Compiler/Facilities/DiagnosticsLogger.fsi index ddd7b871abc..4321407961c 100644 --- a/src/Compiler/Facilities/DiagnosticsLogger.fsi +++ b/src/Compiler/Facilities/DiagnosticsLogger.fsi @@ -460,6 +460,8 @@ type StackGuard = type CompilationGlobalsScope = new: diagnosticsLogger: DiagnosticsLogger * buildPhase: BuildPhase -> CompilationGlobalsScope + new: unit -> CompilationGlobalsScope + interface IDisposable member DiagnosticsLogger: DiagnosticsLogger From 030bcb696c9284ccef15db8c9bc5397c1adc333b Mon Sep 17 00:00:00 2001 From: Jakub Majocha Date: Tue, 27 Feb 2024 15:37:21 +0100 Subject: [PATCH 3/6] some special cases --- src/Compiler/Facilities/BuildGraph.fs | 9 ++++++++- src/Compiler/Facilities/DiagnosticsLogger.fs | 16 +++++++++++----- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/Compiler/Facilities/BuildGraph.fs b/src/Compiler/Facilities/BuildGraph.fs index 4ddc45696fc..c237647f16b 100644 --- a/src/Compiler/Facilities/BuildGraph.fs +++ b/src/Compiler/Facilities/BuildGraph.fs @@ -31,7 +31,14 @@ type Async<'T> with static member AwaitNodeCode(node: NodeCode<'T>) = match node with - | Node(computation) -> wrapThreadStaticInfo computation + | Node(computation) -> + async { + try + return! wrapThreadStaticInfo computation + finally + // Reset threadstatics so they don't leak to unrelated computation. + DiagnosticsThreadStatics.DiagnosticsLoggerNodeCode <- AssertFalseDiagnosticsLogger + } [] type NodeCodeBuilder() = diff --git a/src/Compiler/Facilities/DiagnosticsLogger.fs b/src/Compiler/Facilities/DiagnosticsLogger.fs index aa30edc525e..049857de03e 100644 --- a/src/Compiler/Facilities/DiagnosticsLogger.fs +++ b/src/Compiler/Facilities/DiagnosticsLogger.fs @@ -376,12 +376,18 @@ type CapturingDiagnosticsLogger(nm, ?eagerFormat) = errors |> Array.iter diagnosticsLogger.DiagnosticSink -let currentDiagnosticsLogger = AsyncLocal() +let currentDiagnosticsLogger = AsyncLocal() let checkIfSame v = match currentDiagnosticsLogger.Value, v with - | c, _ when box c |> isNull -> v - | c, v when c = v -> v + | ValueSome c, v when c = v -> v // Good. + | ValueNone, _ -> v // New context. + | _, v when v = AssertFalseDiagnosticsLogger -> v // Not initialized but will catch up. + | ValueSome c, _ when c = AssertFalseDiagnosticsLogger -> v // Not initialized but will catch up. + | _, v when v.DebugDisplay().Contains("CachingDiagnosticsLogger") -> v // AsyncMemoize, disregard for now. + | ValueSome c, _ when c.DebugDisplay().Contains("CachingDiagnosticsLogger") -> v // AsyncMemoize, disregard for now. + | ValueSome c, _ when c.DebugDisplay().Contains("NodeCode") -> v // NodeCode.Parallel boundary. + | _, v when v.DebugDisplay().Contains("NodeCode") -> v // NodeCode.Parallel boundary. | _ -> failwith "AsyncLocal does not match ThreadStatic." /// Type holds thread-static globals for use by the compiler. @@ -407,7 +413,7 @@ type internal DiagnosticsThreadStatics = | Null -> AssertFalseDiagnosticsLogger | _ -> checkIfSame DiagnosticsThreadStatics.diagnosticsLogger and set v = - currentDiagnosticsLogger.Value <- v + currentDiagnosticsLogger.Value <- ValueSome v DiagnosticsThreadStatics.diagnosticsLogger <- v static member DiagnosticsLoggerNodeCode @@ -551,7 +557,7 @@ type CompilationGlobalsScope(diagnosticsLogger: DiagnosticsLogger, buildPhase: B let unwindEL = UseDiagnosticsLogger diagnosticsLogger let unwindBP = UseBuildPhase buildPhase - new() = CompilationGlobalsScope(DiagnosticsThreadStatics.DiagnosticsLogger, DiagnosticsThreadStatics.BuildPhase) + new() = new CompilationGlobalsScope(DiagnosticsThreadStatics.DiagnosticsLogger, DiagnosticsThreadStatics.BuildPhase) member _.DiagnosticsLogger = diagnosticsLogger member _.BuildPhase = buildPhase From f89e08b2b815a5f0af42fd8ade48ae875d4f4cf6 Mon Sep 17 00:00:00 2001 From: Jakub Majocha Date: Tue, 27 Feb 2024 16:18:03 +0100 Subject: [PATCH 4/6] f --- src/Compiler/Facilities/DiagnosticsLogger.fs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Compiler/Facilities/DiagnosticsLogger.fs b/src/Compiler/Facilities/DiagnosticsLogger.fs index 049857de03e..3c56e8dc95c 100644 --- a/src/Compiler/Facilities/DiagnosticsLogger.fs +++ b/src/Compiler/Facilities/DiagnosticsLogger.fs @@ -375,7 +375,6 @@ type CapturingDiagnosticsLogger(nm, ?eagerFormat) = let errors = diagnostics.ToArray() errors |> Array.iter diagnosticsLogger.DiagnosticSink - let currentDiagnosticsLogger = AsyncLocal() let checkIfSame v = From 2e7e578aa0c7ce499213091c83b47692df9853f5 Mon Sep 17 00:00:00 2001 From: Jakub Majocha Date: Mon, 4 Mar 2024 15:03:34 +0100 Subject: [PATCH 5/6] deparallelize for a moment --- src/Compiler/Facilities/AsyncMemoize.fs | 4 +++ src/Compiler/Facilities/BuildGraph.fs | 32 +++++++++++--------- src/Compiler/Facilities/DiagnosticsLogger.fs | 27 ++++++++++------- src/Compiler/Service/IncrementalBuild.fs | 26 ++++++++-------- 4 files changed, 51 insertions(+), 38 deletions(-) diff --git a/src/Compiler/Facilities/AsyncMemoize.fs b/src/Compiler/Facilities/AsyncMemoize.fs index d6ae83ada6e..f2bbeaaae3e 100644 --- a/src/Compiler/Facilities/AsyncMemoize.fs +++ b/src/Compiler/Facilities/AsyncMemoize.fs @@ -463,6 +463,8 @@ type internal AsyncMemoize<'TKey, 'TVersion, 'TValue when 'TKey: equality and 'T member this.Get'(key, computation) = + failwith "AsyncMemoize" + let wrappedKey = { new ICacheKey<_, _> with member _.GetKey() = key @@ -474,6 +476,8 @@ type internal AsyncMemoize<'TKey, 'TVersion, 'TValue when 'TKey: equality and 'T member _.Get(key: ICacheKey<_, _>, computation) = + failwith "AsyncMemoize" + let key = { Label = key.GetLabel() diff --git a/src/Compiler/Facilities/BuildGraph.fs b/src/Compiler/Facilities/BuildGraph.fs index c237647f16b..c6202b0839b 100644 --- a/src/Compiler/Facilities/BuildGraph.fs +++ b/src/Compiler/Facilities/BuildGraph.fs @@ -25,20 +25,18 @@ let wrapThreadStaticInfo computation = DiagnosticsThreadStatics.BuildPhase <- phase } + +let reset() = + DiagnosticsThreadStatics.DiagnosticsLoggerNodeCode <- AssertFalseDiagnosticsLogger + DiagnosticsThreadStatics.BuildPhase <- BuildPhase.DefaultPhase + let unwrapNode (Node(computation)) = computation type Async<'T> with static member AwaitNodeCode(node: NodeCode<'T>) = match node with - | Node(computation) -> - async { - try - return! wrapThreadStaticInfo computation - finally - // Reset threadstatics so they don't leak to unrelated computation. - DiagnosticsThreadStatics.DiagnosticsLoggerNodeCode <- AssertFalseDiagnosticsLogger - } + | Node(computation) -> wrapThreadStaticInfo computation [] type NodeCodeBuilder() = @@ -212,9 +210,12 @@ type NodeCode private () = let logger = concurrentLogging.GetLoggerForTask($"NodeCode.Parallel {i}") async { - DiagnosticsThreadStatics.DiagnosticsLogger <- logger - DiagnosticsThreadStatics.BuildPhase <- phase - return! unwrapNode computation + try + DiagnosticsThreadStatics.DiagnosticsLogger <- logger + DiagnosticsThreadStatics.BuildPhase <- phase + return! unwrapNode computation + finally + reset() } return! @@ -263,6 +264,7 @@ type GraphNode<'T> private (computation: NodeCode<'T>, cachedResult: ValueOption cachedResultNode else node { + use _ = new CompilationGlobalsScope() Interlocked.Increment(&requestCount) |> ignore try @@ -291,13 +293,13 @@ type GraphNode<'T> private (computation: NodeCode<'T>, cachedResult: ValueOption | ValueSome value -> return value | _ -> let tcs = TaskCompletionSource<'T>() - let (Node(p)) = computation Async.StartWithContinuations( - async { + node { Thread.CurrentThread.CurrentUICulture <- GraphNode.culture - return! p - }, + reset() + return! computation + } |> Async.AwaitNodeCode, (fun res -> cachedResult <- ValueSome res cachedResultNode <- node.Return res diff --git a/src/Compiler/Facilities/DiagnosticsLogger.fs b/src/Compiler/Facilities/DiagnosticsLogger.fs index 3c56e8dc95c..4ebf1e494b9 100644 --- a/src/Compiler/Facilities/DiagnosticsLogger.fs +++ b/src/Compiler/Facilities/DiagnosticsLogger.fs @@ -377,17 +377,22 @@ type CapturingDiagnosticsLogger(nm, ?eagerFormat) = let currentDiagnosticsLogger = AsyncLocal() -let checkIfSame v = - match currentDiagnosticsLogger.Value, v with - | ValueSome c, v when c = v -> v // Good. - | ValueNone, _ -> v // New context. - | _, v when v = AssertFalseDiagnosticsLogger -> v // Not initialized but will catch up. - | ValueSome c, _ when c = AssertFalseDiagnosticsLogger -> v // Not initialized but will catch up. - | _, v when v.DebugDisplay().Contains("CachingDiagnosticsLogger") -> v // AsyncMemoize, disregard for now. - | ValueSome c, _ when c.DebugDisplay().Contains("CachingDiagnosticsLogger") -> v // AsyncMemoize, disregard for now. - | ValueSome c, _ when c.DebugDisplay().Contains("NodeCode") -> v // NodeCode.Parallel boundary. - | _, v when v.DebugDisplay().Contains("NodeCode") -> v // NodeCode.Parallel boundary. - | _ -> failwith "AsyncLocal does not match ThreadStatic." +let checkIfSame threadStatic = + match currentDiagnosticsLogger.Value with + + // Big hurdle in testing this is the fact that threadstatics leak from one unrelated test to another. + // At least that's what happens in VS test runner. In effect, at the beginning of some computation when we expect + // to see uninitialized threadstatic, we get some value that stayed alive from another test function. + // Especially FinalizeTypeCheckTask shows up here a lot. + // That's why we have to disregard the threadstatics at the beggining of each execution context, + // up to the moment of proper initialization (the first call of UseDiagnosticsLogger usually). + | ValueNone -> threadStatic + + | ValueSome asyncLocal -> + match asyncLocal, threadStatic with + | a, t when a = t -> t // Good. + | _ when threadStatic = AssertFalseDiagnosticsLogger -> threadStatic // AsyncLocal is always good on thread switches. ThreadStatic needs to catch up. + | _ -> failwith $"AsyncLocal does not match ThreadStatic. a: {asyncLocal.DebugDisplay()} t: {threadStatic.DebugDisplay()}" /// Type holds thread-static globals for use by the compiler. type internal DiagnosticsThreadStatics = diff --git a/src/Compiler/Service/IncrementalBuild.fs b/src/Compiler/Service/IncrementalBuild.fs index f59a1e9b6a5..a95542d41c0 100644 --- a/src/Compiler/Service/IncrementalBuild.fs +++ b/src/Compiler/Service/IncrementalBuild.fs @@ -375,11 +375,13 @@ type BoundModel private ( return diags } |> GraphNode - let startComputingFullTypeCheck = - node { - let! _ = tcInfoExtras.GetOrComputeValue() - return! diagnostics.GetOrComputeValue() - } + //let startComputingFullTypeCheck() = + // async { + // use _ = InitCompilationGlobalsScope() + // do! tcInfoExtras.GetOrComputeValue() |> Async.AwaitNodeCode |> Async.Ignore + // do! diagnostics.GetOrComputeValue() |> Async.AwaitNodeCode |> Async.Ignore + // } + // |> Async.Start let tcInfo, tcInfoExtras = match tcStateOpt with @@ -391,7 +393,7 @@ type BoundModel private ( GraphNode.FromResult tcInfo, tcInfoExtras | _ -> // start computing extras, so that typeCheckNode can be GC'd quickly - startComputingFullTypeCheck |> Async.AwaitNodeCode |> Async.Catch |> Async.Ignore |> Async.Start + // startComputingFullTypeCheck() getTcInfo typeCheckNode, tcInfoExtras member val Diagnostics = diagnostics @@ -735,7 +737,7 @@ module IncrementalBuilderHelpers = } /// Finish up the typechecking to produce outputs for the rest of the compilation process - let FinalizeTypeCheckTask (tcConfig: TcConfig) tcGlobals partialCheck assemblyName outfile (boundModels: GraphNode seq) = + let FinalizeTypeCheckTask (tcConfig: TcConfig) tcGlobals _partialCheck assemblyName outfile (boundModels: GraphNode seq) = node { let diagnosticsLogger = CompilationDiagnosticLogger("FinalizeTypeCheckTask", tcConfig.diagnosticsOptions) use _ = new CompilationGlobalsScope(diagnosticsLogger, BuildPhase.TypeCheck) @@ -751,13 +753,13 @@ module IncrementalBuilderHelpers = let! latestImplFiles = computedBoundModels |> Seq.map (fun boundModel -> node { - if partialCheck then - return None - else + //if partialCheck then + // return None + //else let! tcInfoExtras = boundModel.GetOrComputeTcInfoExtras() return tcInfoExtras.latestImplFile }) - |> NodeCode.Parallel + |> NodeCode.Sequential let results = [ for tcInfo, latestImplFile in Seq.zip tcInfos latestImplFiles -> @@ -826,7 +828,7 @@ module IncrementalBuilderHelpers = let! partialDiagnostics = computedBoundModels |> Seq.map (fun m -> m.Diagnostics.GetOrComputeValue()) - |> NodeCode.Parallel + |> NodeCode.Sequential let diagnostics = [ diagnosticsLogger.GetDiagnostics() yield! partialDiagnostics |> Seq.rev From a82fb9be0e347cc2f5dfb388fa1183231b7e6265 Mon Sep 17 00:00:00 2001 From: Jakub Majocha Date: Mon, 4 Mar 2024 15:32:39 +0100 Subject: [PATCH 6/6] restore --- src/Compiler/Driver/CompilerImports.fs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Compiler/Driver/CompilerImports.fs b/src/Compiler/Driver/CompilerImports.fs index 6d564b62a0c..090e5597440 100644 --- a/src/Compiler/Driver/CompilerImports.fs +++ b/src/Compiler/Driver/CompilerImports.fs @@ -2244,8 +2244,6 @@ and [] TcImports |> List.map (fun nm -> node { try - use _ = new CompilationGlobalsScope() - return! tcImports.TryRegisterAndPrepareToImportReferencedDll(ctok, nm) with e -> errorR (Error(FSComp.SR.buildProblemReadingAssembly (nm.resolvedPath, e.Message), nm.originalReference.Range))